Skip to contentSkip to conversation

sched: consolidate sched domains]

PATCH In review James Bottomley 5 participants 9 messages Sep 3, 2004 – Sep 6, 2004 1 message not in the archive

lore
Share & tools
lore

James Bottomley

author
#0 lore raw

PATCH 1/1 sched: consolidate sched domains]

Could we get this in please? The current screw up in the scheduling domain patch means that any architecture that actually hotplugs CPUs will crash in find_busiest_group() ... and I notice this has just bitten the z Series people...

James

Quoted 1 line · William Lee Irwin III

Okay, if you prefer the #ifdef:

And for the other half of it:

kernel/sched.c +9 -0
Index: wait-2.6.9-rc1-mm1/kernel/sched.c
===================================================================
@@ -4224,7 +4224,11 @@
 		sd = &per_cpu(phys_domains, i);
 		group = cpu_to_phys_group(i);
 		*sd = SD_CPU_INIT;
+#ifdef CONFIG_NUMA
 		sd->span = nodemask;
+#else
+		sd->span = cpu_possible_map;
+#endif
 		sd->parent = p;
 		sd->groups = &sched_group_phys[group];
 

@@ -4262,6 +4266,7 @@
 						&cpu_to_isolated_group);
 	}
 
+#ifdef CONFIG_NUMA
 	/* Set up physical groups */
 	for (i = 0; i < MAX_NUMNODES; i++) {
 		cpumask_t nodemask = node_to_cpumask(i);

@@ -4273,6 +4278,10 @@
 		init_sched_build_groups(sched_group_phys, nodemask,
 						&cpu_to_phys_group);
 	}
+#else
+	init_sched_build_groups(sched_group_phys, cpu_possible_map,
+							&cpu_to_phys_group);
+#endif
 
 #ifdef CONFIG_NUMA
 	/* Set up node groups */
#1 lore raw
Quoted 5 lines · James Bottomley

Could we get this in please? The current screw up in the scheduling domain patch means that any architecture that actually hotplugs CPUs will crash in find_busiest_group() ... and I notice this has just bitten the z Series people...

Have we yet seen anything which looks like a completed and tested patch?

author
#2 lore raw
Quoted 8 lines · Andrew Morton
Quoted 5 lines · James Bottomley

Could we get this in please? The current screw up in the scheduling domain patch means that any architecture that actually hotplugs CPUs will crash in find_busiest_group() ... and I notice this has just bitten the z Series people...

Have we yet seen anything which looks like a completed and tested patch?

The attached was what I've tested on parisc. It looks complete to me; what's wrong with it?

James

#3 lore raw
In reply to Andrew Morton
Quoted 4 lines · James Bottomley
Quoted 4 lines

Could we get this in please? The current screw up in the scheduling domain patch means that any architecture that actually hotplugs CPUs will crash in find_busiest_group() ... and I notice this has just bitten the z Series people...

Quoted 1 line · Andrew Morton

Have we yet seen anything which looks like a completed and tested patch?

This is the whole thing; the "other half" referred to a new hunk added to the patch (identical to this one) posted in its entirety.

-- wli

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

[sched] fix sched_domains hotplug bootstrap ordering vs. cpu_online_map issue

Quoted 2 lines · William Lee Irwin III
Quoted 2 lines

This is the whole thing; the "other half" referred to a new hunk added to the patch (identical to this one) posted in its entirety.

Quoted 1 line · Andrew Morton

ho-hum. changelog, please?

cpu_online_map is not set up at the time of sched domain initialization when hotplug cpu paths are used for SMP booting. At this phase of bootstrapping, cpu_possible_map can be used by the various architectures using cpu hotplugging for SMP bootstrap, but the manipulations of cpu_online_map done on behalf of NUMA architectures, done indirectly via node_to_cpumask(), can't, because cpu_online_map starts depopulated and hasn't yet been populated. On true NUMA architectures this is a distinct cpumask_t from cpu_online_map and so the unpatched code works on NUMA; on non-NUMA architectures the definition of node_to_cpumask() this way breaks and would require an invasive sweeping of users of node_to_cpumask() to change it to e.g. cpu_possible_map, as cpu_possible_map is not suitable for use at runtime as a substitute for cpu_online_map.

