Re: [PATCH tip/core/rcu 04/20] metag: Use common outgoing-CPU-notification code
From: Paul E. McKenney
Date: Tue Mar 10 2015 - 12:59:53 EST
On Tue, Mar 10, 2015 at 03:30:42PM +0000, James Hogan wrote:
> Hi Paul,
> On 03/03/15 17:42, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxxxxxxx>
> > This commit removes the open-coded CPU-offline notification with new
> > common code. This change avoids calling scheduler code using RCU from
> > an offline CPU that RCU is ignoring. This commit is compatible with
> > the existing code in not checking for timeout during a prior offline
> > for a given CPU.
> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > Cc: James Hogan <james.hogan@xxxxxxxxxx>
> > Cc: <linux-metag@xxxxxxxxxxxxxxx>
> I gave this a try via linux-next, but unfortunately it causes the
> following warning every time a CPU goes down:
> META213-Thread0 DSP [LogF] CPU1: unable to kill
That is certainly not what I had in mind, thank you for finding this!
> If I add printks, I see that the state on entry to both cpu_wait_death
> and cpu_report_death is already CPU_POST_DEAD, suggesting that it hasn't
> changed from its initial value.
> Should arches other than x86 now be calling cpu_set_state_online()? The
> patchlet below seems to resolve it for Meta (not sure if that is the
> best place in the startup sequence to do it, perhaps it doesn't matter).
> diff --git a/arch/metag/kernel/smp.c b/arch/metag/kernel/smp.c
> index ac3a199e33e7..430e379ec71f 100644
> --- a/arch/metag/kernel/smp.c
> +++ b/arch/metag/kernel/smp.c
> @@ -383,6 +383,7 @@ asmlinkage void secondary_start_kernel(void)
> * OK, now it's safe to let the boot CPU continue
> set_cpu_online(cpu, true);
> + cpu_set_state_online(cpu);
> Looking at the comment before cpu_set_state_online:
> > /*
> > * Mark the specified CPU online.
> > *
> > * Note that it is permissible to omit this call entirely, as is
> > * done in architectures that do no CPU-hotplug error checking.
> > */
> Which suggests it wasn't wrong to omit it before your patches came
And that suggestion is quite correct. The idea was indeed to accommodate
architectures that do not do error checking.
Does the following patch (on top of current -next) remove the need for
your addition of cpu_set_state_online() above?
diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index 18688e0b0422..80400e019c86 100644
@@ -460,7 +460,7 @@ bool cpu_report_death(void)
oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
- if (oldstate == CPU_ONLINE)
+ if (oldstate == CPU_ONLINE || CPU_POST_DEAD)
newstate = CPU_DEAD;
newstate = CPU_DEAD_FROZEN;
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/