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

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

[Chuck Ebbert]

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;

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

[Ingo Molnar]

* 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

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

[Andi Kleen]

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

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

[Chuck Ebbert]

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.

[patch x86/core] x86: allow number of additional hotplug CPUs to be set at compile time, V2

[Chuck Ebbert]

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;

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

[Ingo Molnar]

* 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

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

[Andi Kleen]

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

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

[H. Peter Anvin]

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

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

[Ingo Molnar]

* 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

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

[Chuck Ebbert]

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.)

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

[Andi Kleen]

> 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

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

[Andi Kleen]

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

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

[Chuck Ebbert]

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
> 

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

[Ingo Molnar]

* 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

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

[Thomas Gleixner]

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

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

[Ingo Molnar]

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

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

[Thomas Gleixner]

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;
 

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

[Ingo Molnar]

* 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

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

[Andi Kleen]

> 
>> 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

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

[Andi Kleen]

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

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

[Thomas Gleixner]

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

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

[Andi Kleen]

> 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

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

[Andi Kleen]

On Mon, Oct 06, 2008 at 12:59:18PM +0200, Bodo Eggert wrote:
> I don't have a machine with pluggable CPUs, but I'd imagine if you'd take out
> some CPUs, the number of additional CPUs you can plug in will increase by the
> same number (forcing me to change the kernel command lince if I do), while the

It only costs you some memory to have more hotpluggable CPUs (on my
kernel here around 40k/possible CPU), so there's very little "force" in 
practice unless you set excessive values.

Some other architectures invented a "possible_cpus=..." parameter to address
this. I presume this could be done on x86 too, but it doesn't seem
like a very pressing issue. Even if it was done it would be better
to keep additional_cpus for compatibility.

> number of slots (and the number of possible CPUs) stays the same unless
> somebody offers a new kind of CPU card. Therefore, I'd expect "maxcpus=" to be
> the only interface I want for this purpose. Put in a value bigger than the
> amount plugged in -> voila.

maxcpus=... doesn't affect the cpu_possible_mask.  It really is only 
good for limiting CPUs.

> OTOH, looking at Thomas' patch, I'd guess it would not work as expected ...
> and looking at the code seems to confirm this. Besides that, I'd possibly
> want a way to limit the number of online CPUs at boot saying something like
> "onlinecpus=", which would not limit the number of CPUs I can plug in.

That is exactly what maxcpus=... does.

Anyways I don't see the additional_cpus=... removal patch in tip
so hopefully things are fine now

-Andi

-- 
ak@linux.intel.com

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

[Bodo Eggert]

Andi Kleen <andi@firstfloor.org> wrote:

> 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.

I don't have a machine with pluggable CPUs, but I'd imagine if you'd take out
some CPUs, the number of additional CPUs you can plug in will increase by the
same number (forcing me to change the kernel command lince if I do), while the
number of slots (and the number of possible CPUs) stays the same unless
somebody offers a new kind of CPU card. Therefore, I'd expect "maxcpus=" to be
the only interface I want for this purpose. Put in a value bigger than the
amount plugged in -> voila.

OTOH, looking at Thomas' patch, I'd guess it would not work as expected ...
and looking at the code seems to confirm this. Besides that, I'd possibly
want a way to limit the number of online CPUs at boot saying something like
"onlinecpus=", which would not limit the number of CPUs I can plug in.