Patchwork [RFC] sparc64: remove __ARCH_WANT_UNLOCKED_CTXSW usage

login
register
mail settings
Submitter Peter Zijlstra
Date Dec. 12, 2011, 3:14 p.m.
Message ID <1323702852.13285.25.camel@twins>
Download mbox | patch
Permalink /patch/130763/
State RFC
Delegated to: David Miller
Headers show

Comments

Peter Zijlstra - Dec. 12, 2011, 3:14 p.m.
Hi David,

As mentioned a while ago, I was looking into why SPARC64 uses
__ARCH_WANT_UNLOCKED_CTXSW. You thought to remember some AB-BA deadlock
with the rq->lock so I went through the various sparc64 arch hooks but
came up empty.

switch_to()
  flush_tlb_pending()
    flush_tsb_user
      mm->context.lock

switch_mm()
  mm->context.lock
  get_new_mmu_context()
    ctx_alloc_lock()

I went through all sites where either mm->context.lock or ctx_alloc_lock
was used but could not find anything calling back into the scheduler --
this would have to be a wakeup because everything is ran with IRQs
disabled.

There's also activate_mm() that takes both these locks and is ran under
task_lock(), but there too I cannot see any problems with rq->lock.

This investigation has one assumption; that pure assembly functions are
'clean', ie. they don't take locks and don't go calling try_to_wake_up()
etc. This because my sparc64 asm is very much gone from memory (some 10+
years ago I could have maybe followed it).

The only thing resting me is asking you to simply test the below patch
and report what happens. Hopefully things will simply work.. if not I've
messed up and need to go look harder :/


---
 arch/sparc/include/asm/system_64.h |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 21, 2011, 9:44 p.m.
From: Peter Zijlstra <peterz@infradead.org>
Date: Mon, 12 Dec 2011 16:14:12 +0100

> The only thing resting me is asking you to simply test the below patch
> and report what happens. Hopefully things will simply work.. if not I've
> messed up and need to go look harder :/

I'll give it a spin, and get back to you, thanks!
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 21, 2011, 11:05 p.m.
From: David Miller <davem@davemloft.net>
Date: Wed, 21 Dec 2011 16:44:26 -0500 (EST)

> From: Peter Zijlstra <peterz@infradead.org>
> Date: Mon, 12 Dec 2011 16:14:12 +0100
> 
>> The only thing resting me is asking you to simply test the below patch
>> and report what happens. Hopefully things will simply work.. if not I've
>> messed up and need to go look harder :/
> 
> I'll give it a spin, and get back to you, thanks!

Ok, so far so good, I stressed out my Niagara-T3 box for a while doing gcc
bootstraps, running the testsuite, etc. and all seems well so far.

I'll keep beating on it and let you know if anything funny happens, but for
now as far as I'm concerned:

Acked-by: David S. Miller <davem@davemloft.net>
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Zijlstra - Dec. 22, 2011, 7:16 p.m.
On Wed, 2011-12-21 at 18:05 -0500, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 21 Dec 2011 16:44:26 -0500 (EST)
> 
> > From: Peter Zijlstra <peterz@infradead.org>
> > Date: Mon, 12 Dec 2011 16:14:12 +0100
> > 
> >> The only thing resting me is asking you to simply test the below patch
> >> and report what happens. Hopefully things will simply work.. if not I've
> >> messed up and need to go look harder :/
> > 
> > I'll give it a spin, and get back to you, thanks!
> 
> Ok, so far so good, I stressed out my Niagara-T3 box for a while doing gcc
> bootstraps, running the testsuite, etc. and all seems well so far.
> 
> I'll keep beating on it and let you know if anything funny happens, but for
> now as far as I'm concerned:
> 
> Acked-by: David S. Miller <davem@davemloft.net>

Thanks David!! 

I'll probably post again (and propose merger) once Catalin's patches
that remove __ARCH_WANT_INTERRUPTS_ON_CTXSW from ARM have landed.

ARM being the last user of that feature, and with only SPARC64 and IA64
(needlessly) using __ARCH_WANT_UNLOCKED_CTXSW we could finally remove
some of that hairy code from the core scheduler.


--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Dec. 22, 2011, 7:22 p.m.
From: Peter Zijlstra <peterz@infradead.org>
Date: Thu, 22 Dec 2011 20:16:39 +0100

> ARM being the last user of that feature, and with only SPARC64 and IA64
> (needlessly) using __ARCH_WANT_UNLOCKED_CTXSW we could finally remove
> some of that hairy code from the core scheduler.

W00t! :-)
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/include/asm/system_64.h b/arch/sparc/include/asm/system_64.h
index 10bcabc..715fefa 100644
--- a/arch/sparc/include/asm/system_64.h
+++ b/arch/sparc/include/asm/system_64.h
@@ -123,8 +123,6 @@  extern void __flushw_user(void);
 #define flush_user_windows flushw_user
 #define flush_register_windows flushw_all
 
-/* Don't hold the runqueue lock over context switch */
-#define __ARCH_WANT_UNLOCKED_CTXSW
 #define prepare_arch_switch(next)		\
 do {						\
 	flushw_all();				\