[PATCH 0/9] BSP or CPU0 online/offline

[PATCH 0/9] BSP or CPU0 online/offline

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

BSP or CPU0 has been the last obstacle to CPU hotplug on x86. This patch set
implements BSP online and offline and removes this obstacle to CPU hotplug.

RAS needs the feature. If socket0 needs to be hotplugged for any reason (any
thread on socket0 is bad, shared cache issue, uncore issue, etc), CPU0 is
required to be offline or hot replaced to keep the system run.

Fenghua Yu (9):
  include/linux/cpu.h: Define architecture dependent cpu map update and
    state check functions
  kernel/cpu.c: Add arch dependent cpu map update functions
  x86/i387.c: Thread xstate is initialized only on BSP once
  x86/common.c: Init BSP data during BSP online
  x86/mtrr/main.c: Ask the first online CPU to save mtrr
  kernel/power/suspend.c,hibernate.c: Don't hibernate/suspend if CPU0
    is offline
  x86/topology.c: Support functions for BSP online/offline
  x86/smpboot.c: Don't offline BSP if any irq can not be migrated out
    of it
  Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0
    online/offline feature

 Documentation/cpu-hotplug.txt       |   19 +++++++++++++++
 Documentation/kernel-parameters.txt |   13 ++++++++++
 arch/x86/include/asm/processor.h    |    1 +
 arch/x86/kernel/cpu/common.c        |   13 ++++++++--
 arch/x86/kernel/cpu/mtrr/main.c     |    9 +++++-
 arch/x86/kernel/i387.c              |    9 ++++++-
 arch/x86/kernel/smpboot.c           |   43 ++++++++++++++++++++++++++++-----
 arch/x86/kernel/topology.c          |   44 +++++++++++++++++++++++++++++-----
 include/linux/cpu.h                 |    6 ++++
 kernel/cpu.c                        |   12 +++++++++
 kernel/power/hibernate.c            |    5 ++++
 kernel/power/main.c                 |    5 ++++
 kernel/power/suspend.c              |    4 +++
 13 files changed, 163 insertions(+), 20 deletions(-)

[PATCH 1/9] include/linux/cpu.h: Define architecture dependent cpu map update and state check functions

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

arch_cpu_maps_update_begin() and arch_cpu_maps_update_done() are architecture
dependent cpu map update functions which are called during cpu_up() and
cpu_down().

arch_state_check() is architecture dependent state check function which is
called during hibernate and suspend.

They are empty __weak functions and are overriden in x86.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 include/linux/cpu.h |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index b1a635a..88a6390 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -137,6 +137,8 @@ int cpu_up(unsigned int cpu);
 void notify_cpu_starting(unsigned int cpu);
 extern void cpu_maps_update_begin(void);
 extern void cpu_maps_update_done(void);
+extern void arch_cpu_maps_update_begin(void);
+extern void arch_cpu_maps_update_done(void);
 
 #else	/* CONFIG_SMP */
 
@@ -207,4 +209,8 @@ static inline int disable_nonboot_cpus(void) { return 0; }
 static inline void enable_nonboot_cpus(void) {}
 #endif /* !CONFIG_PM_SLEEP_SMP */
 
+#ifdef CONFIG_PM
+extern int  arch_state_check(void);
+#endif
+
 #endif /* _LINUX_CPU_H_ */
-- 
1.6.0.3

[PATCH 2/9] kernel/cpu.c: Add arch dependent cpu map update functions

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Because resume from hibernate and suspend always starts from CPU0 on x86 BIOS,
we need to check if CPU0 is online before hibernate or suspend. This causes a
race condition on cpu_online_map.

To cope with the race condition, we add arch_cpu_maps_update_begin() and
arch_cpu_maps_update_done() during cpu_down() and cpu_up(). The functions are
empty on non x86 platforms and are overriden on x86 platforms with real
functions to deal with the race condition.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 kernel/cpu.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/kernel/cpu.c b/kernel/cpu.c
index 12b7458..4d80365 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -20,6 +20,14 @@
 /* Serializes the updates to cpu_online_mask, cpu_present_mask */
 static DEFINE_MUTEX(cpu_add_remove_lock);
 
