[PATCH] add command line init_start_cpus

[PATCH] add command line init_start_cpus

[Yinghai Lu]

Impact: new command line

so could select cpus to be started during init stage

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 Documentation/kernel-parameters.txt |   14 ++++++++++++++
 init/main.c                         |   29 ++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -918,6 +918,20 @@ and is between 256 and 4096 characters.
 		forcesac
 		soft
 
+	init_start_cpus=	[KNL,SMP] set CPUs that will be started during
+			init stage.
+			Format:
+			<cpu number>,...,<cpu number>
+			or
+			<cpu number>-<cpu number>
+			(must be a positive range in ascending order)
+			or a mixture
+			<cpu number>,...,<cpu number>-<cpu number>
+
+			This option can be used to specify one or more CPUs
+			to be started during init stage.
+			<cpu number> begins at 0 and the maximum value is
+			"number of CPUs in system - 1".
 
 	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
 		off
Index: linux-2.6/init/main.c
===================================================================
--- linux-2.6.orig/init/main.c
+++ linux-2.6/init/main.c
@@ -413,11 +413,35 @@ static void __init setup_per_cpu_areas(v
 }
 #endif /* CONFIG_HAVE_SETUP_PER_CPU_AREA */
 
+static __initdata cpumask_var_t cpu_init_start_mask;
+
+/* Setup the mask of cpus configured for init start */
+static int __init init_start_cpu_setup(char *str)
+{
+	cpulist_parse(str, *cpu_init_start_mask);
+
+	return 1;
+}
+
+__setup("init_start_cpus=", init_start_cpu_setup);
+
 /* Called by boot processor to activate the rest. */
 static void __init smp_init(void)
 {
 	unsigned int cpu;
+	struct cpumask *start_mask;
+
+	if (cpumask_empty(cpu_init_start_mask))
+		start_mask = &cpu_present_map;
+	else {
+		start_mask = kmalloc(cpumask_size(), GFP_KERNEL);
 
+		if (!start_mask)
+			start_mask = &cpu_present_map;
+		else
+			cpumask_and(start_mask, cpu_init_start_mask,
+					 &cpu_present_map);
+	}
 	/*
 	 * Set up the current CPU as possible to migrate to.
 	 * The other ones will be done by cpu_up/cpu_down()
@@ -426,13 +450,16 @@ static void __init smp_init(void)
 	cpu_set(cpu, cpu_active_map);
 
 	/* FIXME: This should be done in userspace --RR */
-	for_each_present_cpu(cpu) {
+	for_each_cpu_mask_nr(cpu, *start_mask) {
 		if (num_online_cpus() >= setup_max_cpus)
 			break;
 		if (!cpu_online(cpu))
 			cpu_up(cpu);
 	}
 
+	if (start_mask != &cpu_present_map)
+		kfree(start_mask);
+
 	/* Any cleanup work */
 	printk(KERN_INFO "Brought up %ld CPUs\n", (long)num_online_cpus());
 	smp_cpus_done(setup_max_cpus);

Re: [PATCH] add command line init_start_cpus

[Andrew Morton]

On Sat, 13 Dec 2008 22:06:49 -0800 Yinghai Lu <yinghai@kernel.org> wrote:

> 
> Impact: new command line
> 
> so could select cpus to be started during init stage
> 

Please always always always provide a description of the *reason* for
making a change to the kernel.  For patches other than bugfixes, this
is the most important part of the patch, because without a reason, why
should we even look at the code changes?

> 
> ---
>  Documentation/kernel-parameters.txt |   14 ++++++++++++++
>  init/main.c                         |   29 ++++++++++++++++++++++++++++-
>  2 files changed, 42 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/Documentation/kernel-parameters.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/kernel-parameters.txt
> +++ linux-2.6/Documentation/kernel-parameters.txt
> @@ -918,6 +918,20 @@ and is between 256 and 4096 characters.
>  		forcesac
>  		soft
>  
> +	init_start_cpus=	[KNL,SMP] set CPUs that will be started during
> +			init stage.
> +			Format:
> +			<cpu number>,...,<cpu number>
> +			or
> +			<cpu number>-<cpu number>
> +			(must be a positive range in ascending order)
> +			or a mixture
> +			<cpu number>,...,<cpu number>-<cpu number>
> +
> +			This option can be used to specify one or more CPUs
> +			to be started during init stage.
> +			<cpu number> begins at 0 and the maximum value is
> +			"number of CPUs in system - 1".

I can't even begin to imagine why would we want this feature.

>  	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
>  		off
> Index: linux-2.6/init/main.c
> ===================================================================
> --- linux-2.6.orig/init/main.c
> +++ linux-2.6/init/main.c

Can we put this all somewhere else? init/main.c is a random dumping ground.

Perhaps kernel/cpu.c would be appropriate.  There's a bunch of other
cpumasky stuff in init/main.c which could also be moved into cpu.c
(and tidied up - it's a godawful ifdef mess).

> @@ -413,11 +413,35 @@ static void __init setup_per_cpu_areas(v
>  }
>  #endif /* CONFIG_HAVE_SETUP_PER_CPU_AREA */
>  
> +static __initdata cpumask_var_t cpu_init_start_mask;
> +
> +/* Setup the mask of cpus configured for init start */
> +static int __init init_start_cpu_setup(char *str)
> +{
> +	cpulist_parse(str, *cpu_init_start_mask);
> +
> +	return 1;
> +}
> +
> +__setup("init_start_cpus=", init_start_cpu_setup);
> +
>  /* Called by boot processor to activate the rest. */
>  static void __init smp_init(void)
>  {
>  	unsigned int cpu;
> +	struct cpumask *start_mask;
> +
> +	if (cpumask_empty(cpu_init_start_mask))
> +		start_mask = &cpu_present_map;
> +	else {
> +		start_mask = kmalloc(cpumask_size(), GFP_KERNEL);

Why is this dynamically allocated?  I'd have though that a simple
__initdata static store would suit?


> +		if (!start_mask)
> +			start_mask = &cpu_present_map;
> +		else
> +			cpumask_and(start_mask, cpu_init_start_mask,
> +					 &cpu_present_map);
> +	}
>  	/*
>  	 * Set up the current CPU as possible to migrate to.
>  	 * The other ones will be done by cpu_up/cpu_down()
> @@ -426,13 +450,16 @@ static void __init smp_init(void)
>  	cpu_set(cpu, cpu_active_map);
>  
>  	/* FIXME: This should be done in userspace --RR */
> -	for_each_present_cpu(cpu) {
> +	for_each_cpu_mask_nr(cpu, *start_mask) {
>  		if (num_online_cpus() >= setup_max_cpus)
>  			break;
>  		if (!cpu_online(cpu))
>  			cpu_up(cpu);
>  	}
>  
> +	if (start_mask != &cpu_present_map)
> +		kfree(start_mask);
> +

[PATCH] add init_start_cpus to config boot cpus -v2

[Yinghai Lu]

Impact: new command line

so could select cpus to be started during init stage.

example:
init_start_cpus=2,4,6
to start core 0 only on every node or thread 0 or all cores

init_start_cpus=0
not start other APs, and later let user to use
 echo 1 > /sys/devices/cpu/cpu2
to start them in user space.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 Documentation/kernel-parameters.txt |   14 ++++++++++++++
 init/main.c                         |   19 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
--- linux-2.6.orig/Documentation/kernel-parameters.txt
+++ linux-2.6/Documentation/kernel-parameters.txt
@@ -918,6 +918,20 @@ and is between 256 and 4096 characters.
 		forcesac
 		soft
 
+	init_start_cpus=	[KNL,SMP] set CPUs that will be started during
+			init stage.
+			Format:
+			<cpu number>,...,<cpu number>
+			or
+			<cpu number>-<cpu number>
+			(must be a positive range in ascending order)
+			or a mixture
+			<cpu number>,...,<cpu number>-<cpu number>
+
+			This option can be used to specify one or more CPUs
+			to be started during init stage.
+			<cpu number> begins at 0 and the maximum value is
+			"number of CPUs in system - 1".
 
 	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
 		off
Index: linux-2.6/init/main.c
===================================================================
--- linux-2.6.orig/init/main.c
+++ linux-2.6/init/main.c
@@ -413,11 +413,28 @@ static void __init setup_per_cpu_areas(v
 }
 #endif /* CONFIG_HAVE_SETUP_PER_CPU_AREA */
 
+static __initdata cpumask_var_t cpu_init_start_mask;
+
+/* Setup the mask of cpus configured for init start */
+static int __init init_start_cpu_setup(char *str)
+{
+	cpulist_parse(str, *cpu_init_start_mask);
+
+	return 1;
+}
+
+__setup("init_start_cpus=", init_start_cpu_setup);
+
 /* Called by boot processor to activate the rest. */
 static void __init smp_init(void)
 {
 	unsigned int cpu;
 
+	if (cpumask_empty(cpu_init_start_mask))
+		cpumask_copy(cpu_init_start_mask, &cpu_present_map);
+	else
+		cpumask_and(cpu_init_start_mask, cpu_init_start_mask,
+					 &cpu_present_map);
 	/*
 	 * Set up the current CPU as possible to migrate to.
 	 * The other ones will be done by cpu_up/cpu_down()
@@ -426,7 +443,7 @@ static void __init smp_init(void)
 	cpu_set(cpu, cpu_active_map);
 
 	/* FIXME: This should be done in userspace --RR */
-	for_each_present_cpu(cpu) {
+	for_each_cpu_mask_nr(cpu, *cpu_init_start_mask) {
 		if (num_online_cpus() >= setup_max_cpus)
 			break;
 		if (!cpu_online(cpu))

Re: [PATCH] add init_start_cpus to config boot cpus -v2

[Andrew Morton]

On Sat, 13 Dec 2008 23:28:36 -0800 Yinghai Lu <yinghai@kernel.org> wrote:

> 
> Impact: new command line
> 
> so could select cpus to be started during init stage.

I already knew that :(


> example:
> init_start_cpus=2,4,6
> to start core 0 only on every node or thread 0 or all cores
> 
> init_start_cpus=0
> not start other APs, and later let user to use
>  echo 1 > /sys/devices/cpu/cpu2
> to start them in user space.
> 

But why?  Why is this useful?  Why did you even bother writing the
code?  Who wants this?  For what reason?  What value has it?

This stuff matters.

> +static __initdata cpumask_var_t cpu_init_start_mask;

OK.

I also suggested that this code not be added in init/main.c.  What
happened to that idea?

Re: [PATCH] add command line init_start_cpus

[Andi Kleen]

Yinghai Lu <yinghai@kernel.org> writes:

> Impact: new command line
>
> so could select cpus to be started during init stage

maxcpus=N should do this already, at least for the first N cpus.
If you really need a non continuous range of N (why?) then it would
be better to fix maxcpus instead of adding a new option.

-Andi

-- 
ak@linux.intel.com

Re: [PATCH] add command line init_start_cpus

[Yinghai Lu]

On Sun, Dec 14, 2008 at 4:11 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Yinghai Lu <yinghai@kernel.org> writes:
>
>> Impact: new command line
>>
>> so could select cpus to be started during init stage
>
> maxcpus=N should do this already, at least for the first N cpus.
> If you really need a non continuous range of N (why?) then it would
> be better to fix maxcpus instead of adding a new option.

it seems it is hard to make maxcpus= to support random cpus selecting and
maxcpus=0 looks insane

also maxcpus=1, will disable smp, can you enable other cpus later in user space?

YH

Re: [PATCH] add command line init_start_cpus

[Ingo Molnar]

* Yinghai Lu <yinghai@kernel.org> wrote:

> On Sun, Dec 14, 2008 at 4:11 PM, Andi Kleen <andi@firstfloor.org> wrote:
> > Yinghai Lu <yinghai@kernel.org> writes:
> >
> >> Impact: new command line
> >>
> >> so could select cpus to be started during init stage
> >
> > maxcpus=N should do this already, at least for the first N cpus.
> > If you really need a non continuous range of N (why?) then it would
> > be better to fix maxcpus instead of adding a new option.
> 
> it seems it is hard to make maxcpus= to support random cpus selecting 
> and maxcpus=0 looks insane

yes, maxcpus=0 looks sucky. If we want this via a boot option then it 
should be a separate one.

> also maxcpus=1, will disable smp, can you enable other cpus later in 
> user space?

only booting maxcpus=0 or nosmp will disable SMP permanently. maxcpus=1 
should give the boot CPU [if not then we need to fix it], and you can then 
enable an arbitrary permutation of good CPUs via:

  echo 1 > /sys/devices/system/cpu/cpuX/online

as long as you CONFIG_HOTPLUG_CPU enabled.

What _might_ make sense is to introduce badcpus=2,4,6 that would 
permanently disnumerate all the broken CPUs, by striking that mask from 
the possible-cpus-map - inaccessible to hotplugging as well. (Note that by 
naming it in the inverse way the thing gets a whole new quality - the boot 
option in itself explains its purpose.)

	Ingo