diff mbox

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

Message ID 1429819654-10498-1-git-send-email-paulmck@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul E. McKenney April 23, 2015, 8:07 p.m. UTC 
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(-)

Comments

Catalin Marinas April 30, 2015, 9:50 a.m. UTC | #1 
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.
Paul E. McKenney May 1, 2015, 3:08 p.m. UTC | #2 
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
Catalin Marinas May 1, 2015, 4:29 p.m. UTC | #3 
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?
Paul E. McKenney May 1, 2015, 7:18 p.m. UTC | #4 
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
diff mbox

Patch

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