linux-kernel - Re: [PATCH tip/core/rcu 04/20] metag: Use common outgoing-CPU-notification code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Mar 2015 09:59:35 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	James Hogan <james.hogan@...tec.com>
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	laijs@...fujitsu.com, dipankar@...ibm.com,
	akpm@...ux-foundation.org, mathieu.desnoyers@...icios.com,
	josh@...htriplett.org, tglx@...utronix.de, peterz@...radead.org,
	rostedt@...dmis.org, dhowells@...hat.com, edumazet@...gle.com,
	dvhart@...ux.intel.com, fweisbec@...il.com, oleg@...hat.com,
	bobby.prani@...il.com, linux-metag@...r.kernel.org
Subject: Re: [PATCH tip/core/rcu 04/20] metag: Use common
 outgoing-CPU-notification code

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@...ux.vnet.ibm.com>
> > 
> > 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@...ux.vnet.ibm.com>
> > Cc: James Hogan <james.hogan@...tec.com>
> > Cc: <linux-metag@...r.kernel.org>
> 
> 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);
>  	complete(&cpu_running);
>  
>  	/*
> 
> 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
> along.

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?

							Thanx, Paul

------------------------------------------------------------------------

diff --git a/kernel/smpboot.c b/kernel/smpboot.c
index 18688e0b0422..80400e019c86 100644
--- a/kernel/smpboot.c
+++ b/kernel/smpboot.c
@@ -460,7 +460,7 @@ bool cpu_report_death(void)
 
 	do {
 		oldstate = atomic_read(&per_cpu(cpu_hotplug_state, cpu));
-		if (oldstate == CPU_ONLINE)
+		if (oldstate == CPU_ONLINE || CPU_POST_DEAD)
 			newstate = CPU_DEAD;
 		else
 			newstate = CPU_DEAD_FROZEN;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/