From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753602AbbCJPay (ORCPT ); Tue, 10 Mar 2015 11:30:54 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:20950 "EHLO imgpgp01.kl.imgtec.org" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752233AbbCJPaw (ORCPT ); Tue, 10 Mar 2015 11:30:52 -0400 X-PGP-Universal: processed; by imgpgp01.kl.imgtec.org on Tue, 10 Mar 2015 15:30:50 +0000 Message-ID: <54FF0E22.3010904@imgtec.com> Date: Tue, 10 Mar 2015 15:30:42 +0000 From: James Hogan User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: "Paul E. McKenney" , CC: , , , , , , , , , , , , , , , Subject: Re: [PATCH tip/core/rcu 04/20] metag: Use common outgoing-CPU-notification code 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> In-Reply-To: <1425404595-17816-4-git-send-email-paulmck@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm" X-Originating-IP: [192.168.154.110] X-ESG-ENCRYPT-TAG: da4c5968 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Hi Paul, On 03/03/15 17:42, Paul E. McKenney wrote: > From: "Paul E. McKenney" >=20 > 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. >=20 > 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 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); =20 /* 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(-) >=20 > 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) > } > =20 > #ifdef CONFIG_HOTPLUG_CPU > -static DECLARE_COMPLETION(cpu_killed); > =20 > /* > * __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); > } > =20 > @@ -314,7 +313,7 @@ void cpu_die(void) > local_irq_disable(); > idle_task_exit(); > =20 > - complete(&cpu_killed); > + (void)cpu_report_death(); > =20 > asm ("XOR TXENABLE, D0Re0,D0Re0\n"); > } >=20 --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU/w4pAAoJEGwLaZPeOHZ68lMP/R5ONF63WmYQKuPq9XhzBRZO 4BViN0APD+5SAhQdVjsW+nKkV7yurMRNs/B3K+wUDQBBAWtrQhkYKDFUVoDaeZ9t qaMhmMuj4+Z+p+iW3L/nWkn7eSI2rdDoJtqQq8UjkxV+npMdj4sqGME9yLuaqlzo WUPQUdgMUwWN+9u/pGoYEM6Odq3W7HcEERovygPvtzqO8Azy2fUYx+fzBpYbYok7 8y73ojS/L2qptpMfF/uf1dIH+n+V3ueARRtdUIdfFjikCId3VLpHHEkZjjLIgcjn J3tcsRPpOYSfLPVMoKNmegaSL9vvIWh+Kq4x49OsCrnBgFHr3FU3dmbfGyGjJkWY t74mQehRhUdMJGO93S5luRckiqga3MPVbOgbz7C/F4sDfbKuMSbpXHTRsEFs0vzj SnlIsUhWc/zQTQDbf0VEV6+Akc2/a1M9dwm18As4kXlHYwzrk7PRWJFNtpIiiARe bPdTf4QT6TY8hbbkQOLXSk2R31eSwQgXfbs3zf67Ialzjwf42XawTAYuMwtDQTTU EmBie8RX5mlarS1LaJgqph3cI9E8yMJrWapko/9YFgscb9uRvAQoIna/r27NGqtK VFvPHEoq2jj91SV6NHZYEzxtJN08EDv6XUch350kGbcrmCovEsStZjhC9zz66Lth VvwEWxoqrC52YFRcLBlA =rMqd -----END PGP SIGNATURE----- --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: James Hogan Subject: Re: [PATCH tip/core/rcu 04/20] metag: Use common outgoing-CPU-notification code Date: Tue, 10 Mar 2015 15:30:42 +0000 Message-ID: <54FF0E22.3010904@imgtec.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> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm" Return-path: In-Reply-To: <1425404595-17816-4-git-send-email-paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> Sender: linux-metag-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: To: "Paul E. McKenney" , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org, dipankar-xthvdsQ13ZrQT0dZR+AlfA@public.gmane.org, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org, josh-iaAMLnmF4UmaiuxdJuQwMA@public.gmane.org, tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org, peterz-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org, rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org, dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org, dvhart-VuQAYsv1563Yd54FQh9/CA@public.gmane.org, fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org, linux-metag-u79uwXL29TY76Z2rM5mHXA@public.gmane.org --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Hi Paul, On 03/03/15 17:42, Paul E. McKenney wrote: > From: "Paul E. McKenney" >=20 > 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. >=20 > 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 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); =20 /* 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(-) >=20 > 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) > } > =20 > #ifdef CONFIG_HOTPLUG_CPU > -static DECLARE_COMPLETION(cpu_killed); > =20 > /* > * __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); > } > =20 > @@ -314,7 +313,7 @@ void cpu_die(void) > local_irq_disable(); > idle_task_exit(); > =20 > - complete(&cpu_killed); > + (void)cpu_report_death(); > =20 > asm ("XOR TXENABLE, D0Re0,D0Re0\n"); > } >=20 --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJU/w4pAAoJEGwLaZPeOHZ68lMP/R5ONF63WmYQKuPq9XhzBRZO 4BViN0APD+5SAhQdVjsW+nKkV7yurMRNs/B3K+wUDQBBAWtrQhkYKDFUVoDaeZ9t qaMhmMuj4+Z+p+iW3L/nWkn7eSI2rdDoJtqQq8UjkxV+npMdj4sqGME9yLuaqlzo WUPQUdgMUwWN+9u/pGoYEM6Odq3W7HcEERovygPvtzqO8Azy2fUYx+fzBpYbYok7 8y73ojS/L2qptpMfF/uf1dIH+n+V3ueARRtdUIdfFjikCId3VLpHHEkZjjLIgcjn J3tcsRPpOYSfLPVMoKNmegaSL9vvIWh+Kq4x49OsCrnBgFHr3FU3dmbfGyGjJkWY t74mQehRhUdMJGO93S5luRckiqga3MPVbOgbz7C/F4sDfbKuMSbpXHTRsEFs0vzj SnlIsUhWc/zQTQDbf0VEV6+Akc2/a1M9dwm18As4kXlHYwzrk7PRWJFNtpIiiARe bPdTf4QT6TY8hbbkQOLXSk2R31eSwQgXfbs3zf67Ialzjwf42XawTAYuMwtDQTTU EmBie8RX5mlarS1LaJgqph3cI9E8yMJrWapko/9YFgscb9uRvAQoIna/r27NGqtK VFvPHEoq2jj91SV6NHZYEzxtJN08EDv6XUch350kGbcrmCovEsStZjhC9zz66Lth VvwEWxoqrC52YFRcLBlA =rMqd -----END PGP SIGNATURE----- --m3BRP7MDDWHggMdfPnK8PmJOMHJEDO8Jm-- -- To unsubscribe from this list: send the line "unsubscribe linux-metag" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html