+void __weak arch_cpu_maps_update_begin(void)
+{
+}
+
+void __weak arch_cpu_maps_update_done(void)
+{
+}
+
 /*
  * The following two API's must be used when attempting
  * to serialize the updates to cpu_online_mask, cpu_present_mask.
@@ -274,6 +282,7 @@ int __ref cpu_down(unsigned int cpu)
 	int err;
 
 	cpu_maps_update_begin();
+	arch_cpu_maps_update_begin();
 
 	if (cpu_hotplug_disabled) {
 		err = -EBUSY;
@@ -284,6 +293,7 @@ int __ref cpu_down(unsigned int cpu)
 
 out:
 	cpu_maps_update_done();
+	arch_cpu_maps_update_done();
 	return err;
 }
 EXPORT_SYMBOL(cpu_down);
@@ -367,6 +377,7 @@ int __cpuinit cpu_up(unsigned int cpu)
 #endif
 
 	cpu_maps_update_begin();
+	arch_cpu_maps_update_begin();
 
 	if (cpu_hotplug_disabled) {
 		err = -EBUSY;
@@ -377,6 +388,7 @@ int __cpuinit cpu_up(unsigned int cpu)
 
 out:
 	cpu_maps_update_done();
+	arch_cpu_maps_update_done();
 	return err;
 }
 
-- 
1.6.0.3

Re: [PATCH 2/9] kernel/cpu.c: Add arch dependent cpu map update functions

[Srivatsa S. Bhat]

On 11/05/2011 03:33 AM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> Because resume from hibernate and suspend always starts from CPU0 on x86 BIOS,
> we need to check if CPU0 is online before hibernate or suspend. This causes a
> race condition on cpu_online_map.
> 
> To cope with the race condition, we add arch_cpu_maps_update_begin() and
> arch_cpu_maps_update_done() during cpu_down() and cpu_up(). The functions are
> empty on non x86 platforms and are overriden on x86 platforms with real
> functions to deal with the race condition.
> 

The race between CPU Hotplug and suspend/hibernate has been taken care of by
my patch here (for another usecase): https://lkml.org/lkml/2011/11/2/487
This is not yet in mainline, but in linux-pm/linux-next.
Please see if this solves your case too. And please CC linux-pm mailing list
(linux-pm@vger.kernel.org) on patches related to power management.

> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  kernel/cpu.c |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 12b7458..4d80365 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -20,6 +20,14 @@
>  /* Serializes the updates to cpu_online_mask, cpu_present_mask */
>  static DEFINE_MUTEX(cpu_add_remove_lock);
> 
> +void __weak arch_cpu_maps_update_begin(void)
> +{
> +}
> +
> +void __weak arch_cpu_maps_update_done(void)
> +{
> +}
> +
>  /*
>   * The following two API's must be used when attempting
>   * to serialize the updates to cpu_online_mask, cpu_present_mask.
> @@ -274,6 +282,7 @@ int __ref cpu_down(unsigned int cpu)
>  	int err;
> 
>  	cpu_maps_update_begin();
> +	arch_cpu_maps_update_begin();
> 
>  	if (cpu_hotplug_disabled) {
>  		err = -EBUSY;
> @@ -284,6 +293,7 @@ int __ref cpu_down(unsigned int cpu)
> 
>  out:
>  	cpu_maps_update_done();
> +	arch_cpu_maps_update_done();
>  	return err;
>  }

See my comments above about whether this is really necessary.
By the way, the locking/unlocking order here seems rather weird to me.
Why have you not chosen to do something like:

cpu_maps_update_begin()
arch_cpu_maps_update_begin()
...
arch_cpu_maps_update_done()
cpu_maps_update_done()


>  EXPORT_SYMBOL(cpu_down);
> @@ -367,6 +377,7 @@ int __cpuinit cpu_up(unsigned int cpu)
>  #endif
> 
>  	cpu_maps_update_begin();
> +	arch_cpu_maps_update_begin();
> 
>  	if (cpu_hotplug_disabled) {
>  		err = -EBUSY;
> @@ -377,6 +388,7 @@ int __cpuinit cpu_up(unsigned int cpu)
> 
>  out:
>  	cpu_maps_update_done();
> +	arch_cpu_maps_update_done();
>  	return err;

Same here. See my comments above.

Thanks,
Srivatsa S. Bhat

[PATCH 3/9] x86/i387.c: Thread xstate is initialized only on BSP once

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

init_thread_xstate() is only called on BSP once to avoid to override
xstate_size.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/i387.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 739d859..f721a61 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -93,6 +93,7 @@ void __cpuinit fpu_init(void)
 {
 	unsigned long cr0;
 	unsigned long cr4_mask = 0;
+	static int once = 1;
 
 	if (cpu_has_fxsr)
 		cr4_mask |= X86_CR4_OSFXSR;
@@ -107,8 +108,14 @@ void __cpuinit fpu_init(void)
 		cr0 |= X86_CR0_EM;
 	write_cr0(cr0);
 
-	if (!smp_processor_id())
+	/*
+	 * init_thread_xstate is only called on BSP once to avoid to override
+	 * xstate_size.
+	 */
+	if (!smp_processor_id() && once) {
+		once = 0;
 		init_thread_xstate();
+	}
 
 	mxcsr_feature_mask_init();
 	/* clean state in init */
