lkml.org 
[lkml]   [2008]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time

* 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);

\
 
 \ /
  Last update: 2008-10-05 17:23    [W:0.151 / U:0.684 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site