Fixes for ARM use of scheduler when offline
Subsystems (2)
cover Fixes for ARM use of scheduler when offline
Hello!
This series is strictly FYI.
I am simply sending it as a heads-up. The diagnostics surrounding using RCU from an offline CPU will be tightening up, which might cause splats to come my way. If such splats do come my way, I will reply with the patches in this series.
I know you guys have been working to put something better together, which is great -- just letting you know of fallback patches, when and if. If you would like to push these up your tree or ack them and have me push them, that is of course fine as well.
The first patch in the series does arm64, and the second does arm.
Thanx, Paul
------------------------------------------------------------------------
b/arch/arm/kernel/smp.c | 6 ++---- b/arch/arm64/kernel/smp.c | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-)
PATCH 1/2 arm64: Use common outgoing-CPU-notification code
From: "Paul E. McKenney" <[email protected]>
This commit removes the open-coded CPU-offline notification with new common code. In particular, this change avoids calling scheduler code using RCU from an offline CPU that RCU is ignoring. This is a minimal change. A more intrusive change might invoke the cpu_check_up_prepare() and cpu_set_state_online() functions at CPU-online time, which would allow onlining throw an error if the CPU did not go offline properly.
--- arch/arm64/kernel/smp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
arch/arm64/kernel/smp.c +2 -4
@@ -252,15 +252,13 @@ static int op_cpu_kill(unsigned int cpu)return cpu_ops[cpu]->cpu_kill (cpu); } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_crit ("CPU%u: cpu didn't die\n" , cpu);return ; }
@@ -293,7 +291,7 @@ void cpu_die(void)local_irq_disable ();/* Tell __cpu_die() that this CPU is now safe to dispose of */ -complete (&cpu_died); + (void )cpu_report_death ();/* * Actually shutdown the CPU. This must never fail. The specific hotplug
Signature · 1 line
1.8.1.5
PATCH 2/2 arm: Use common outgoing-CPU-notification code
From: "Paul E. McKenney" <[email protected]>
This commit removes the open-coded CPU-offline notification with new common code. In particular, this change avoids calling scheduler code using RCU from an offline CPU that RCU is ignoring. This is a minimal change. A more intrusive change might invoke the cpu_check_up_prepare() and cpu_set_state_online() functions at CPU-online time, which would allow onlining throw an error if the CPU did not go offline properly.
--- arch/arm/kernel/smp.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
arch/arm/kernel/smp.c +2 -4
@@ -213,15 +213,13 @@ int __cpu_disable(void)return 0 ; } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_err ("CPU%u: cpu didn't die\n" , cpu);return ; }
@@ -267,7 +265,7 @@ void __ref cpu_die(void)* this returns, power and/or clocks can be removed at any point * from this CPU and its cache by platform_cpu_kill(). */ -complete (&cpu_died); + (void )cpu_report_death ();/* * Ensure that the cache lines associated with that completion are
Signature · 1 line
1.8.1.5
Catalin Marinas
maintainerRe: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code
In reply to Paul E. McKenneyHi Paul,
Quoted 22 lines · Paul E. McKenney
arch/arm64/kernel/smp.c +1
-3@@ -252,15 +252,13 @@ static int op_cpu_kill(unsigned int cpu)return cpu_ops[cpu]->cpu_kill (cpu); } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_crit ("CPU%u: cpu didn't die\n" , cpu);return ; }
arch/arm64/kernel/smp.c void cpu_die(void) · line 297
Quoted 8 lines
arch/arm64/kernel/smp.c +1
-1@@ -293,7 +291,7 @@ void cpu_die(void)local_irq_disable ();/* Tell __cpu_die() that this CPU is now safe to dispose of */ -complete (&cpu_died); + (void )cpu_report_death ();/* * Actually shutdown the CPU. This must never fail. The specific hotplug
Are these functions are only defined when HOTPLUG_CPU is enabled? On arm64 we can end up with this option disabled if SUSPEND is disabled.
Signature · 1 line
Catalin
Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code
Quoted 31 lines · Catalin Marinas
Hi Paul,
Quoted 28 lines · Paul E. McKenney
arch/arm64/kernel/smp.c +2
-4@@ -252,15 +252,13 @@ static int op_cpu_kill(unsigned int cpu)return cpu_ops[cpu]->cpu_kill (cpu); } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_crit ("CPU%u: cpu didn't die\n" , cpu);return ; }@@ -293,7 +291,7 @@ void cpu_die(void)local_irq_disable ();/* Tell __cpu_die() that this CPU is now safe to dispose of */ -complete (&cpu_died); + (void )cpu_report_death ();
Quoted 5 lines
arch/arm64/kernel/smp.c void cpu_die(void) · line 297
Quoted 2 lines
arch/arm64/kernel/smp.c +0
-0/* * Actually shutdown the CPU. This must never fail. The specific hotplug Are these functions are only defined when
HOTPLUG_CPUis enabled? On arm64 we can end up with this option disabled if SUSPEND is disabled.
Yep, only with HOTPLUG_CPU. And yes, both SUSPEND and HIBERNATION can force HOTPLUG_CPU, so if you have neiter SUSPEND nor HIBERNATION, you won't have HOTPLUG_CPU. This -should- be OK, because you should also not have __cpu_die() if !HOTPLUG_CPU.
Or am I missing your point?
Thanx, Paul
Catalin Marinas
maintainerRe: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code
Quoted 36 lines · Paul E. McKenney
Quoted 29 lines · Catalin Marinas
Quoted 28 lines · Paul E. McKenney
arch/arm64/kernel/smp.c +2
-4@@ -252,15 +252,13 @@ static int op_cpu_kill(unsigned int cpu)return cpu_ops[cpu]->cpu_kill (cpu); } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_crit ("CPU%u: cpu didn't die\n" , cpu);return ; }@@ -293,7 +291,7 @@ void cpu_die(void)local_irq_disable ();/* Tell __cpu_die() that this CPU is now safe to dispose of */ -complete (&cpu_died); + (void )cpu_report_death ();Quoted 5 lines
arch/arm64/kernel/smp.c void cpu_die(void) · line 297
Quoted 2 lines
arch/arm64/kernel/smp.c +0
-0/* * Actually shutdown the CPU. This must never fail. The specific hotplug Are these functions are only defined when
HOTPLUG_CPUis enabled? On arm64 we can end up with this option disabled if SUSPEND is disabled.
Quoted 4 lines
Yep, only with
HOTPLUG_CPU. And yes, both SUSPEND and HIBERNATION can forceHOTPLUG_CPU, so if you have neiter SUSPEND nor HIBERNATION, you won't haveHOTPLUG_CPU. This -should- be OK, because you should also not have__cpu_die()if !HOTPLUG_CPU.
Ah, you are right. I got myself confused.
So is this patch ready for upstreaming or just for our information?
Signature · 1 line
Catalin
Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code
Quoted 37 lines · Catalin Marinas
Quoted 36 lines · Paul E. McKenney
Quoted 34 lines · Catalin Marinas
Quoted 22 lines · Paul E. McKenney
arch/arm64/kernel/smp.c +1
-3@@ -252,15 +252,13 @@ static int op_cpu_kill(unsigned int cpu)return cpu_ops[cpu]->cpu_kill (cpu); } -static DECLARE_COMPLETION (cpu_died); -/* * called on the thread which is asking for a CPU to be shutdown - * waits until shutdown has completed, or it is timed out. */ void __cpu_die (unsigned int cpu) { -if (!wait_for_completion_timeout (&cpu_died,msecs_to_jiffies (5000 ))) { +if (!cpu_wait_death (cpu,5 )) {pr_crit ("CPU%u: cpu didn't die\n" , cpu);return ; }arch/arm64/kernel/smp.c void cpu_die(void) · line 297
Quoted 8 lines
arch/arm64/kernel/smp.c +1
-1@@ -293,7 +291,7 @@ void cpu_die(void)local_irq_disable ();/* Tell __cpu_die() that this CPU is now safe to dispose of */ -complete (&cpu_died); + (void )cpu_report_death ();/* * Actually shutdown the CPU. This must never fail. The specific hotplug Are these functions are only defined when
HOTPLUG_CPUis enabled? On arm64 we can end up with this option disabled if SUSPEND is disabled.
Quoted 8 lines
Quoted 4 lines
Yep, only with
HOTPLUG_CPU. And yes, both SUSPEND and HIBERNATION can forceHOTPLUG_CPU, so if you have neiter SUSPEND nor HIBERNATION, you won't haveHOTPLUG_CPU. This -should- be OK, because you should also not have__cpu_die()if !HOTPLUG_CPU.Ah, you are right. I got myself confused.
So is this patch ready for upstreaming or just for our information?
It does need some testing, and I don't have access to an arm64 system. That said, very similar patches have worked for other architectures. So if it works on one of your systems, it should be ready to go upstream. Which would be better than me sending it individually to people who run into the new more-strict diagnostics. ;-)
Thanx, Paul