Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time

[Ingo Molnar <mingo@elte.hu>]

* Thomas Gleixner <tglx@linutronix.de> wrote:

> On Sun, 5 Oct 2008, Ingo Molnar wrote:
> > * Chuck Ebbert <cebbert@redhat.com> wrote:
> > 
> > > Yes, it works and I don't see how it could cause any problems.
> > > 
> > > Ingo, can we get this in 2.6.27? You can drop my original patch.
> > > 
> > > Tested-by: Chuck Ebbert <cebbert@redhat.com>
> > 
> > looks good, applied to tip/x86/core, thanks!
> 
> No, this patch is horrible. 
> 
> The correct check is num_present_cpus(). There is no need to make the 
> weird additional_cpus hackery globally available.

ah, indeed!

applied to tip/x86/core and i've zapped Andi's patch.

> Btw, additional_cpus has interesting properties. Providing a negative 
> number < -1 on the kernel command line - happened due to a typo - 
> explodes in early boot, which is not really surprising, but should be 
> sanity checked.

indeed, and that mess was introduced, interestingly, by this commit, 
three years ago, by Andi:

| From 420f8f68c9c5148dddf946bebdbc7eacde2172cb Mon Sep 17 00:00:00 2001
| From: Andi Kleen <ak@suse.de>
| Date: Sat, 5 Nov 2005 17:25:54 +0100
| Subject: [PATCH] [PATCH] x86_64: New heuristics to find out hotpluggable CPUs.

so to clean up the mess i've removed the additional_cpus= boot parameter 
and the Kconfig entry as well - see the patch in x86/core below.

thanks Thomas for decoding this ...

and no way can any of this go into v2.6.27: this is fragile code with a 
lot of historic baggage and the original error is non-fatal to begin 
with. It can easily be backported to .27.1 if testing shows that it has 
no other adverse side-effects.

	Ingo

------------------>
>From a3f493ab543d300b3a01ad18bf12f73746ae5c9f Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Sun, 5 Oct 2008 17:12:36 +0200
Subject: [PATCH] x86: remove additional_cpus configurability

additional_cpus=<x> parameter is dangerous and broken: for example
if we boot additional_cpus=-2 on a stock dual-core system it will
crash the box on bootup.

So reduce the maze of code a bit by removingthe user-configurability
angle.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/Kconfig          |   18 ------------------
 arch/x86/kernel/smpboot.c |    8 +-------
 2 files changed, 1 insertions(+), 25 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7dff081..e2c060e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1365,24 +1365,6 @@ config HOTPLUG_CPU
 	  Say N if you want to disable CPU hotplug and don't need to
 	  suspend.
 
-config HOTPLUG_RESTRICT_TO_BOOTUP_CPUS
-	def_bool n
-	prompt "Restrict CPU hotplugging to processors found during boot" if HOTPLUG_CPU
-	---help---
-	  Say no here to use the default, which allows as many CPUs as are marked
-	  "disabled" by ACPI or MPTABLES to be hotplugged after bootup.
-
-	  Say yes if you do not want to allow CPUs to be added after booting, for
-	  example if you only need CPU hotplugging enabled for suspend/resume.
-
-	  If CPU_HOTPLUG is enabled this value may be overridden at boot time
-	  with the "additional_cpus" kernel parameter.
-
-config HOTPLUG_ADDITIONAL_CPUS
-	int
-	default  0 if !HOTPLUG_CPU || HOTPLUG_RESTRICT_TO_BOOTUP_CPUS
-	default -1
-
 config COMPAT_VDSO
 	def_bool y
 	prompt "Compat VDSO support"
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 9ac428a..3868018 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1260,7 +1260,7 @@ void __init native_smp_cpus_done(unsigned int max_cpus)
 	check_nmi_watchdog();
 }
 
-static int additional_cpus __initdata = CONFIG_HOTPLUG_ADDITIONAL_CPUS;
+static int additional_cpus = -1;
 
 /*
  * cpu_possible_map should be static, it cannot change as cpu's
@@ -1333,12 +1333,6 @@ static void remove_siblinginfo(int cpu)
 	cpu_clear(cpu, cpu_sibling_setup_map);
 }
 
-static __init int setup_additional_cpus(char *s)
-{
-	return s && get_option(&s, &additional_cpus) ? 0 : -EINVAL;
-}
-early_param("additional_cpus", setup_additional_cpus);
-
 static void __ref remove_cpu_from_maps(int cpu)
 {
 	cpu_clear(cpu, cpu_online_map);