* [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time @ 2008-10-01 23:19 Chuck Ebbert 2008-10-02 8:12 ` Ingo Molnar 2008-10-02 9:12 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Andi Kleen 0 siblings, 2 replies; 22+ messages in thread From: Chuck Ebbert @ 2008-10-01 23:19 UTC (permalink / raw) To: Ingo Molnar; +Cc: linux-kernel, Arjan van de Ven From: Chuck Ebbert <cebbert@redhat.com> x86: allow number of additional hotplug CPUs to be set at compile time The default number of additional CPU IDs for hotplugging is determined by asking ACPI or mptables how many "disabled" CPUs there are in the system, but many systems get this wrong so that e.g. a uniprocessor machine gets an extra CPU allocated and never switches to single CPU mode. And sometimes CPU hotplugging is enabled only for suspend/hibernate anyway, so the additional CPU IDs are not wanted. Allow the number to be set to zero at compile time. Also, force the number of extra CPUs to zero if hotplugging is disabled which allows removing some conditional code. Tested on uniprocessor x86_64 that ACPI claims has a disabled processor, with CPU hotplugging configured. ("After" has the number of additional CPUs set to 0) Before: NR_CPUS: 512, nr_cpu_ids: 2, nr_node_ids 1 After: NR_CPUS: 512, nr_cpu_ids: 1, nr_node_ids 1 Signed-off-by: Chuck Ebbert <cebbert@redhat.com> --- Index: linux-2.6.26.noarch/arch/x86/Kconfig =================================================================== --- linux-2.6.26.noarch.orig/arch/x86/Kconfig +++ linux-2.6.26.noarch/arch/x86/Kconfig @@ -1366,6 +1366,24 @@ config HOTPLUG_CPU Say N if you want to disable CPU hotplug and don't need to suspend. +config HOTPLUG_DEFAULT_ADDITIONAL_CPUS + def_bool y + prompt "Allocate extra CPUs for hotplugging after boot" if HOTPLUG_CPU + ---help--- + Say yes here to use the default, which allows as many CPUs as are marked + "disabled" by ACPI or MPTABLES to be hotplugged after bootup. + + Say no 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. + + This value may be overridden at boot time with the "additional_cpus" + kernel parameter, if CPU_HOTPLUG is enabled. + +config HOTPLUG_ADDITIONAL_CPUS + int + default 0 if !HOTPLUG_CPU || !HOTPLUG_DEFAULT_ADDITIONAL_CPUS + default -1 + config COMPAT_VDSO def_bool y prompt "Compat VDSO support" Index: linux-2.6.26.noarch/arch/x86/kernel/smpboot.c =================================================================== --- linux-2.6.26.noarch.orig/arch/x86/kernel/smpboot.c +++ linux-2.6.26.noarch/arch/x86/kernel/smpboot.c @@ -1254,7 +1254,7 @@ void __init native_smp_cpus_done(unsigne check_nmi_watchdog(); } -static int additional_cpus __initdata = -1; +static int additional_cpus __initdata = CONFIG_HOTPLUG_ADDITIONAL_CPUS; /* * cpu_possible_map should be static, it cannot change as cpu's @@ -1282,16 +1282,13 @@ __init void prefill_possible_map(void) if (!num_processors) num_processors = 1; -#ifdef CONFIG_HOTPLUG_CPU if (additional_cpus == -1) { if (disabled_cpus > 0) additional_cpus = disabled_cpus; else additional_cpus = 0; } -#else - additional_cpus = 0; -#endif + possible = num_processors + additional_cpus; if (possible > NR_CPUS) possible = NR_CPUS; ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-01 23:19 [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Chuck Ebbert @ 2008-10-02 8:12 ` Ingo Molnar 2008-10-02 19:30 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 Chuck Ebbert 2008-10-02 9:12 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Andi Kleen 1 sibling, 1 reply; 22+ messages in thread From: Ingo Molnar @ 2008-10-02 8:12 UTC (permalink / raw) To: Chuck Ebbert Cc: linux-kernel, Arjan van de Ven, H. Peter Anvin, Thomas Gleixner * Chuck Ebbert <cebbert@redhat.com> wrote: > From: Chuck Ebbert <cebbert@redhat.com> > > x86: allow number of additional hotplug CPUs to be set at compile time > > The default number of additional CPU IDs for hotplugging is determined > by asking ACPI or mptables how many "disabled" CPUs there are in the > system, but many systems get this wrong so that e.g. a uniprocessor > machine gets an extra CPU allocated and never switches to single CPU > mode. > > And sometimes CPU hotplugging is enabled only for suspend/hibernate > anyway, so the additional CPU IDs are not wanted. Allow the number to > be set to zero at compile time. > > Also, force the number of extra CPUs to zero if hotplugging is > disabled which allows removing some conditional code. > > Tested on uniprocessor x86_64 that ACPI claims has a disabled > processor, with CPU hotplugging configured. > > ("After" has the number of additional CPUs set to 0) > Before: NR_CPUS: 512, nr_cpu_ids: 2, nr_node_ids 1 > After: NR_CPUS: 512, nr_cpu_ids: 1, nr_node_ids 1 > > Signed-off-by: Chuck Ebbert <cebbert@redhat.com> hm, wouldnt this option kill 'real' hot-plug CPUs (how rare they might be) which are properly enumerated in the BIOS tables? i dont mind having a facility to disable real CPU hotplug, but the CONFIG_HOTPLUG_DEFAULT_ADDITIONAL_CPUS does not spell that out clearly IMO. Something like CONFIG_HOTPLUG_RESTRICT_TO_BOOTUP_CPUS=y would be more appropriately named i think? Ingo ^ permalink raw reply [flat|nested] 22+ messages in thread
* [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 2008-10-02 8:12 ` Ingo Molnar @ 2008-10-02 19:30 ` Chuck Ebbert 2008-10-02 19:42 ` Ingo Molnar 0 siblings, 1 reply; 22+ messages in thread From: Chuck Ebbert @ 2008-10-02 19:30 UTC (permalink / raw) To: Ingo Molnar Cc: linux-kernel, Arjan van de Ven, H. Peter Anvin, Thomas Gleixner From: Chuck Ebbert <cebbert@redhat.com> x86: allow number of additional hotplug CPUs to be set at compile time, V2 The default number of additional CPU IDs for hotplugging is determined by asking ACPI or mptables how many "disabled" CPUs there are in the system, but many systems get this wrong so that e.g. a uniprocessor machine gets an extra CPU allocated and never switches to single CPU mode. And sometimes CPU hotplugging is enabled only for suspend/hibernate anyway, so the additional CPU IDs are not wanted. Allow the number to be set to zero at compile time. Also, force the number of extra CPUs to zero if hotplugging is disabled which allows removing some conditional code. Tested on uniprocessor x86_64 that ACPI claims has a disabled processor, with CPU hotplugging configured. ("After" has the number of additional CPUs set to 0) Before: NR_CPUS: 512, nr_cpu_ids: 2, nr_node_ids 1 After: NR_CPUS: 512, nr_cpu_ids: 1, nr_node_ids 1 [Changed the name of the option and the prompt according to Ingo's suggestion.] --- Index: linux-2.6.26.noarch/arch/x86/Kconfig =================================================================== --- linux-2.6.26.noarch.orig/arch/x86/Kconfig +++ linux-2.6.26.noarch/arch/x86/Kconfig @@ -1366,6 +1366,24 @@ 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" Index: linux-2.6.26.noarch/arch/x86/kernel/smpboot.c =================================================================== --- linux-2.6.26.noarch.orig/arch/x86/kernel/smpboot.c +++ linux-2.6.26.noarch/arch/x86/kernel/smpboot.c @@ -1254,7 +1254,7 @@ void __init native_smp_cpus_done(unsigne check_nmi_watchdog(); } -static int additional_cpus __initdata = -1; +static int additional_cpus __initdata = CONFIG_HOTPLUG_ADDITIONAL_CPUS; /* * cpu_possible_map should be static, it cannot change as cpu's @@ -1282,16 +1282,13 @@ __init void prefill_possible_map(void) if (!num_processors) num_processors = 1; -#ifdef CONFIG_HOTPLUG_CPU if (additional_cpus == -1) { if (disabled_cpus > 0) additional_cpus = disabled_cpus; else additional_cpus = 0; } -#else - additional_cpus = 0; -#endif + possible = num_processors + additional_cpus; if (possible > NR_CPUS) possible = NR_CPUS; ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 2008-10-02 19:30 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 Chuck Ebbert @ 2008-10-02 19:42 ` Ingo Molnar 2008-10-02 19:48 ` H. Peter Anvin 0 siblings, 1 reply; 22+ messages in thread From: Ingo Molnar @ 2008-10-02 19:42 UTC (permalink / raw) To: Chuck Ebbert Cc: linux-kernel, Arjan van de Ven, H. Peter Anvin, Thomas Gleixner, Li Zefan * Chuck Ebbert <cebbert@redhat.com> wrote: > From: Chuck Ebbert <cebbert@redhat.com> > > x86: allow number of additional hotplug CPUs to be set at compile time, V2 > > The default number of additional CPU IDs for hotplugging is determined > by asking ACPI or mptables how many "disabled" CPUs there are in the > system, but many systems get this wrong so that e.g. a uniprocessor > machine gets an extra CPU allocated and never switches to single CPU > mode. > > And sometimes CPU hotplugging is enabled only for suspend/hibernate > anyway, so the additional CPU IDs are not wanted. Allow the number to > be set to zero at compile time. > > Also, force the number of extra CPUs to zero if hotplugging is disabled > which allows removing some conditional code. > > Tested on uniprocessor x86_64 that ACPI claims has a disabled processor, > with CPU hotplugging configured. > > ("After" has the number of additional CPUs set to 0) > Before: NR_CPUS: 512, nr_cpu_ids: 2, nr_node_ids 1 > After: NR_CPUS: 512, nr_cpu_ids: 1, nr_node_ids 1 > > [Changed the name of the option and the prompt according to Ingo's > suggestion.] > +config HOTPLUG_RESTRICT_TO_BOOTUP_CPUS > + def_bool n > + prompt "Restrict CPU hotplugging to processors found during boot" if HOTPLUG_CPU ok, that description and naming makes the purpose much clearer, and it's default-disabled as well. Applied to tip/x86/core, thanks Chuck! note that this chunk: > @@ -1282,16 +1282,13 @@ __init void prefill_possible_map(void) > if (!num_processors) > num_processors = 1; > > -#ifdef CONFIG_HOTPLUG_CPU > if (additional_cpus == -1) { > if (disabled_cpus > 0) > additional_cpus = disabled_cpus; > else > additional_cpus = 0; > } > -#else > - additional_cpus = 0; > -#endif > + was already in -tip, by virtue of: | commit 2bd455dbfebfd632a8dcf1d3d1612737986fde0a | Author: Li Zefan <lizf@cn.fujitsu.com> | Date: Mon Aug 4 11:26:38 2008 +0800 | | x86: remove nesting CONFIG_HOTPLUG_CPU please double-check latest tip/master nevertheless: http://people.redhat.com/mingo/tip.git/README to make sure i merged your patch correctly and that it plays well with other changes. Thanks, Ingo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 2008-10-02 19:42 ` Ingo Molnar @ 2008-10-02 19:48 ` H. Peter Anvin 2008-10-02 19:50 ` Ingo Molnar 0 siblings, 1 reply; 22+ messages in thread From: H. Peter Anvin @ 2008-10-02 19:48 UTC (permalink / raw) To: Ingo Molnar Cc: Chuck Ebbert, linux-kernel, Arjan van de Ven, Thomas Gleixner, Li Zefan Ingo Molnar wrote: >> >> The default number of additional CPU IDs for hotplugging is determined >> by asking ACPI or mptables how many "disabled" CPUs there are in the >> system, but many systems get this wrong so that e.g. a uniprocessor >> machine gets an extra CPU allocated and never switches to single CPU >> mode. >> >> And sometimes CPU hotplugging is enabled only for suspend/hibernate >> anyway, so the additional CPU IDs are not wanted. Allow the number to >> be set to zero at compile time. >> Wouldn't this be better to have a runtime option? -hpa ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 2008-10-02 19:48 ` H. Peter Anvin @ 2008-10-02 19:50 ` Ingo Molnar 0 siblings, 0 replies; 22+ messages in thread From: Ingo Molnar @ 2008-10-02 19:50 UTC (permalink / raw) To: H. Peter Anvin Cc: Chuck Ebbert, linux-kernel, Arjan van de Ven, Thomas Gleixner, Li Zefan * H. Peter Anvin <hpa@zytor.com> wrote: > Ingo Molnar wrote: >>> >>> The default number of additional CPU IDs for hotplugging is >>> determined by asking ACPI or mptables how many "disabled" CPUs there >>> are in the system, but many systems get this wrong so that e.g. a >>> uniprocessor machine gets an extra CPU allocated and never switches >>> to single CPU mode. >>> >>> And sometimes CPU hotplugging is enabled only for suspend/hibernate >>> anyway, so the additional CPU IDs are not wanted. Allow the number to >>> be set to zero at compile time. >>> > > Wouldn't this be better to have a runtime option? yeah - and we already have the additional_cpus=x boot option, but a boot option is not generally useful to a distribution. Ingo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-01 23:19 [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Chuck Ebbert 2008-10-02 8:12 ` Ingo Molnar @ 2008-10-02 9:12 ` Andi Kleen 2008-10-02 19:25 ` Chuck Ebbert 1 sibling, 1 reply; 22+ messages in thread From: Andi Kleen @ 2008-10-02 9:12 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Ingo Molnar, linux-kernel, Arjan van de Ven Chuck Ebbert <cebbert@redhat.com> writes: > The default number of additional CPU IDs for hotplugging is determined > by asking ACPI or mptables how many "disabled" CPUs there are in the > system, but many systems get this wrong so that e.g. a uniprocessor > machine gets an extra CPU allocated and never switches to single CPU > mode. You can set this with additional_cpus=... at boot time. I don't think each runtime option needs a CONFIG option too. -Andi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-02 9:12 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Andi Kleen @ 2008-10-02 19:25 ` Chuck Ebbert 2008-10-02 19:44 ` Andi Kleen 0 siblings, 1 reply; 22+ messages in thread From: Chuck Ebbert @ 2008-10-02 19:25 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, linux-kernel, Arjan van de Ven On Thu, 02 Oct 2008 11:12:51 +0200 Andi Kleen <andi@firstfloor.org> wrote: > Chuck Ebbert <cebbert@redhat.com> writes: > > > The default number of additional CPU IDs for hotplugging is determined > > by asking ACPI or mptables how many "disabled" CPUs there are in the > > system, but many systems get this wrong so that e.g. a uniprocessor > > machine gets an extra CPU allocated and never switches to single CPU > > mode. > > You can set this with additional_cpus=... at boot time. > I don't think each runtime option needs a CONFIG option too. > Well not all of them, but this one is a good candidate. Either that or we should just change the default to zero. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-02 19:25 ` Chuck Ebbert @ 2008-10-02 19:44 ` Andi Kleen 2008-10-02 20:09 ` Chuck Ebbert 0 siblings, 1 reply; 22+ messages in thread From: Andi Kleen @ 2008-10-02 19:44 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Andi Kleen, Ingo Molnar, linux-kernel, Arjan van de Ven On Thu, Oct 02, 2008 at 03:25:21PM -0400, Chuck Ebbert wrote: > On Thu, 02 Oct 2008 11:12:51 +0200 > Andi Kleen <andi@firstfloor.org> wrote: > > > Chuck Ebbert <cebbert@redhat.com> writes: > > > > > The default number of additional CPU IDs for hotplugging is determined > > > by asking ACPI or mptables how many "disabled" CPUs there are in the > > > system, but many systems get this wrong so that e.g. a uniprocessor > > > machine gets an extra CPU allocated and never switches to single CPU > > > mode. What do you mean with single cpu mode? e.g. the lock prefix rewriting is only determined by online CPUs, not possible CPUs. And this only affects the possible ones. I'm not aware of any other special mode with num_possible_cpus() == 1, only for num_online_cpus() == 1 > > > > You can set this with additional_cpus=... at boot time. > > I don't think each runtime option needs a CONFIG option too. > > > > Well not all of them, but this one is a good candidate. Either that or > we should just change the default to zero. An extra possible CPU is not that costly. A 64bit kernel with my old defconfig has about 40k of per CPU data which would be duplicated. And having real hotplug always require that option would be nasty. What you could probably do if you really worry about the 40k is to do some special casing, as if you know there is only a single socket (can be determined from the APIC IDs in the ACPI tables) and the CPU is only single core then don't allocate it. For HT it is harder, there is no real way to distingush P4 Celerons with HT fused off. But frankly having such special code just for 40k savings (or likely much less on 32bit) seems a little overkill to me. -Andi -- ak@linux.intel.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-02 19:44 ` Andi Kleen @ 2008-10-02 20:09 ` Chuck Ebbert 2008-10-02 20:40 ` Andi Kleen 0 siblings, 1 reply; 22+ messages in thread From: Chuck Ebbert @ 2008-10-02 20:09 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, linux-kernel, Arjan van de Ven On Thu, 2 Oct 2008 21:44:09 +0200 Andi Kleen <andi@firstfloor.org> wrote: > On Thu, Oct 02, 2008 at 03:25:21PM -0400, Chuck Ebbert wrote: > > On Thu, 02 Oct 2008 11:12:51 +0200 > > Andi Kleen <andi@firstfloor.org> wrote: > > > > > Chuck Ebbert <cebbert@redhat.com> writes: > > > > > > > The default number of additional CPU IDs for hotplugging is determined > > > > by asking ACPI or mptables how many "disabled" CPUs there are in the > > > > system, but many systems get this wrong so that e.g. a uniprocessor > > > > machine gets an extra CPU allocated and never switches to single CPU > > > > mode. > > What do you mean with single cpu mode? > > e.g. the lock prefix rewriting is only determined by online CPUs, > not possible CPUs. And this only affects the possible ones. > The prefix rewriting doesn't happen unless I boot with additional_cpus=0, maxcpus=1, or with this patch applied and the config option set. I think the rules for when/if the rewriting happens changed a while ago to avoid multiple switches and now it's not happening at all on this machine by default. Oh, and with NR_CPUS=512 I am seeing 1.6MB per-cpu data (I'll have to check that, but I remember being surprised at how big the number was.) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-02 20:09 ` Chuck Ebbert @ 2008-10-02 20:40 ` Andi Kleen 2008-10-04 16:52 ` <PING> " Andi Kleen 0 siblings, 1 reply; 22+ messages in thread From: Andi Kleen @ 2008-10-02 20:40 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Andi Kleen, Ingo Molnar, linux-kernel, Arjan van de Ven > The prefix rewriting doesn't happen unless I boot with additional_cpus=0, > maxcpus=1, or with this patch applied and the config option set. I think > the rules for when/if the rewriting happens changed a while ago to avoid > multiple switches and now it's not happening at all on this machine by > default. Well then something is broken, but the fix is not to lower num_possible_cpus(), but to fix the root cause. Does the appended patch help? > Oh, and with NR_CPUS=512 I am seeing 1.6MB per-cpu data (I'll have to > check that, but I remember being surprised at how big the number was.) How did you measure? And you mean total right? If it's total then it's ~32KB/CPU which is roughly similar to my 40k number (probably depends on CONFIG options) The standard way is __per_cpu_start - __per_cpu_end (on 64bit; or the other way round on 32bit). That segment is duplicated per CPU. Ok there are some dynamic data structures that scale too, so it's possible a little more. Single per cpu data should not scale with NR_CPUS, but be constant. -Andi --- Take disabled cpus into account in alternative.c Otherwise an UP system with one hotplug CPU will not go into UP mode. Signed-off-by: Andi Kleen <ak@linux.intel.com> Index: linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c =================================================================== --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/alternative.c +++ linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c @@ -454,7 +454,7 @@ void __init alternative_instructions(voi _text, _etext); /* Only switch to UP mode if we don't immediately boot others */ - if (num_possible_cpus() == 1 || setup_max_cpus <= 1) + if (num_possible_cpus() - additional_cpus == 1 || setup_max_cpus <= 1) alternatives_smp_switch(0); } #endif Index: linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c =================================================================== --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/smpboot.c +++ linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c @@ -1276,7 +1276,7 @@ static void remove_siblinginfo(int cpu) cpu_clear(cpu, cpu_sibling_setup_map); } -static int additional_cpus __initdata = -1; +int additional_cpus = -1; static __init int setup_additional_cpus(char *s) { Index: linux-2.6.27-rc4-misc/include/asm-x86/smp.h =================================================================== --- linux-2.6.27-rc4-misc.orig/include/asm-x86/smp.h +++ linux-2.6.27-rc4-misc/include/asm-x86/smp.h @@ -204,5 +204,7 @@ static inline int hard_smp_processor_id( extern void cpu_uninit(void); #endif +extern int additional_cpus; + #endif /* __ASSEMBLY__ */ #endif -- ak@linux.intel.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-02 20:40 ` Andi Kleen @ 2008-10-04 16:52 ` Andi Kleen 2008-10-04 22:30 ` Chuck Ebbert 0 siblings, 1 reply; 22+ messages in thread From: Andi Kleen @ 2008-10-04 16:52 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Ingo Molnar, linux-kernel, Arjan van de Ven Andi Kleen <andi@firstfloor.org> writes: >> The prefix rewriting doesn't happen unless I boot with additional_cpus=0, >> maxcpus=1, or with this patch applied and the config option set. I think >> the rules for when/if the rewriting happens changed a while ago to avoid >> multiple switches and now it's not happening at all on this machine by >> default. > > Well then something is broken, but the fix is not to lower num_possible_cpus(), > but to fix the root cause. > > Does the appended patch help? Ping? Can you please test the patch? I think that's the correct fix. I see Ingo unfortunately merged your initial broken hack, but it's wrong and when actually used on a distribution will break real CPU hotplug there. Please don't enable that CONFIG option in Fedora. Ideally drop the CONFIG patch completely because it cannot do much good. It should be replaced with the appended patch, which should go into 2.6.27 after it is confirmed to fix the problem. Thanks. -Andi > --- > > Take disabled cpus into account in alternative.c > > Otherwise an UP system with one hotplug CPU will not > go into UP mode. > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > Index: linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c > =================================================================== > --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/alternative.c > +++ linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c > @@ -454,7 +454,7 @@ void __init alternative_instructions(voi > _text, _etext); > > /* Only switch to UP mode if we don't immediately boot others */ > - if (num_possible_cpus() == 1 || setup_max_cpus <= 1) > + if (num_possible_cpus() - additional_cpus == 1 || setup_max_cpus <= 1) > alternatives_smp_switch(0); > } > #endif > Index: linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c > =================================================================== > --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/smpboot.c > +++ linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c > @@ -1276,7 +1276,7 @@ static void remove_siblinginfo(int cpu) > cpu_clear(cpu, cpu_sibling_setup_map); > } > > -static int additional_cpus __initdata = -1; > +int additional_cpus = -1; > > static __init int setup_additional_cpus(char *s) > { > Index: linux-2.6.27-rc4-misc/include/asm-x86/smp.h > =================================================================== > --- linux-2.6.27-rc4-misc.orig/include/asm-x86/smp.h > +++ linux-2.6.27-rc4-misc/include/asm-x86/smp.h > @@ -204,5 +204,7 @@ static inline int hard_smp_processor_id( > extern void cpu_uninit(void); > #endif > > +extern int additional_cpus; > + > #endif /* __ASSEMBLY__ */ > #endif > -- > ak@linux.intel.com -- ak@linux.intel.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-04 16:52 ` <PING> " Andi Kleen @ 2008-10-04 22:30 ` Chuck Ebbert 2008-10-05 10:28 ` Ingo Molnar 0 siblings, 1 reply; 22+ messages in thread From: Chuck Ebbert @ 2008-10-04 22:30 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, linux-kernel, Arjan van de Ven On Sat, 04 Oct 2008 18:52:06 +0200 Andi Kleen <andi@firstfloor.org> wrote: > > Ping? Can you please test the patch? I think that's the correct fix. > > I see Ingo unfortunately merged your initial broken hack, but it's wrong > and when actually used on a distribution will break real CPU hotplug there. > Please don't enable that CONFIG option in Fedora. Ideally drop > the CONFIG patch completely because it cannot do much good. > > It should be replaced with the appended patch, which should go into 2.6.27 > after it is confirmed to fix the problem. > 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> > > --- > > > > Take disabled cpus into account in alternative.c > > > > Otherwise an UP system with one hotplug CPU will not > > go into UP mode. > > > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > > > Index: linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c > > =================================================================== > > --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/alternative.c > > +++ linux-2.6.27-rc4-misc/arch/x86/kernel/alternative.c > > @@ -454,7 +454,7 @@ void __init alternative_instructions(voi > > _text, _etext); > > > > /* Only switch to UP mode if we don't immediately boot others */ > > - if (num_possible_cpus() == 1 || setup_max_cpus <= 1) > > + if (num_possible_cpus() - additional_cpus == 1 || setup_max_cpus <= 1) > > alternatives_smp_switch(0); > > } > > #endif > > Index: linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c > > =================================================================== > > --- linux-2.6.27-rc4-misc.orig/arch/x86/kernel/smpboot.c > > +++ linux-2.6.27-rc4-misc/arch/x86/kernel/smpboot.c > > @@ -1276,7 +1276,7 @@ static void remove_siblinginfo(int cpu) > > cpu_clear(cpu, cpu_sibling_setup_map); > > } > > > > -static int additional_cpus __initdata = -1; > > +int additional_cpus = -1; > > > > static __init int setup_additional_cpus(char *s) > > { > > Index: linux-2.6.27-rc4-misc/include/asm-x86/smp.h > > =================================================================== > > --- linux-2.6.27-rc4-misc.orig/include/asm-x86/smp.h > > +++ linux-2.6.27-rc4-misc/include/asm-x86/smp.h > > @@ -204,5 +204,7 @@ static inline int hard_smp_processor_id( > > extern void cpu_uninit(void); > > #endif > > > > +extern int additional_cpus; > > + > > #endif /* __ASSEMBLY__ */ > > #endif > > -- > > ak@linux.intel.com > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-04 22:30 ` Chuck Ebbert @ 2008-10-05 10:28 ` Ingo Molnar 2008-10-05 14:52 ` Thomas Gleixner 0 siblings, 1 reply; 22+ messages in thread From: Ingo Molnar @ 2008-10-05 10:28 UTC (permalink / raw) To: Chuck Ebbert; +Cc: Andi Kleen, linux-kernel, Arjan van de Ven * 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! it's too late for v2.6.27 (and not a serious regression anyway), but i marked it for .27.1 backporting. Your other patch still makes sense - as additional_cpus is an existing boot parameter it can be specified at .config time as well. You should not enable it in a distro kernel though, if you intend to support real hotplug CPU hardware. (which is extremely rare btw.) Ingo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 10:28 ` Ingo Molnar @ 2008-10-05 14:52 ` Thomas Gleixner 2008-10-05 15:20 ` Ingo Molnar 0 siblings, 1 reply; 22+ messages in thread From: Thomas Gleixner @ 2008-10-05 14:52 UTC (permalink / raw) To: Ingo Molnar; +Cc: Chuck Ebbert, Andi Kleen, linux-kernel, Arjan van de Ven 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. 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. Thanks, tglx ----------------> Subject: x86: make UP alternatives switch depend on present cpus From: Thomas Gleixner <tglx@linutronix.de> Date: Sun, 05 Oct 2008 16:45:22 +0200 num_possible_cpus() can be > 1 when disabled CPUs have been accounted. Disabled CPUs are not in the cpu_present_map, so we can use num_present_cpus() as a safe indicator to switch to UP alternatives. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index fb04e49..a84ac7b 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -444,7 +444,7 @@ void __init alternative_instructions(void) _text, _etext); /* Only switch to UP mode if we don't immediately boot others */ - if (num_possible_cpus() == 1 || setup_max_cpus <= 1) + if (num_present_cpus() == 1 || setup_max_cpus <= 1) alternatives_smp_switch(0); } #endif ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 14:52 ` Thomas Gleixner @ 2008-10-05 15:20 ` Ingo Molnar 2008-10-05 15:51 ` Thomas Gleixner 2008-10-05 20:28 ` Andi Kleen 0 siblings, 2 replies; 22+ messages in thread From: Ingo Molnar @ 2008-10-05 15:20 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Chuck Ebbert, Andi Kleen, linux-kernel, Arjan van de Ven * 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); ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 15:20 ` Ingo Molnar @ 2008-10-05 15:51 ` Thomas Gleixner 2008-10-05 15:56 ` Ingo Molnar 2008-10-05 20:39 ` Andi Kleen 2008-10-05 20:28 ` Andi Kleen 1 sibling, 2 replies; 22+ messages in thread From: Thomas Gleixner @ 2008-10-05 15:51 UTC (permalink / raw) To: Ingo Molnar; +Cc: Chuck Ebbert, Andi Kleen, linux-kernel, Arjan van de Ven On Sun, 5 Oct 2008, Ingo Molnar wrote: > > * 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. Please lets get rid of all this. Thanks, tglx ----------------> >From 344707c1f43dd0d080828497aacb60c0cc0a8c13 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner <tglx@linutronix.de> Date: Sun, 5 Oct 2008 17:27:56 +0200 Subject: [PATCH] x86, smpboot: remove additional_cpus remove remainder of additional_cpus logic. We now just listen to the disabled_cpus value like we did for years. disabled_cpus is always >= 0 so no need for an extra check. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- arch/x86/kernel/smpboot.c | 14 ++------------ 1 files changed, 2 insertions(+), 12 deletions(-) diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c index 3868018..d6a4d95 100644 --- a/arch/x86/kernel/smpboot.c +++ b/arch/x86/kernel/smpboot.c @@ -1260,8 +1260,6 @@ void __init native_smp_cpus_done(unsigned int max_cpus) check_nmi_watchdog(); } -static int additional_cpus = -1; - /* * cpu_possible_map should be static, it cannot change as cpu's * are onlined, or offlined. The reason is per-cpu data-structures @@ -1281,21 +1279,13 @@ static int additional_cpus = -1; */ __init void prefill_possible_map(void) { - int i; - int possible; + int i, possible; /* no processor from mptable or madt */ if (!num_processors) num_processors = 1; - if (additional_cpus == -1) { - if (disabled_cpus > 0) - additional_cpus = disabled_cpus; - else - additional_cpus = 0; - } - - possible = num_processors + additional_cpus; + possible = num_processors + disabled_cpus; if (possible > NR_CPUS) possible = NR_CPUS; ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 15:51 ` Thomas Gleixner @ 2008-10-05 15:56 ` Ingo Molnar 2008-10-05 20:39 ` Andi Kleen 1 sibling, 0 replies; 22+ messages in thread From: Ingo Molnar @ 2008-10-05 15:56 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Chuck Ebbert, Andi Kleen, linux-kernel, Arjan van de Ven * Thomas Gleixner <tglx@linutronix.de> wrote: > > 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. > > Please lets get rid of all this. > > Thanks, > > tglx > ----------------> > >From 344707c1f43dd0d080828497aacb60c0cc0a8c13 Mon Sep 17 00:00:00 2001 > From: Thomas Gleixner <tglx@linutronix.de> > Date: Sun, 5 Oct 2008 17:27:56 +0200 > Subject: [PATCH] x86, smpboot: remove additional_cpus > > remove remainder of additional_cpus logic. We now just listen to the > disabled_cpus value like we did for years. disabled_cpus is always >= > 0 so no need for an extra check. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> applied to tip/x86/core - thanks Thomas! Ingo ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 15:51 ` Thomas Gleixner 2008-10-05 15:56 ` Ingo Molnar @ 2008-10-05 20:39 ` Andi Kleen 2008-10-05 21:49 ` Thomas Gleixner 1 sibling, 1 reply; 22+ messages in thread From: Andi Kleen @ 2008-10-05 20:39 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Ingo Molnar, Chuck Ebbert, linux-kernel, Arjan van de Ven > > Please lets get rid of all this. The result is that there's no way to override a BIOS now that doesn't declare the number of hotplug CPUs with the Linux hotplug extension. Completely trusting the BIOS seems like a bad idea. So I think you still need the command line parameter back, otherwise there's no way to override the BIOS. -Andi (who wished you guys would check with the original author before removing code you clearly don't understand) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 20:39 ` Andi Kleen @ 2008-10-05 21:49 ` Thomas Gleixner 2008-10-05 22:45 ` Andi Kleen 0 siblings, 1 reply; 22+ messages in thread From: Thomas Gleixner @ 2008-10-05 21:49 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, Chuck Ebbert, linux-kernel, Arjan van de Ven On Sun, 5 Oct 2008, Andi Kleen wrote: > > > > Please lets get rid of all this. > > The result is that there's no way to override a BIOS now that > doesn't declare the number of hotplug CPUs with the Linux hotplug > extension. > > Completely trusting the BIOS seems like a bad idea. > > So I think you still need the command line parameter back, otherwise there's no > way to override the BIOS. > > -Andi (who wished you guys would check with the original author > before removing code you clearly don't understand) Sigh, could you please start thinking first before you insult me ? Chucks problem is that the BIOS advertises 1 present CPU and 1 disabled CPU Now the current code does not switch to UP alternatives because the check in alternatives.c is: if (num_possible_cpus() == 1 ... which evaluates to false, due to: num_possible_cpus = num_processors + additional_cpus where additional_cpus = disabled_cpus when no command line parameter is given, i.e. the default behaviour of the kernel. and where num_processors = num_present_cpus() so in Chucks case num_possible_cpus() == 2 Your proposed solution is: exporting additional_cpus and change the check to: if ((num_possible_cpus() - additional_cpus) == 1 ... This is simply stupid. We have the information already in num_present_cpus() and that's the correct check in the alternatives code. if (num_present_cpus() == 1 ... This is not a question of trusting the BIOS: If we can not trust present_cpu_map then additional_cpus does not help at all. additional_cpus is useless ballast. Thanks, tglx - who refrains from adding a nasty insulting comment ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 21:49 ` Thomas Gleixner @ 2008-10-05 22:45 ` Andi Kleen 0 siblings, 0 replies; 22+ messages in thread From: Andi Kleen @ 2008-10-05 22:45 UTC (permalink / raw) To: Thomas Gleixner; +Cc: Ingo Molnar, Chuck Ebbert, linux-kernel, Arjan van de Ven > Your proposed solution is: > > exporting additional_cpus and change the check to: > > if ((num_possible_cpus() - additional_cpus) == 1 ... > > This is simply stupid. We have the information already in > num_present_cpus() and that's the correct check in the alternatives > code. Yes that is correct. Using present_cpus is cleaner than my version patch (although it wasn't quite at a "mess" level imho). No argument on that. But that is not what my emails were about. They were about the removal of the additional_cpus=... parameter which was unfortunately done too. additional_cpus is used for more than just alternatives processing. It is also used to size the possible map and declare additional hotplug CPUs. Please read the Documentation cpu-hotplug-spec I referred to earlier for background. Or Documentation/cpu-hotplug.txt which has it too. additional_cpus is used to tell Linux that there are more (or less) CPUs hotpluggable than the BIOS declares at boot time. This is needed because the way the BIOS is declaring it is a Linux extension, not standard. But even if it was standard it would be good policy to be able to override it because as we all know BIOS can be wrong, as in declare far too many. But now that option is gone so this needed override isn't there > additional_cpus is useless ballast. For the alternatives heuristics yes. For hotplug no. Only when you assume that all hotpluggable BIOS implement the "cpu-hotplug-spec" Linux extension and always declare hotplug correctly. That's quite dubious. > tglx - who refrains from adding a nasty insulting comment Feel free to speak your mind if the facts are correct. -Andi ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: <PING> Re: [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time 2008-10-05 15:20 ` Ingo Molnar 2008-10-05 15:51 ` Thomas Gleixner @ 2008-10-05 20:28 ` Andi Kleen 1 sibling, 0 replies; 22+ messages in thread From: Andi Kleen @ 2008-10-05 20:28 UTC (permalink / raw) To: Ingo Molnar; +Cc: Thomas Gleixner, Chuck Ebbert, linux-kernel, Arjan van de Ven > >> 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: What mess do you mean? That not all parameters are 100% sanity checked? "Unix gives you enough rope to shot yourself into the foot." Normally people who set kernel parameters are expected to know what they are doing. That said the parameter should probably use some unsigned form of get_option, but unfortunately there isn't any. But if you describe any kernel parameter that isn't 100% sanity checked as mess I'm afraid there won't be many non messy parameters left. > | 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. This also breaks real CPU hotplug on systems that do not follow the Documentation/x86/x86_64/cpu-hotplug-spec specification. This extension is not part of the standard ACPI spec (I invented it), so I don't think everyone requiring to follow such a Linux specific extension is a good idea. I tried to lobby a few vendors to follow this extension, but I doubt I was 100% successfull. So please readd the boot parameter. Or if you don't chose it at least modify the document clearly stating that all CPU hotplug requires a non ACPI standard extension now and that the users is screwed if their vendor doesn't follow it. But it would be better to keep the parameter as a fallback. > and no way can any of this go into v2.6.27: this is fragile code with a > lot of historic baggage In my experience CPU discovery in ACPI/mptables isn't particularly fragile. alternatives can be, but this patch doesn't affect its basic operation. > and the original error is non-fatal to begin > with. That is true, it's just a performance issue especially on systems with slow LOCK prefix (like P4s) which commonly have the bogus extra CPU in the BIOS tables when they don't support HT directly. -Andi ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-10-05 22:45 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-10-01 23:19 [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Chuck Ebbert 2008-10-02 8:12 ` Ingo Molnar 2008-10-02 19:30 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2 Chuck Ebbert 2008-10-02 19:42 ` Ingo Molnar 2008-10-02 19:48 ` H. Peter Anvin 2008-10-02 19:50 ` Ingo Molnar 2008-10-02 9:12 ` [patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time Andi Kleen 2008-10-02 19:25 ` Chuck Ebbert 2008-10-02 19:44 ` Andi Kleen 2008-10-02 20:09 ` Chuck Ebbert 2008-10-02 20:40 ` Andi Kleen 2008-10-04 16:52 ` <PING> " Andi Kleen 2008-10-04 22:30 ` Chuck Ebbert 2008-10-05 10:28 ` Ingo Molnar 2008-10-05 14:52 ` Thomas Gleixner 2008-10-05 15:20 ` Ingo Molnar 2008-10-05 15:51 ` Thomas Gleixner 2008-10-05 15:56 ` Ingo Molnar 2008-10-05 20:39 ` Andi Kleen 2008-10-05 21:49 ` Thomas Gleixner 2008-10-05 22:45 ` Andi Kleen 2008-10-05 20:28 ` Andi Kleen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).