Signed-off-by William Irwin <[email protected]>
kernel/sched.c +9 -0
Index: wait-2.6.9-rc1-mm1/kernel/sched.c
===================================================================
@@ -4224,7 +4224,11 @@
 		sd = &per_cpu(phys_domains, i);
 		group = cpu_to_phys_group(i);
 		*sd = SD_CPU_INIT;
+#ifdef CONFIG_NUMA
 		sd->span = nodemask;
+#else
+		sd->span = cpu_possible_map;
+#endif
 		sd->parent = p;
 		sd->groups = &sched_group_phys[group];
 

@@ -4262,6 +4266,7 @@
 						&cpu_to_isolated_group);
 	}
 
+#ifdef CONFIG_NUMA
 	/* Set up physical groups */
 	for (i = 0; i < MAX_NUMNODES; i++) {
 		cpumask_t nodemask = node_to_cpumask(i);

@@ -4273,6 +4278,10 @@
 		init_sched_build_groups(sched_group_phys, nodemask,
 						&cpu_to_phys_group);
 	}
+#else
+	init_sched_build_groups(sched_group_phys, cpu_possible_map,
+							&cpu_to_phys_group);
+#endif
 
 #ifdef CONFIG_NUMA
 	/* Set up node groups */
#6 lore raw

Re: [sched] fix sched_domains hotplug bootstrap ordering vs. cpu_online_map issue

Quoted 25 lines · William Lee Irwin III
Quoted 2 lines · William Lee Irwin III
Quoted 2 lines

This is the whole thing; the "other half" referred to a new hunk added to the patch (identical to this one) posted in its entirety.

Quoted 1 line · Andrew Morton

ho-hum. changelog, please?

cpu_online_map is not set up at the time of sched domain initialization when hotplug cpu paths are used for SMP booting. At this phase of bootstrapping, cpu_possible_map can be used by the various architectures using cpu hotplugging for SMP bootstrap, but the manipulations of cpu_online_map done on behalf of NUMA architectures, done indirectly via node_to_cpumask(), can't, because cpu_online_map starts depopulated and hasn't yet been populated. On true NUMA architectures this is a distinct cpumask_t from cpu_online_map and so the unpatched code works on NUMA; on non-NUMA architectures the definition of node_to_cpumask() this way breaks and would require an invasive sweeping of users of node_to_cpumask() to change it to e.g. cpu_possible_map, as cpu_possible_map is not suitable for use at runtime as a substitute for cpu_online_map.

Quoted 2 lines
Signed-off-by William Irwin <[email protected]>

Yeah I guess this is probably the best thing to do for now.

Martin, I wonder if this patch fixes your z/VM problem?

#7 lore raw

Re: [sched] fix sched_domains hotplug bootstrap ordering vs. cpu_online_map issue

In reply to William Lee Irwin III
Quoted 21 lines · William Lee Irwin III
Quoted 2 lines · William Lee Irwin III
Quoted 2 lines

This is the whole thing; the "other half" referred to a new hunk added to the patch (identical to this one) posted in its entirety.

Quoted 1 line · Andrew Morton

ho-hum. changelog, please?

cpu_online_map is not set up at the time of sched domain initialization when hotplug cpu paths are used for SMP booting. At this phase of bootstrapping, cpu_possible_map can be used by the various architectures using cpu hotplugging for SMP bootstrap, but the manipulations of cpu_online_map done on behalf of NUMA architectures, done indirectly via node_to_cpumask(), can't, because cpu_online_map starts depopulated and hasn't yet been populated. On true NUMA architectures this is a distinct cpumask_t from cpu_online_map and so the unpatched code works on NUMA; on non-NUMA architectures the definition of node_to_cpumask() this way breaks and would require an invasive sweeping of users of node_to_cpumask() to change it to e.g. cpu_possible_map, as cpu_possible_map is not suitable for use at runtime as a substitute for cpu_online_map.

Quoted 1 line
Signed-off-by William Irwin <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>

Ingo

author
#8 lore raw

Re: [sched] fix sched_domains hotplug bootstrap ordering vs. cpu_online_map issue

Quoted 14 lines · Ingo Molnar
Quoted 14 lines

