ARM: CPU hotplug: Delegate complete() to surviving CPU
PATCH 1/1 ARM: CPU hotplug: Delegate complete() to surviving CPU
The ARM implementation of arch_cpu_idle_dead() invokes complete(), but does so after RCU has stopped watching the outgoing CPU, which results in lockdep complaints because complete() invokes functions containing RCU readers. This patch therefore uses Thomas Gleixner's trick of delegating the complete() call to a surviving CPU via smp_call_function_single().
arch/arm/kernel/smp.c +12 -2
@@ -267,6 +267,14 @@ void __cpu_die(unsigned int cpu) }/* +* Invoke complete() on behalf of the outgoing CPU. +*/ +static void arch_cpu_idle_dead_complete (void *arg) +{ +complete (&cpu_died); +} + +/* * Called from the idle thread for the CPU which has been shutdown. * * Note that we disable IRQs here, but do not re-enable them
@@ -293,9 +301,11 @@ void arch_cpu_idle_dead(void)/* * Tell __cpu_die() that this CPU is now safe to dispose of. Once * this returns, power and/or clocks can be removed at any point -* from this CPU and its cache by platform_cpu_kill(). +* from this CPU and its cache by platform_cpu_kill(). We cannot +* call complete() this late, so we delegate it to an online CPU. */ -complete (&cpu_died); +smp_call_function_single (cpumask_first (cpu_online_mask), + arch_cpu_idle_dead_complete,NULL ,0 );/* * Ensure that the cache lines associated with that completion are
Russell King - ARM Linux
maintainerQuoted 6 lines · Paul E. McKenney
The ARM implementation of
arch_cpu_idle_dead()invokescomplete(), but does so after RCU has stopped watching the outgoing CPU, which results in lockdep complaints becausecomplete()invokes functions containing RCU readers. This patch therefore uses Thomas Gleixner's trick of delegating thecomplete()call to a surviving CPU viasmp_call_function_single().
Quoted 10 lines
Reported-by Peng Fan <[email protected]>Reported-by Russell King - ARM Linux <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Cc Russell King <[email protected]>Cc Ingo Molnar <[email protected]>Cc "Peter Zijlstra (Intel)" <[email protected]>Cc Michal Hocko <[email protected]>Cc Thomas Gleixner <[email protected]>Cc <[email protected]>
As I just described in response to Fabio's testing, this doesn't solve anything if CONFIG_BL_SWITCHER is enabled. We could lose the unlock of a spinlock in the GIC code for sending the IPI. As I already said previously in our discussion (but I guess you just don't believe me):
"2. there's some optional locking in the GIC driver that cause problems
for the cpu dying path.
The concensus last time around was that the IPI solution is a non- starter, so the seven year proven-reliable solution (disregarding the RCU warning) persists because I don't think anyone came up with a better solution."
Using smp_call_function_single() invokes the IPI paths.
Signature · 3 lines
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
Quoted 18 lines · Russell King - ARM Linux
Quoted 6 lines · Paul E. McKenney
The ARM implementation of
arch_cpu_idle_dead()invokescomplete(), but does so after RCU has stopped watching the outgoing CPU, which results in lockdep complaints becausecomplete()invokes functions containing RCU readers. This patch therefore uses Thomas Gleixner's trick of delegating thecomplete()call to a surviving CPU viasmp_call_function_single().Quoted 10 lines
Reported-by Peng Fan <[email protected]>Reported-by Russell King - ARM Linux <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Cc Russell King <[email protected]>Cc Ingo Molnar <[email protected]>Cc "Peter Zijlstra (Intel)" <[email protected]>Cc Michal Hocko <[email protected]>Cc Thomas Gleixner <[email protected]>Cc <[email protected]>
Quoted 4 lines
As I just described in response to Fabio's testing, this doesn't solve anything if
CONFIG_BL_SWITCHERis enabled. We could lose the unlock of a spinlock in the GIC code for sending the IPI. As I already said previously in our discussion (but I guess you just don't believe me):
Sorry, Russell, but most days I don't even believe myself. So it is nothing personal, just one of the occupational hazards of being me.
Quoted 9 lines
"2. there's some optional locking in the GIC driver that cause problems
for the cpu dying path.The concensus last time around was that the IPI solution is a non- starter, so the seven year proven-reliable solution (disregarding the RCU warning) persists because I don't think anyone came up with a better solution."
Using
smp_call_function_single()invokes the IPI paths.
OK, another approach is to have the dying CPU simply set an in-memory flag, which a surviving CPU polls for. There are of course any number of ways of doing the polling loop.
So what bad thing happens when you use that approach?
Thanx, Paul
Hi Paul,
Quoted 10 lines · Paul E. McKenney
The ARM implementation of
arch_cpu_idle_dead()invokescomplete(), but does so after RCU has stopped watching the outgoing CPU, which results in lockdep complaints becausecomplete()invokes functions containing RCU readers. This patch therefore uses Thomas Gleixner's trick of delegating thecomplete()call to a surviving CPU viasmp_call_function_single().Reported-by Peng Fan <[email protected]>Reported-by Russell King - ARM Linux <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
Fabio reported only once, though he might have tested twice.
baruch
Quoted 40 lines
Cc Russell King <[email protected]>Cc Ingo Molnar <[email protected]>Cc "Peter Zijlstra (Intel)" <[email protected]>Cc Michal Hocko <[email protected]>Cc Thomas Gleixner <[email protected]>Cc <[email protected]>arch/arm/kernel/smp.c +12
-2@@ -267,6 +267,14 @@ void __cpu_die(unsigned int cpu) }/* +* Invoke complete() on behalf of the outgoing CPU. +*/ +static void arch_cpu_idle_dead_complete (void *arg) +{ +complete (&cpu_died); +} + +/* * Called from the idle thread for the CPU which has been shutdown. * * Note that we disable IRQs here, but do not re-enable them @@ -293,9 +301,11 @@ void arch_cpu_idle_dead(void)/* * Tell __cpu_die() that this CPU is now safe to dispose of. Once * this returns, power and/or clocks can be removed at any point -* from this CPU and its cache by platform_cpu_kill(). +* from this CPU and its cache by platform_cpu_kill(). We cannot +* call complete() this late, so we delegate it to an online CPU. */ -complete (&cpu_died); +smp_call_function_single (cpumask_first (cpu_online_mask), + arch_cpu_idle_dead_complete,NULL ,0 );/* * Ensure that the cache lines associated with that completion are
Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -Quoted 9 lines · Baruch Siach
Hi Paul,
Quoted 6 lines · Paul E. McKenney
The ARM implementation of
arch_cpu_idle_dead()invokescomplete(), but does so after RCU has stopped watching the outgoing CPU, which results in lockdep complaints becausecomplete()invokes functions containing RCU readers. This patch therefore uses Thomas Gleixner's trick of delegating thecomplete()call to a surviving CPU viasmp_call_function_single().
Quoted 6 lines
Quoted 4 lines
Reported-by Peng Fan <[email protected]>Reported-by Russell King - ARM Linux <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>Fabio reported only once, though he might have tested twice.
Actually, he did test twice. Apparently this patch has problems with CONFIG_BL_SWITCHER=y kernels.
But yes, I guess I did get a bit carried away with the Tested-by's, didn't I? ;-)
Thanx, Paul
Quoted 48 lines
baruch
Quoted 40 lines
Cc Russell King <[email protected]>Cc Ingo Molnar <[email protected]>Cc "Peter Zijlstra (Intel)" <[email protected]>Cc Michal Hocko <[email protected]>Cc Thomas Gleixner <[email protected]>Cc <[email protected]>arch/arm/kernel/smp.c +12
-2@@ -267,6 +267,14 @@ void __cpu_die(unsigned int cpu) }/* +* Invoke complete() on behalf of the outgoing CPU. +*/ +static void arch_cpu_idle_dead_complete (void *arg) +{ +complete (&cpu_died); +} + +/* * Called from the idle thread for the CPU which has been shutdown. * * Note that we disable IRQs here, but do not re-enable them @@ -293,9 +301,11 @@ void arch_cpu_idle_dead(void)/* * Tell __cpu_die() that this CPU is now safe to dispose of. Once * this returns, power and/or clocks can be removed at any point -* from this CPU and its cache by platform_cpu_kill(). +* from this CPU and its cache by platform_cpu_kill(). We cannot +* call complete() this late, so we delegate it to an online CPU. */ -complete (&cpu_died); +smp_call_function_single (cpumask_first (cpu_online_mask), + arch_cpu_idle_dead_complete,NULL ,0 );/* * Ensure that the cache lines associated with that completion are Signature · 3 lines
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -