Re: [PATCH] Check for online cpus before bringing them up

[Ashok Raj <ashok.raj@intel.com>]

On Fri, Mar 17, 2006 at 12:46:53AM -0800, Srivatsa Vaddagiri wrote:
> 
> 
>    Well  ..other  arch-es need to have a similar check if they get around
>    to
>    implement physical hot-add (even if they allow offlining of all CPUs).
> 

This is really not for physical hotplug, but due to fact i386 startup code wasnt clean
enough. On x86_64 Andi cleaned this up and we dont need those hacks on x86_64.

>    +       if (cpu_online(cpu)) {

Should we add !cpu_present(cpu) check as well just to be consistent with checks 
in cpu_up() ? Probably better if we can move smp_prepare_cpu() to within cpu_up()?

How does the attached patch look.


Check if cpu can be onlined before calling smp_prepare_cpu()

- Moved check for online cpu out of smp_prepare_cpu()
- Moved default declaration of smp_prepare_cpu() to kernel/cpu.c
- Removed lock_cpu_hotplug() from smp_prepare_cpu() to around it, since
  its called from cpu_up() as well now.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
--------------------------------------------------------
 arch/i386/kernel/smpboot.c |    2 --
 drivers/base/cpu.c         |    9 +--------
 kernel/cpu.c               |    9 +++++++++
 kernel/power/smp.c         |    2 ++
 4 files changed, 12 insertions(+), 10 deletions(-)

Index: linux-2.6.16-rc6-mm1/arch/i386/kernel/smpboot.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/arch/i386/kernel/smpboot.c
+++ linux-2.6.16-rc6-mm1/arch/i386/kernel/smpboot.c
@@ -1028,7 +1028,6 @@ int __devinit smp_prepare_cpu(int cpu)
 	struct work_struct task;
 	int	apicid, ret;
 
-	lock_cpu_hotplug();
 	apicid = x86_cpu_to_apicid[cpu];
 	if (apicid == BAD_APICID) {
 		ret = -ENODEV;
@@ -1053,7 +1052,6 @@ int __devinit smp_prepare_cpu(int cpu)
 	zap_low_mappings();
 	ret = 0;
 exit:
-	unlock_cpu_hotplug();
 	return ret;
 }
 #endif
Index: linux-2.6.16-rc6-mm1/drivers/base/cpu.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/drivers/base/cpu.c
+++ linux-2.6.16-rc6-mm1/drivers/base/cpu.c
@@ -19,11 +19,6 @@ EXPORT_SYMBOL(cpu_sysdev_class);
 static struct sys_device *cpu_sys_devices[NR_CPUS];
 
 #ifdef CONFIG_HOTPLUG_CPU
-int __attribute__((weak)) smp_prepare_cpu (int cpu)
-{
-	return 0;
-}
-
 static ssize_t show_online(struct sys_device *dev, char *buf)
 {
 	struct cpu *cpu = container_of(dev, struct cpu, sysdev);
@@ -44,9 +39,7 @@ static ssize_t store_online(struct sys_d
 			kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
 		break;
 	case '1':
-		ret = smp_prepare_cpu(cpu->sysdev.id);
-		if (!ret)
-			ret = cpu_up(cpu->sysdev.id);
+		ret = cpu_up(cpu->sysdev.id);
 		if (!ret)
 			kobject_uevent(&dev->kobj, KOBJ_ONLINE);
 		break;
Index: linux-2.6.16-rc6-mm1/kernel/cpu.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/kernel/cpu.c
+++ linux-2.6.16-rc6-mm1/kernel/cpu.c
@@ -198,6 +198,11 @@ out:
 }
 #endif /*CONFIG_HOTPLUG_CPU*/
 
+int __attribute__((weak)) smp_prepare_cpu (int cpu)
+{
+	return 0;
+}
+
 int __devinit cpu_up(unsigned int cpu)
 {
 	int ret;
@@ -211,6 +216,10 @@ int __devinit cpu_up(unsigned int cpu)
 		goto out;
 	}
 
+	ret = smp_prepare_cpu(cpu);
+	if (ret)
+		goto out;
+
 	ret = notifier_call_chain(&cpu_chain, CPU_UP_PREPARE, hcpu);
 	if (ret == NOTIFY_BAD) {
 		printk("%s: attempt to bring up CPU %u failed\n",
Index: linux-2.6.16-rc6-mm1/kernel/power/smp.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/kernel/power/smp.c
+++ linux-2.6.16-rc6-mm1/kernel/power/smp.c
@@ -49,7 +49,9 @@ void enable_nonboot_cpus(void)
 
 	printk("Thawing cpus ...\n");
 	for_each_cpu_mask(cpu, frozen_cpus) {
+		lock_cpu_hotplug();
 		error = smp_prepare_cpu(cpu);
+		unlock_cpu_hotplug();
 		if (!error)
 			error = cpu_up(cpu);
 		if (!error) {
-
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/