[PATCH arm 0/2] Fixes for ARM use of scheduler when offline

[PATCH arm 0/2] Fixes for ARM use of scheduler when offline

[Paul E. McKenney]

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 arm 1/2] arm64: Use common outgoing-CPU-notification code

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

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.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/smp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 328b8ce4b007..6dc727a6e73e 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -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
-- 
1.8.1.5

[PATCH arm 2/2] arm: Use common outgoing-CPU-notification code

[Paul E. McKenney]

From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>

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.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Russell King <linux@arm.linux.org.uk>
---
 arch/arm/kernel/smp.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 86ef244c5a24..de309f431463 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -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
-- 
1.8.1.5

Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code

[Catalin Marinas]

Hi Paul,

On Thu, Apr 23, 2015 at 01:07:33PM -0700, Paul E. McKenney wrote:
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 328b8ce4b007..6dc727a6e73e 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -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

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.

-- 
Catalin

Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code

[Paul E. McKenney]

On Thu, Apr 30, 2015 at 10:50:55AM +0100, Catalin Marinas wrote:
> Hi Paul,
> 
> On Thu, Apr 23, 2015 at 01:07:33PM -0700, Paul E. McKenney wrote:
> > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> > index 328b8ce4b007..6dc727a6e73e 100644
> > --- a/arch/arm64/kernel/smp.c
> > +++ b/arch/arm64/kernel/smp.c
> > @@ -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
> 
> 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.

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

Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code

[Catalin Marinas]

On Fri, May 01, 2015 at 08:08:48AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 30, 2015 at 10:50:55AM +0100, Catalin Marinas wrote:
> > On Thu, Apr 23, 2015 at 01:07:33PM -0700, Paul E. McKenney wrote:
> > > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> > > index 328b8ce4b007..6dc727a6e73e 100644
> > > --- a/arch/arm64/kernel/smp.c
> > > +++ b/arch/arm64/kernel/smp.c
> > > @@ -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
> > 
> > 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.
> 
> 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.

Ah, you are right. I got myself confused.

So is this patch ready for upstreaming or just for our information?

-- 
Catalin

Re: [PATCH arm 1/2] arm64: Use common outgoing-CPU-notification code

[Paul E. McKenney]

On Fri, May 01, 2015 at 05:29:15PM +0100, Catalin Marinas wrote:
> On Fri, May 01, 2015 at 08:08:48AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 30, 2015 at 10:50:55AM +0100, Catalin Marinas wrote:
> > > On Thu, Apr 23, 2015 at 01:07:33PM -0700, Paul E. McKenney wrote:
> > > > diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> > > > index 328b8ce4b007..6dc727a6e73e 100644
> > > > --- a/arch/arm64/kernel/smp.c
> > > > +++ b/arch/arm64/kernel/smp.c
> > > > @@ -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
> > > 
> > > 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.
> > 
> > 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.
> 
> 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