cpu_online_map is not set up at the time of sched domain initialization when hotplug cpu paths are used for SMP booting. At this phase of bootstrapping, cpu_possible_map can be used by the various architectures using cpu hotplugging for SMP bootstrap, but the manipulations of cpu_online_map done on behalf of NUMA architectures, done indirectly via node_to_cpumask(), can't, because cpu_online_map starts depopulated and hasn't yet been populated. On true NUMA architectures this is a distinct cpumask_t from cpu_online_map and so the unpatched code works on NUMA; on non-NUMA architectures the definition of node_to_cpumask() this way breaks and would require an invasive sweeping of users of node_to_cpumask() to change it to e.g. cpu_possible_map, as cpu_possible_map is not suitable for use at runtime as a substitute for cpu_online_map.

Quoted 3 lines
Quoted 1 line
Signed-off-by William Irwin <[email protected]>
Signed-off-by Ingo Molnar <[email protected]>

Well this patch got in, which is what I want, since it allows the non-NUMA machines to work with hotplug CPUs again. However, is anyone actually looking to fix this for real?

The fundamental problem is that NUMA or the scheduler (or both) are broken with regard to hotplug.

The origin of the breakage is the differences between cpu_possible_map and cpu_online_map. In hotplug CPU, there are two ways to do
initialisations: you can initialise from cpu_online_map, but then you *must* have a cpu hotplug notify listener to add data structures for the extra CPUs as they come on-line, or you can initialise from cpu_possible_map and not bother with a notifier. The disadvantage of the latter is that cpu_possible_map may be vastly larger than cpu_online_map ever gets to, thus wasting valuable kernel memory.

The scheduler code is schizophrenic in this regard in that it does both:
it initialises static data structures from cpu_possible_map, but it also has a hotplug cpu listener for starting things like the migration threads.

I suspect the NUMA people would like us all to go to the former method (initialise only from cpu_online_map and have a proper hotplug listener) since their possible maps are pretty huge. However, which is it to be:
fix NUMA (to have two cpu_to_node() maps for the possible and online cpus per node) or fix the scheduler to do initialisation correctly?

Perhaps this should be phased: change NUMA first temporarily for phase one and then fix the scheduler (and everyone else initialising from cpu_possible_map) in the second.

James

#9 lore raw

Re: [sched] fix sched_domains hotplug bootstrap ordering vs. cpu_online_map issue

Quoted 6 lines · James Bottomley

Well this patch got in, which is what I want, since it allows the non-NUMA machines to work with hotplug CPUs again. However, is anyone actually looking to fix this for real?

I think someone else (tm) is looking at it :)
Some of the IBM hotplug guys I think.

Quoted 23 lines

The fundamental problem is that NUMA or the scheduler (or both) are broken with regard to hotplug.

The origin of the breakage is the differences between cpu_possible_map and cpu_online_map. In hotplug CPU, there are two ways to do
initialisations: you can initialise from cpu_online_map, but then you *must* have a cpu hotplug notify listener to add data structures for the extra CPUs as they come on-line, or you can initialise from cpu_possible_map and not bother with a notifier. The disadvantage of the latter is that cpu_possible_map may be vastly larger than cpu_online_map ever gets to, thus wasting valuable kernel memory.

The scheduler code is schizophrenic in this regard in that it does both:
it initialises static data structures from cpu_possible_map, but it also has a hotplug cpu listener for starting things like the migration threads.

I suspect the NUMA people would like us all to go to the former method (initialise only from cpu_online_map and have a proper hotplug listener) since their possible maps are pretty huge. However, which is it to be:
fix NUMA (to have two cpu_to_node() maps for the possible and online cpus per node) or fix the scheduler to do initialisation correctly?

Quoted 5 lines

Perhaps this should be phased: change NUMA first temporarily for phase one and then fix the scheduler (and everyone else initialising from cpu_possible_map) in the second.

The scheduler *should* be able to be fixed nicely by using cpu_online_map everywhere, and basically undoing then redoing the domains setup before and after the hoplug, respectively.

So you'd re-attach the dummy domain to all CPUs, do the hotplug operation, then setup the domains again and re-attach them.

This whole sequence could be pretty expensive, but I don't think the hotplug guys care. It would allow us to get rid of cpus_and(... cpu_online_map) from a lot of places in the scheduler too, which would be nice.

The actual code to do it shouldn't be more than a few lines (but I could be overlooking something).