Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752698Ab2FCIyY (ORCPT ); Sun, 3 Jun 2012 04:54:24 -0400 Received: from mail-pb0-f46.google.com ([209.85.160.46]:42875 "EHLO mail-pb0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751327Ab2FCIyW (ORCPT ); Sun, 3 Jun 2012 04:54:22 -0400 Date: Sun, 3 Jun 2012 16:53:59 +0800 From: Yong Zhang To: "Srivatsa S. Bhat" Cc: "linux-kernel@vger.kernel.org" , Tony Luck , Ralf Baechle , Benjamin Herrenschmidt , Paul Mundt , Chris Metcalf , Konrad Rzeszutek Wilk , Mike Frysinger , Hirokazu Takata , Richard Kuo , David Howells , Bob Liu , "David S. Miller" , Richard Weinberger , Jesper Nilsson , "James E.J. Bottomley" , Martin Schwidefsky , Russell King , Matt Turner , nikunj@linux.vnet.ibm.com Subject: Re: [PATCH 01/27] smpboot: Provide a generic method to boot secondary processors Message-ID: <20120603085359.GE16829@zhy> Reply-To: Yong Zhang References: <20120601090952.31979.24799.stgit@srivatsabhat.in.ibm.com> <20120601091008.31979.93586.stgit@srivatsabhat.in.ibm.com> <4FC8B0F7.3060705@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4FC8B0F7.3060705@linux.vnet.ibm.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 12241 Lines: 399 On Fri, Jun 01, 2012 at 05:39:27PM +0530, Srivatsa S. Bhat wrote: > +void __cpuinit smpboot_start_secondary(void *arg) > +{ > + unsigned int cpu; > + > + /* > + * SMP booting is extremely fragile in some architectures. So run > + * the cpu initialization code first before anything else. > + */ > + __cpu_pre_starting(arg); > + > + preempt_disable(); > + cpu = smp_processor_id(); > + > + /* Invoke the CPU_STARTING notifier callbacks */ > + notify_cpu_starting(cpu); > + > + __cpu_pre_online(arg); > + > + /* Set the CPU in the cpu_online_mask */ > + set_cpu_online(cpu, true); > + > + __cpu_post_online(arg); > + Seems it worth to catch incorrect irq state here: WARN_ON_ONCE(!irqs_disabled()); Thanks, Yong > + /* Enable local interrupts now */ > + local_irq_enable(); > + > + wmb(); > + cpu_idle(); > + > + /* We should never reach here! */ > + BUG(); > +} > diff --git a/kernel/smpboot.h b/kernel/smpboot.h > index 80c0acf..9753cc0 100644 > --- a/kernel/smpboot.h > +++ b/kernel/smpboot.h > @@ -1,5 +1,5 @@ > -#ifndef SMPBOOT_H > -#define SMPBOOT_H > +#ifndef __SMPBOOT_H > +#define __SMPBOOT_H > > struct task_struct; > > > > From: Srivatsa S. Bhat > Subject: [PATCH 02/27] smpboot: Add provisions for arch-specific locking around cpu_online_mask > > We want to make smp booting as generic as possible and remove code > duplication in arch/ directories. > > While manipulating the cpu_online_mask, x86 uses an additional lock, i.e., > 'vector_lock'. So provide a generic way to implement such arch-specific > extra locking, by providing weakly defined functions arch_vector_lock() > and arch_vector_unlock() which can be overriden by different architectures > suitably. > > Cc: Thomas Gleixner > Cc: Suresh Siddha > Cc: Venkatesh Pallipadi > Signed-off-by: Srivatsa S. Bhat > --- > > kernel/smpboot.c | 11 ++++++++++- > 1 files changed, 10 insertions(+), 1 deletions(-) > > diff --git a/kernel/smpboot.c b/kernel/smpboot.c > index 6c26133..5ae1805 100644 > --- a/kernel/smpboot.c > +++ b/kernel/smpboot.c > @@ -107,6 +107,13 @@ void __weak __cpu_pre_online(void *arg) {} > void __weak __cpu_post_online(void *arg) {} > #endif > > +/* > + * Optional arch-specific locking for manipulating cpu_online_mask while > + * bringing up secondary CPUs. > + */ > +void __weak arch_vector_lock(void) {} > +void __weak arch_vector_unlock(void) {} > + > > /** > * smpboot_start_secondary - Generic way to boot secondary processors > @@ -129,8 +136,10 @@ void __cpuinit smpboot_start_secondary(void *arg) > > __cpu_pre_online(arg); > > - /* Set the CPU in the cpu_online_mask */ > + /* Set the CPU in the cpu_online_mask with required locks held */ > + arch_vector_lock(); > set_cpu_online(cpu, true); > + arch_vector_unlock(); > > __cpu_post_online(arg); > > > > From: Srivatsa S. Bhat > Subject: [PATCH 03/27] smpboot: Define and use cpu_state per-cpu variable in generic code > > The per-cpu variable cpu_state is used in x86 and also used in other > architectures, to track the state of the cpu during bringup and hotplug. > Pull it out into generic code. > > Cc: Tony Luck > Cc: Fenghua Yu > Cc: Ralf Baechle > Cc: Benjamin Herrenschmidt > Cc: Paul Mundt > Cc: Chris Metcalf > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: x86@kernel.org > Cc: Konrad Rzeszutek Wilk > Cc: Jeremy Fitzhardinge > Cc: Peter Zijlstra > Cc: Andrew Morton > Cc: Mike Frysinger > Cc: Yong Zhang > Cc: Venkatesh Pallipadi > Cc: Suresh Siddha > Cc: linux-ia64@vger.kernel.org > Cc: linux-mips@linux-mips.org > Cc: linuxppc-dev@lists.ozlabs.org > Cc: linux-sh@vger.kernel.org > Cc: xen-devel@lists.xensource.com > Cc: virtualization@lists.linux-foundation.org > Signed-off-by: Srivatsa S. Bhat > --- > > arch/ia64/include/asm/cpu.h | 2 -- > arch/ia64/kernel/process.c | 1 + > arch/ia64/kernel/smpboot.c | 6 +----- > arch/mips/cavium-octeon/smp.c | 4 +--- > arch/powerpc/kernel/smp.c | 6 +----- > arch/sh/include/asm/smp.h | 2 -- > arch/sh/kernel/smp.c | 4 +--- > arch/tile/kernel/smpboot.c | 4 +--- > arch/x86/include/asm/cpu.h | 2 -- > arch/x86/kernel/smpboot.c | 4 +--- > arch/x86/xen/smp.c | 1 + > include/linux/smpboot.h | 1 + > kernel/smpboot.c | 4 ++++ > 13 files changed, 13 insertions(+), 28 deletions(-) > > diff --git a/arch/ia64/include/asm/cpu.h b/arch/ia64/include/asm/cpu.h > index fcca30b..1c3acac 100644 > --- a/arch/ia64/include/asm/cpu.h > +++ b/arch/ia64/include/asm/cpu.h > @@ -12,8 +12,6 @@ struct ia64_cpu { > > DECLARE_PER_CPU(struct ia64_cpu, cpu_devices); > > -DECLARE_PER_CPU(int, cpu_state); > - > #ifdef CONFIG_HOTPLUG_CPU > extern int arch_register_cpu(int num); > extern void arch_unregister_cpu(int); > diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c > index 5e0e86d..32566c7 100644 > --- a/arch/ia64/kernel/process.c > +++ b/arch/ia64/kernel/process.c > @@ -29,6 +29,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/arch/ia64/kernel/smpboot.c b/arch/ia64/kernel/smpboot.c > index 963d2db..df00a3c 100644 > --- a/arch/ia64/kernel/smpboot.c > +++ b/arch/ia64/kernel/smpboot.c > @@ -39,6 +39,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -111,11 +112,6 @@ extern unsigned long ia64_iobase; > > struct task_struct *task_for_booting_cpu; > > -/* > - * State for each CPU > - */ > -DEFINE_PER_CPU(int, cpu_state); > - > cpumask_t cpu_core_map[NR_CPUS] __cacheline_aligned; > EXPORT_SYMBOL(cpu_core_map); > DEFINE_PER_CPU_SHARED_ALIGNED(cpumask_t, cpu_sibling_map); > diff --git a/arch/mips/cavium-octeon/smp.c b/arch/mips/cavium-octeon/smp.c > index 97e7ce9..93cd4b0 100644 > --- a/arch/mips/cavium-octeon/smp.c > +++ b/arch/mips/cavium-octeon/smp.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -252,9 +253,6 @@ static void octeon_cpus_done(void) > > #ifdef CONFIG_HOTPLUG_CPU > > -/* State of each CPU. */ > -DEFINE_PER_CPU(int, cpu_state); > - > extern void fixup_irqs(void); > > static DEFINE_SPINLOCK(smp_reserve_lock); > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c > index e1417c4..1928058a 100644 > --- a/arch/powerpc/kernel/smp.c > +++ b/arch/powerpc/kernel/smp.c > @@ -31,6 +31,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -57,11 +58,6 @@ > #define DBG(fmt...) > #endif > > -#ifdef CONFIG_HOTPLUG_CPU > -/* State of each CPU during hotplug phases */ > -static DEFINE_PER_CPU(int, cpu_state) = { 0 }; > -#endif > - > struct thread_info *secondary_ti; > > DEFINE_PER_CPU(cpumask_var_t, cpu_sibling_map); > diff --git a/arch/sh/include/asm/smp.h b/arch/sh/include/asm/smp.h > index 78b0d0f4..bda041e 100644 > --- a/arch/sh/include/asm/smp.h > +++ b/arch/sh/include/asm/smp.h > @@ -31,8 +31,6 @@ enum { > SMP_MSG_NR, /* must be last */ > }; > > -DECLARE_PER_CPU(int, cpu_state); > - > void smp_message_recv(unsigned int msg); > void smp_timer_broadcast(const struct cpumask *mask); > > diff --git a/arch/sh/kernel/smp.c b/arch/sh/kernel/smp.c > index b86e9ca..8e0fde0 100644 > --- a/arch/sh/kernel/smp.c > +++ b/arch/sh/kernel/smp.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -34,9 +35,6 @@ int __cpu_logical_map[NR_CPUS]; /* Map logical to physical */ > > struct plat_smp_ops *mp_ops = NULL; > > -/* State of each CPU */ > -DEFINE_PER_CPU(int, cpu_state) = { 0 }; > - > void __cpuinit register_smp_ops(struct plat_smp_ops *ops) > { > if (mp_ops) > diff --git a/arch/tile/kernel/smpboot.c b/arch/tile/kernel/smpboot.c > index e686c5a..24a9c06 100644 > --- a/arch/tile/kernel/smpboot.c > +++ b/arch/tile/kernel/smpboot.c > @@ -25,13 +25,11 @@ > #include > #include > #include > +#include > #include > #include > #include > > -/* State of each CPU. */ > -static DEFINE_PER_CPU(int, cpu_state) = { 0 }; > - > /* The messaging code jumps to this pointer during boot-up */ > unsigned long start_cpu_function_addr; > > diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h > index 4564c8e..2d0b239 100644 > --- a/arch/x86/include/asm/cpu.h > +++ b/arch/x86/include/asm/cpu.h > @@ -30,8 +30,6 @@ extern int arch_register_cpu(int num); > extern void arch_unregister_cpu(int); > #endif > > -DECLARE_PER_CPU(int, cpu_state); > - > int mwait_usable(const struct cpuinfo_x86 *); > > #endif /* _ASM_X86_CPU_H */ > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c > index bfbe30e..269bc1f 100644 > --- a/arch/x86/kernel/smpboot.c > +++ b/arch/x86/kernel/smpboot.c > @@ -51,6 +51,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -73,9 +74,6 @@ > #include > #include > > -/* State of each CPU */ > -DEFINE_PER_CPU(int, cpu_state) = { 0 }; > - > #ifdef CONFIG_HOTPLUG_CPU > /* > * We need this for trampoline_base protection from concurrent accesses when > diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c > index 2ef5948..09a7199 100644 > --- a/arch/x86/xen/smp.c > +++ b/arch/x86/xen/smp.c > @@ -16,6 +16,7 @@ > #include > #include > #include > +#include > > #include > #include > diff --git a/include/linux/smpboot.h b/include/linux/smpboot.h > index 63bbedd..834d90c 100644 > --- a/include/linux/smpboot.h > +++ b/include/linux/smpboot.h > @@ -5,6 +5,7 @@ > #ifndef SMPBOOT_H > #define SMPBOOT_H > > +DECLARE_PER_CPU(int, cpu_state); > extern void smpboot_start_secondary(void *arg); > > #endif > diff --git a/kernel/smpboot.c b/kernel/smpboot.c > index 5ae1805..0df43b0 100644 > --- a/kernel/smpboot.c > +++ b/kernel/smpboot.c > @@ -67,6 +67,8 @@ void __init idle_threads_init(void) > } > #endif > > +/* State of each CPU during bringup/teardown */ > +DEFINE_PER_CPU(int, cpu_state) = { 0 }; > > /* Implement the following functions in your architecture, as appropriate. */ > > @@ -141,6 +143,8 @@ void __cpuinit smpboot_start_secondary(void *arg) > set_cpu_online(cpu, true); > arch_vector_unlock(); > > + per_cpu(cpu_state, cpu) = CPU_ONLINE; > + > __cpu_post_online(arg); > > /* Enable local interrupts now */ > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/