From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753531AbbCJQ7u (ORCPT ); Tue, 10 Mar 2015 12:59:50 -0400 Received: from e37.co.us.ibm.com ([32.97.110.158]:60687 "EHLO e37.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752780AbbCJQ7o (ORCPT ); Tue, 10 Mar 2015 12:59:44 -0400 Date: Tue, 10 Mar 2015 09:59:35 -0700 From: "Paul E. McKenney" To: James Hogan Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, laijs@cn.fujitsu.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org, rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com, dvhart@linux.intel.com, fweisbec@gmail.com, oleg@redhat.com, bobby.prani@gmail.com, linux-metag@vger.kernel.org Subject: Re: [PATCH tip/core/rcu 04/20] metag: Use common outgoing-CPU-notification code Message-ID: <20150310165935.GR5708@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20150303174144.GA13139@linux.vnet.ibm.com> <1425404595-17816-1-git-send-email-paulmck@linux.vnet.ibm.com> <1425404595-17816-4-git-send-email-paulmck@linux.vnet.ibm.com> <54FF0E22.3010904@imgtec.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54FF0E22.3010904@imgtec.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15031016-0025-0000-0000-000009270E64 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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" > > > > 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 > > Cc: James Hogan > > Cc: > > 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;