-- 
1.6.0.3

[PATCH 4/9] x86/common.c: Init BSP data during BSP online

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

During BSP online, enable x2apic, set_numa_node, add numa mask, and enable
sep cpu for BSP.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/processor.h |    1 +
 arch/x86/kernel/cpu/common.c     |   13 ++++++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index b650435..4057161 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -162,6 +162,7 @@ extern struct pt_regs *idle_regs(struct pt_regs *);
 
 extern void early_cpu_init(void);
 extern void identify_boot_cpu(void);
+extern void identify_boot_cpu_online(void);
 extern void identify_secondary_cpu(struct cpuinfo_x86 *);
 extern void print_cpu_info(struct cpuinfo_x86 *);
 extern void init_scattered_cpuid_features(struct cpuinfo_x86 *c);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index aa003b1..faa3bda 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -916,6 +916,14 @@ void __init identify_boot_cpu(void)
 #endif
 }
 
+void __cpuinit identify_boot_cpu_online(void)
+{
+	numa_add_cpu(smp_processor_id());
+#ifdef CONFIG_X86_32
+	enable_sep_cpu();
+#endif
+}
+
 void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c)
 {
 	BUG_ON(c == &boot_cpu_data);
@@ -1163,7 +1171,7 @@ void __cpuinit cpu_init(void)
 	oist = &per_cpu(orig_ist, cpu);
 
 #ifdef CONFIG_NUMA
-	if (cpu != 0 && percpu_read(numa_node) == 0 &&
+	if (percpu_read(numa_node) == 0 &&
 	    early_cpu_to_node(cpu) != NUMA_NO_NODE)
 		set_numa_node(early_cpu_to_node(cpu));
 #endif
@@ -1195,8 +1203,7 @@ void __cpuinit cpu_init(void)
 	barrier();
 
 	x86_configure_nx();
-	if (cpu != 0)
-		enable_x2apic();
+	enable_x2apic();
 
 	/*
 	 * set up and load the per-CPU TSS
-- 
1.6.0.3

[PATCH 5/9] x86/mtrr/main.c: Ask the first online CPU to save mtrr

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Ask the first online CPU to save mtrr instead of asking BSP. BSP could be
offline when mtrr_save_state() is called.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/cpu/mtrr/main.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 6b96110..e4c1a41 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -695,11 +695,16 @@ void mtrr_ap_init(void)
 }
 
 /**
- * Save current fixed-range MTRR state of the BSP
+ * Save current fixed-range MTRR state of the first cpu in cpu_online_mask.
  */
 void mtrr_save_state(void)
 {
-	smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1);
+	int first_cpu;
+
+	get_online_cpus();
+	first_cpu = cpumask_first(cpu_online_mask);
+	smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
+	put_online_cpus();
 }
 
 void set_mtrr_aps_delayed_init(void)
-- 
1.6.0.3

[PATCH 6/9] kernel/power/suspend.c,hibernate.c: Don't hibernate/suspend if CPU0 is offline

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Because x86 BIOS requires CPU0 to resume from sleep, suspend/resume or hibernate
can't be executed if CPU0 is detected offline.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 kernel/power/hibernate.c |    5 +++++
 kernel/power/main.c      |    5 +++++
 kernel/power/suspend.c   |    4 ++++
 3 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 1c53f7f..b9d1a37 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -612,6 +612,11 @@ int hibernate(void)
 	int error;
 
 	mutex_lock(&pm_mutex);
+
+	error = arch_state_check();
+	if (error)
+		goto Unlock;
+
 	/* The snapshot device should not be opened while we're running */
 	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
 		error = -EBUSY;
diff --git a/kernel/power/main.c b/kernel/power/main.c
index a52e884..863d05d 100644
--- a/kernel/power/main.c
+++ b/kernel/power/main.c
@@ -447,4 +447,9 @@ static int __init pm_init(void)
 	return sysfs_create_group(power_kobj, &attr_group);
 }
 
