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 15:30:42 +0000
From:	James Hogan <james.hogan@...tec.com>
To:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	<linux-kernel@...r.kernel.org>
CC:	<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

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

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.

Cheers
James


> ---
>  arch/metag/kernel/smp.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/metag/kernel/smp.c b/arch/metag/kernel/smp.c
> index f006d2276f40..ac3a199e33e7 100644
> --- a/arch/metag/kernel/smp.c
> +++ b/arch/metag/kernel/smp.c
> @@ -261,7 +261,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)
>  }
>  
>  #ifdef CONFIG_HOTPLUG_CPU
> -static DECLARE_COMPLETION(cpu_killed);
>  
>  /*
>   * __cpu_disable runs on the processor to be shutdown.
> @@ -299,7 +298,7 @@ int __cpu_disable(void)
>   */
>  void __cpu_die(unsigned int cpu)
>  {
> -	if (!wait_for_completion_timeout(&cpu_killed, msecs_to_jiffies(1)))
> +	if (!cpu_wait_death(cpu, 1))
>  		pr_err("CPU%u: unable to kill\n", cpu);
>  }
>  
> @@ -314,7 +313,7 @@ void cpu_die(void)
>  	local_irq_disable();
>  	idle_task_exit();
>  
> -	complete(&cpu_killed);
> +	(void)cpu_report_death();
>  
>  	asm ("XOR	TXENABLE, D0Re0,D0Re0\n");
>  }
> 


Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)