Skip to contentSkip to conversation

add command line init_start_cpus

PATCH In review Yinghai Lu 4 participants 7 messages Dec 14, 2008 – Dec 16, 2008 Documentation

lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 add command line init_start_cpus

Impact: new command line

so could select cpus to be started during init stage

---
 Documentation/kernel-parameters.txt |   14 ++++++++++++++
 init/main.c                         |   29 ++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)
Documentation/kernel-parameters.txt +14 -0
Index: 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
init/main.c +28 -1
Index: 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);
#1 lore raw
Quoted 5 lines · On Sat, 13 Dec 2008 22:06:49 -0800 Yinghai Lu <[email protected]> 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?

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

Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================

Documentation/kernel-parameters.txt +4 -0
@@ -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>

Documentation/kernel-parameters.txt and is between 256 and 4096 characters. · line 934

Quoted 10 lines
Documentation/kernel-parameters.txt +10 -0
+			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.

init/main.c

Quoted 6 lines
Documentation/kernel-parameters.txt +0 -0
 	intel_iommu=	[DMAR] Intel IOMMU driver (DMAR) option
 		off

Index: linux-2.6/init/main.c
===================================================================

init/main.c +0 -0

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

Quoted 16 lines
init/main.c +12 -0
@@ -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);
+

init/main.c static void __init setup_per_cpu_areas(v · line 437

Quoted 10 lines
init/main.c +6 -0
 /* 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?

Quoted 24 lines
init/main.c +10 -1
+		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);
+
author
#2 lore raw

PATCH 1/1 add init_start_cpus to config boot cpus -v2

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.

---
 Documentation/kernel-parameters.txt |   14 ++++++++++++++
 init/main.c                         |   19 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)
Documentation/kernel-parameters.txt +14 -0
Index: 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
init/main.c +18 -1
Index: 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))
#3 lore raw

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

Quoted 4 lines · On Sat, 13 Dec 2008 23:28:36 -0800 Yinghai Lu <[email protected]> wrote:

Impact: new command line

so could select cpus to be started during init stage.

I already knew that :(

Quoted 9 lines

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.

Quoted 1 line

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

#4 lore raw
In reply to Yinghai Lu
Quoted 3 lines · Yinghai Lu

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

Signature · 1 line
[email protected]
author
#5 lore raw
Quoted 9 lines · Andi Kleen
Quoted 3 lines · Yinghai Lu

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

#6 lore raw
Quoted 5 lines · Yinghai Lu
Quoted 4 lines · Andi Kleen
Quoted 2 lines · Yinghai Lu

Impact: new command line

Quoted 8 lines
Quoted 5 lines
Quoted 1 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.

Quoted 2 lines

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