+int __weak arch_state_check(void)
+{
+	return 0;
+}
+
 core_initcall(pm_init);
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index fdd4263..f86ff6b 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -284,6 +284,10 @@ int enter_state(suspend_state_t state)
 	if (!mutex_trylock(&pm_mutex))
 		return -EBUSY;
 
+	error = arch_state_check();
+	if (error)
+		goto Unlock;
+
 	printk(KERN_INFO "PM: Syncing filesystems ... ");
 	sys_sync();
 	printk("done.\n");
-- 
1.6.0.3

[PATCH 7/9] x86/topology.c: Support functions for BSP online/offline

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

By default, BSP can't be hotpluggable because bsp_hotpluggable is 0. Kernel
parameter bsp_hotplug can enable BSP hotplug feature.

arch_cpu_maps_update_begin() and arch_cpu_maps_update_done() locks/unlocks
pm_mutex. This solves cpu maps race condition between BSP online check during
hibernate/suspend and BSP online/offline operations.

arch_state_check() checks if BSP is still the first online CPU.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/topology.c |   44 +++++++++++++++++++++++++++++++++++++-------
 1 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 8927486..287d7b6 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -29,23 +29,53 @@
 #include <linux/mmzone.h>
 #include <linux/init.h>
 #include <linux/smp.h>
+#include <linux/suspend.h>
 #include <asm/cpu.h>
 
 static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
 
 #ifdef CONFIG_HOTPLUG_CPU
+
+static int bsp_hotpluggable;
+
+static int __init enable_bsp_hotplug(char *str)
+{
+	bsp_hotpluggable = 1;
+	return 0;
+}
+
+__setup("bsp_hotplug", enable_bsp_hotplug);
+
+void arch_cpu_maps_update_begin(void)
+{
+	lock_system_sleep();
+}
+
+void arch_cpu_maps_update_done(void)
+{
+	unlock_system_sleep();
+}
+
+int arch_state_check(void)
+{
+	if (cpumask_first(cpu_online_mask) != 0) {
+		printk(KERN_WARNING "No CPU0.\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 int __ref arch_register_cpu(int num)
 {
 	/*
-	 * CPU0 cannot be offlined due to several
-	 * restrictions and assumptions in kernel. This basically
-	 * doesn't add a control file, one cannot attempt to offline
-	 * BSP.
+	 * Suspend/resume depends on BSP. PIC interrupts depend on BSP.
 	 *
-	 * Also certain PCI quirks require not to enable hotplug control
-	 * for all CPU's.
+	 * If the BSP depencies are under control, tell kernel to
+	 * enable BSP hotplug. This basically adds a control file and
+	 * one can attempt to offline BSP.
 	 */
-	if (num)
+	if (num || bsp_hotpluggable)
 		per_cpu(cpu_devices, num).cpu.hotpluggable = 1;
 
 	return register_cpu(&per_cpu(cpu_devices, num).cpu, num);
-- 
1.6.0.3

Re: [PATCH 7/9] x86/topology.c: Support functions for BSP online/offline

[Srivatsa S. Bhat]

On 11/05/2011 03:33 AM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> By default, BSP can't be hotpluggable because bsp_hotpluggable is 0. Kernel
> parameter bsp_hotplug can enable BSP hotplug feature.
> 
> arch_cpu_maps_update_begin() and arch_cpu_maps_update_done() locks/unlocks
> pm_mutex. This solves cpu maps race condition between BSP online check during
> hibernate/suspend and BSP online/offline operations.
> 
> arch_state_check() checks if BSP is still the first online CPU.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/kernel/topology.c |   44 +++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index 8927486..287d7b6 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -29,23 +29,53 @@
>  #include <linux/mmzone.h>
>  #include <linux/init.h>
>  #include <linux/smp.h>
> +#include <linux/suspend.h>
>  #include <asm/cpu.h>
> 
>  static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
> 
>  #ifdef CONFIG_HOTPLUG_CPU
> +
> +static int bsp_hotpluggable;
> +
> +static int __init enable_bsp_hotplug(char *str)
> +{
> +	bsp_hotpluggable = 1;
> +	return 0;
> +}
> +
> +__setup("bsp_hotplug", enable_bsp_hotplug);
> +
> +void arch_cpu_maps_update_begin(void)
> +{
> +	lock_system_sleep();
> +}
> +
> +void arch_cpu_maps_update_done(void)
> +{
> +	unlock_system_sleep();
> +}
> +
> +int arch_state_check(void)
> +{
> +	if (cpumask_first(cpu_online_mask) != 0) {
> +		printk(KERN_WARNING "No CPU0.\n");

How about a better warning, perhaps like "CPU0 is offline"
or something like that?

> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
>  int __ref arch_register_cpu(int num)
>  {
>  	/*
> -	 * CPU0 cannot be offlined due to several
> -	 * restrictions and assumptions in kernel. This basically
> -	 * doesn't add a control file, one cannot attempt to offline
> -	 * BSP.
> +	 * Suspend/resume depends on BSP. PIC interrupts depend on BSP.
>  	 *
> -	 * Also certain PCI quirks require not to enable hotplug control
> -	 * for all CPU's.
> +	 * If the BSP depencies are under control, tell kernel to
> +	 * enable BSP hotplug. This basically adds a control file and
> +	 * one can attempt to offline BSP.
>  	 */
> -	if (num)
> +	if (num || bsp_hotpluggable)
>  		per_cpu(cpu_devices, num).cpu.hotpluggable = 1;
> 
>  	return register_cpu(&per_cpu(cpu_devices, num).cpu, num);

Thanks,
Srivatsa S. Bhat

Re: [PATCH 7/9] x86/topology.c: Support functions for BSP online/offline

[Srivatsa S. Bhat]

On 11/05/2011 03:33 AM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> By default, BSP can't be hotpluggable because bsp_hotpluggable is 0. Kernel
> parameter bsp_hotplug can enable BSP hotplug feature.
> 
> arch_cpu_maps_update_begin() and arch_cpu_maps_update_done() locks/unlocks
> pm_mutex. This solves cpu maps race condition between BSP online check during
> hibernate/suspend and BSP online/offline operations.
> 
> arch_state_check() checks if BSP is still the first online CPU.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
>  arch/x86/kernel/topology.c |   44 +++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
> index 8927486..287d7b6 100644
> --- a/arch/x86/kernel/topology.c
> +++ b/arch/x86/kernel/topology.c
> @@ -29,23 +29,53 @@
>  #include <linux/mmzone.h>
>  #include <linux/init.h>
>  #include <linux/smp.h>
> +#include <linux/suspend.h>
>  #include <asm/cpu.h>
> 
>  static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);
> 
>  #ifdef CONFIG_HOTPLUG_CPU
> +
> +static int bsp_hotpluggable;
> +
> +static int __init enable_bsp_hotplug(char *str)
> +{
> +	bsp_hotpluggable = 1;
> +	return 0;

Any reason why you return 0 here? Most code I have seen similar to this,
return 1. I understand that anything declared using early_param() would
generate a warning if it returns non-zero, but I am not exactly sure
about how it behaves with __setup(). Kindly give this some thought.

> +}
> +
> +__setup("bsp_hotplug", enable_bsp_hotplug);
> +

Thanks,
Srivatsa S. Bhat

RE: [PATCH 7/9] x86/topology.c: Support functions for BSP online/offline

[Yu, Fenghua]

> > +static int bsp_hotpluggable;
> > +
> > +static int __init enable_bsp_hotplug(char *str)
> > +{
> > +	bsp_hotpluggable = 1;
> > +	return 0;
> 
> Any reason why you return 0 here? Most code I have seen similar to this,
> return 1. I understand that anything declared using early_param() would
> generate a warning if it returns non-zero, but I am not exactly sure
> about how it behaves with __setup(). Kindly give this some thought.

You're right. This function should return 1 here. The parameter setup function returns 1 for success and 0 for failure. The return value is checked in obsolete_checksetup():
                        } else if (p->setup_func(line + n))
                                return 1;

I will change this return value to 1 in the next version of the patch set.

BTW, some places in the current upstream do return 0 for success and return an error number for failure. Maybe we need to clean up them.

Thanks.

-Fenghua

[PATCH 8/9] x86/smpboot.c: Don't offline BSP if any irq can not be migrated out of it

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Don't offline BSP if any irq can not be migrated out of it.

Call identify_boot_cpu_online() for BSP in smp_callin() and continue to online
BSP in native_cpu_up().

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/kernel/smpboot.c |   43 ++++++++++++++++++++++++++++++++++++-------
 1 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 9f548cb..83838c5 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -136,8 +136,8 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);
 atomic_t init_deasserted;
 
 /*
- * Report back to the Boot Processor.
- * Running on AP.
+ * Report back to the Boot Processor during boot time or to the caller processor
+ * during CPU online.
  */
 static void __cpuinit smp_callin(void)
 {
@@ -224,6 +224,13 @@ static void __cpuinit smp_callin(void)
 	smp_store_cpu_info(cpuid);
 
 	/*
+	 * This function won't run on the BSP during boot time. It run
+	 * on BSP only when BSP is offlined and onlined again.
+	 */
+	if (cpuid == 0)
+		identify_boot_cpu_online();
+
+	/*
 	 * This must be done before setting cpu_online_mask
 	 * or calling notify_cpu_starting.
 	 */
@@ -839,7 +846,7 @@ int __cpuinit native_cpu_up(unsigned int cpu)
 
 	pr_debug("++++++++++++++++++++=_---CPU UP  %u\n", cpu);
 
-	if (apicid == BAD_APICID || apicid == boot_cpu_physical_apicid ||
+	if (apicid == BAD_APICID ||
 	    !physid_isset(apicid, phys_cpu_present_map)) {
 		printk(KERN_ERR "%s: bad cpu %d\n", __func__, cpu);
 		return -EINVAL;
@@ -1283,12 +1290,34 @@ int native_cpu_disable(void)
 	 * Perhaps use cpufreq to drop frequency, but that could go
 	 * into generic code.
 	 *
-	 * We won't take down the boot processor on i386 due to some
+	 * We won't take down the boot processor on i386 if some
 	 * interrupts only being able to be serviced by the BSP.
-	 * Especially so if we're not using an IOAPIC	-zwane
+	 * Especially so if we're not using an IOAPIC
+	 * -original comment from zwane, changed by fyu
 	 */
-	if (cpu == 0)
-		return -EBUSY;
+	if (cpu == 0) {
+		int irq;
+		struct irq_desc *desc;
+		struct irq_data *data;
+		struct irq_chip *chip;
+
+		for_each_irq_desc(irq, desc) {
+			raw_spin_lock(&desc->lock);
+			if (!irq_has_action(irq)) {
+				raw_spin_unlock(&desc->lock);
+				continue;
+			}
+
+			data = irq_desc_get_irq_data(desc);
+			chip = irq_data_get_irq_chip(data);
+			if (!chip->irq_set_affinity) {
+				pr_debug("irq%d can't move out of BSP\n", irq);
+				raw_spin_unlock(&desc->lock);
+				return -EBUSY;
+			}
+			raw_spin_unlock(&desc->lock);
+		}
+	}
 
 	clear_local_APIC();
 
-- 
1.6.0.3

[PATCH 9/9] Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0 online/offline feature

[Fenghua Yu]

From: Fenghua Yu <fenghua.yu@intel.com>

Add x86 CPU0 online/offline feature in the two documentations.

By default BSP is not pluggable. Kernel parameter bsp_hotplug enables BSP
online/offline feature.

The documentations point out two BSP dependencies. First, resume from hibernate
or suspend always starts from BSP. So hibernate and suspend are prevented if BSP
is offline. Another dependency is PIC interrupts always go to BSP.

Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
---
---
 Documentation/cpu-hotplug.txt       |   19 +++++++++++++++++++
 Documentation/kernel-parameters.txt |   13 +++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt
index a20bfd4..ebd41e7 100644
--- a/Documentation/cpu-hotplug.txt
+++ b/Documentation/cpu-hotplug.txt
@@ -207,6 +207,25 @@ by making it not-removable.
 
 In such cases you will also notice that the online file is missing under cpu0.
 
+Q: Is CPU0 removable on X86?
+A: Yes. If given kernel option bsp_hotplug, CPU0 is removable.
+
+But some features may depend on BSP. Known dependencies are:
+1. Hibernate/suspend depends on BSP. Hibernate/suspend will fail if BSP is
+offline and you need to online BSP before hibernate/suspend can continue.
+2. PIC interrupts also depend on BSP. BSP can't be removed if a PIC interrupt
+is detected.
+
+It's said poweroff/reboot may depend on BSP on some machines although I haven't
+seen any poweroff/reboot failure so far after BSP is offline on a few tested
+machines.
+
+Please let me know if you know or see any other dependencies of BSP.
+
+If the dependencies are under your control, you can turn on bsp_hotplug.
+
+--Fenghua Yu
+
 Q: How do i find out if a particular CPU is not removable?
 A: Depending on the implementation, some architectures may show this by the
 absence of the "online" file. This is done if it can be determined ahead of
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index a0c5c5f..b1c7e68 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1842,6 +1842,19 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	nox2apic	[X86-64,APIC] Do not enable x2APIC mode.
 
+	bsp_hotplug	[X86] BSP (aka CPU0) is hotpluggable.
+			Some features may depend on BSP. Known dependencies are
+			1. Suspend/resume depends on BSP. Suspend will fail if
+			BSP is offline and you need to online BSP before
+			suspend/resume.
+			2. PIC interrupts also depend on BSP. BSP can't be
+			removed if a PIC interrupt is detected.
+			It's said poweroff/reboot may depend on BSP on some
+			machines although I haven't seen such issues so far
+			after BSP is offline on a few tested machines.
+			If the dependencies are under your control, you can
+			turn on bsp_hotplug.
+
 	nptcg=		[IA-64] Override max number of concurrent global TLB
 			purges which is reported from either PAL_VM_SUMMARY or
 			SAL PALO.
-- 
1.6.0.3

Re: [PATCH 9/9] Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0 online/offline feature

[Randy Dunlap]

On 11/04/2011 03:04 PM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> Add x86 CPU0 online/offline feature in the two documentations.
> 
> By default BSP is not pluggable. Kernel parameter bsp_hotplug enables BSP
> online/offline feature.
> 
> The documentations point out two BSP dependencies. First, resume from hibernate
> or suspend always starts from BSP. So hibernate and suspend are prevented if BSP
> is offline. Another dependency is PIC interrupts always go to BSP.
> 
> Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
> ---
> ---
>  Documentation/cpu-hotplug.txt       |   19 +++++++++++++++++++
>  Documentation/kernel-parameters.txt |   13 +++++++++++++
>  2 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt
> index a20bfd4..ebd41e7 100644
> --- a/Documentation/cpu-hotplug.txt
> +++ b/Documentation/cpu-hotplug.txt
> @@ -207,6 +207,25 @@ by making it not-removable.
>  
>  In such cases you will also notice that the online file is missing under cpu0.
>  
> +Q: Is CPU0 removable on X86?
> +A: Yes. If given kernel option bsp_hotplug, CPU0 is removable.
> +
> +But some features may depend on BSP. Known dependencies are:

Drop "may".

> +1. Hibernate/suspend depends on BSP. Hibernate/suspend will fail if BSP is
> +offline and you need to online BSP before hibernate/suspend can continue.
> +2. PIC interrupts also depend on BSP. BSP can't be removed if a PIC interrupt
> +is detected.
> +
> +It's said poweroff/reboot may depend on BSP on some machines although I haven't
> +seen any poweroff/reboot failure so far after BSP is offline on a few tested
> +machines.
> +
> +Please let me know if you know or see any other dependencies of BSP.
> +
> +If the dependencies are under your control, you can turn on bsp_hotplug.
> +
> +--Fenghua Yu
> +
>  Q: How do i find out if a particular CPU is not removable?
>  A: Depending on the implementation, some architectures may show this by the
>  absence of the "online" file. This is done if it can be determined ahead of
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index a0c5c5f..b1c7e68 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -1842,6 +1842,19 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  
>  	nox2apic	[X86-64,APIC] Do not enable x2APIC mode.
>  
> +	bsp_hotplug	[X86] BSP (aka CPU0) is hotpluggable.
> +			Some features may depend on BSP. Known dependencies are

Drop "may".  Then add ":" after "are".  I.e.,

			Some features depend on BSP.  Known dependencies are:


> +			1. Suspend/resume depends on BSP. Suspend will fail if
> +			BSP is offline and you need to online BSP before
> +			suspend/resume.
> +			2. PIC interrupts also depend on BSP. BSP can't be
> +			removed if a PIC interrupt is detected.
> +			It's said poweroff/reboot may depend on BSP on some
> +			machines although I haven't seen such issues so far
> +			after BSP is offline on a few tested machines.
> +			If the dependencies are under your control, you can
> +			turn on bsp_hotplug.
> +
>  	nptcg=		[IA-64] Override max number of concurrent global TLB
>  			purges which is reported from either PAL_VM_SUMMARY or
>  			SAL PALO.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH 0/9] BSP or CPU0 online/offline

[Srivatsa S. Bhat]

On 11/05/2011 03:33 AM, Fenghua Yu wrote:
> From: Fenghua Yu <fenghua.yu@intel.com>
> 
> BSP or CPU0 has been the last obstacle to CPU hotplug on x86. This patch set
> implements BSP online and offline and removes this obstacle to CPU hotplug.
> 
> RAS needs the feature. If socket0 needs to be hotplugged for any reason (any
> thread on socket0 is bad, shared cache issue, uncore issue, etc), CPU0 is
> required to be offline or hot replaced to keep the system run.
> 
> Fenghua Yu (9):
>   include/linux/cpu.h: Define architecture dependent cpu map update and
>     state check functions
>   kernel/cpu.c: Add arch dependent cpu map update functions
>   x86/i387.c: Thread xstate is initialized only on BSP once
>   x86/common.c: Init BSP data during BSP online
>   x86/mtrr/main.c: Ask the first online CPU to save mtrr
>   kernel/power/suspend.c,hibernate.c: Don't hibernate/suspend if CPU0
>     is offline
>   x86/topology.c: Support functions for BSP online/offline
>   x86/smpboot.c: Don't offline BSP if any irq can not be migrated out
>     of it
>   Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0
>     online/offline feature
> 

The norm usually followed is to update the version number of the patch
or patchset at each iteration and mention the significant changes between
the versions in the changelog/cover-letter (and possibly give links to older
versions). This would make reviewing patches much more easier.

>  Documentation/cpu-hotplug.txt       |   19 +++++++++++++++
>  Documentation/kernel-parameters.txt |   13 ++++++++++
>  arch/x86/include/asm/processor.h    |    1 +
>  arch/x86/kernel/cpu/common.c        |   13 ++++++++--
>  arch/x86/kernel/cpu/mtrr/main.c     |    9 +++++-
>  arch/x86/kernel/i387.c              |    9 ++++++-
>  arch/x86/kernel/smpboot.c           |   43 ++++++++++++++++++++++++++++-----
>  arch/x86/kernel/topology.c          |   44 +++++++++++++++++++++++++++++-----
>  include/linux/cpu.h                 |    6 ++++
>  kernel/cpu.c                        |   12 +++++++++
>  kernel/power/hibernate.c            |    5 ++++
>  kernel/power/main.c                 |    5 ++++
>  kernel/power/suspend.c              |    4 +++
>  13 files changed, 163 insertions(+), 20 deletions(-)
> 

Thanks,
Srivatsa S. Bhat