Dynamic sched domains (v0.6)
Subsystems (2)
PATCH 1/1 Dynamic sched domains (v0.6)
Ok, here is hopefully the last iteration of the dynamic sched domain patches before I can send it off to Andrew.
I would have posted it earlier, except I got sidetracked with all the hotplug+preempt issues. cpusets+hotplug+preempt requires more testing and I'll post those patches sometime later in the week
patch1 - sched.c+sched.h changes
patch2 - cpuset.c+Documentation changes
patch3 - ia64 changes
All patches are against 2.6.12-rc4-mm1 and have been tested (except for the ia64 changes)
linux-2.6.12-rc4-mm1-1/include/linux/sched.h | 2 linux-2.6.12-rc4-mm1-1/kernel/sched.c | 131 +++++++++++++++-------- linux-2.6.12-rc4-mm1-2/Documentation/cpusets.txt | 16 ++ linux-2.6.12-rc4-mm1-2/kernel/cpuset.c | 89 ++++++++++++--- linux-2.6.12-rc4-mm1-3/arch/ia64/kernel/domain.c | 77 ++++++++----- 5 files changed, 225 insertions(+), 90 deletions(-)
o Patch1 adds the new API partition_sched_domains.
I have incorporated all of the feedback from before.
o I didnt think it necessary to add another semaphore to protect the sched domain changes as suggested by Nick. As it was clear last week, hotplug+cpusets causes enough issues with nested sems, so I didnt want to add yet another in the mix
o I have removed the __devinit qualifier from some functions which will now get called anytime changes are made to exclusive cpusets instead of only during boot
o arch_init_sched_domains/arch_destroy_sched_domains now take a pointer to a const cpumask_t * type.
-Dinakar
include/linux/sched.h +2 -0
@@ -561,6 +561,8 @@ struct sched_domain {#endif }; +extern void partition_sched_domains (cpumask_t *partition1, +cpumask_t *partition2);#ifdef ARCH_HAS_SCHED_DOMAIN /* Useful helpers that arch setup code may use. Defined in kernel/sched.c */ extern cpumask_t cpu_isolated_map;
kernel/sched.c +87 -44
275 lines not on this page · raw message
PATCH 2/3 Dynamic sched domains (v0.6)
o Patch2 has updated cpusets documentation and the core update_cpu_domains function
o I have also moved the dentry d_lock as discussed previously
Documentation/cpusets.txt +16 -0
@@ -51,6 +51,14 @@ mems_allowed vector. If a cpuset is cpu or mem exclusive, no other cpuset, other than a direct ancestor or descendent, may share any of the same CPUs or Memory Nodes. +A cpuset that is cpu exclusive has a sched domain associated with it. +The sched domain consists of all cpus in the current cpuset that are not +part of any exclusive child cpusets. +This ensures that the scheduler load balacing code only balances +against the cpus that are in the sched domain as defined above and not +all of the cpus in the system. This removes any overhead due to +load balancing code trying to pull tasks outside of the cpu exclusive +cpuset only to be prevented by the tasks' cpus_allowed mask. User level code may create and destroy cpusets by name in the cpuset virtual file system, manage the attributes and permissions of these
@@ -84,6 +92,9 @@ This can be especially valuable on: and a database), or * NUMA systems running large HPC applications with demanding performance characteristics. + * Also cpu-exclusive cpusets are useful for servers running orthogonal + workloads such as RT applications requiring low latency and HPC + applications that are throughput sensitive These subsets, or "soft partitions" must be able to be dynamically adjusted, as the job mix changes, without impacting other concurrently
@@ -125,6 +136,8 @@ Cpusets extends these two mechanisms as - A cpuset may be marked exclusive, which ensures that no other cpuset (except direct ancestors and descendents) may contain any overlapping CPUs or Memory Nodes. + Also a cpu-exclusive cpuset would be associated with a sched + domain. - You can list all the tasks (by pid) attached to any cpuset. The implementation of cpusets requires a few, simple hooks
@@ -136,6 +149,9 @@ into the rest of the kernel, none in per allowed in that tasks cpuset. - in sched.c migrate_all_tasks(), to keep migrating tasks within the CPUs allowed by their cpuset, if possible. + - in sched.c, a new API partition_sched_domains for handling + sched domain changes associated with cpu-exclusive cpusets + and related changes in both sched.c and arch/ia64/kernel/domain.c - in the mbind and set_mempolicy system calls, to mask the requested Memory Nodes by what's allowed in that tasks cpuset. - in page_alloc, to restrict memory to allowed nodes.
kernel/cpuset.c +74 -15
132 lines not on this page · raw message
Quoted 4 lines · Dinakar Guniguntala
o Patch2 has updated cpusets documentation and the core
update_cpu_domainsfunction
o I have also moved the dentryd_lockas discussed previously
Hi Dinakar,
patch1 looks good. Just one tiny little minor thing:
kernel/cpuset.c static int validate_change(const struct · line 648
Quoted 6 lines
kernel/cpuset.c +6
-0+ +lock_cpu_hotplug (); +partition_sched_domains (&pspan, &cspan); +unlock_cpu_hotplug (); +} +
I don't think the cpu hotplug lock isn't supposed to provide synchronisation between readers (for example, it may be turned into an rwsem), but only between the thread and the cpu hotplug callbacks.
In that case, can you move this locking into kernel/sched.c, and add the comment in partition_sched_domains that the callers must take care of synchronisation (which without reading the code, I assume you're doing with the cpuset sem?).
If you agree with that change, you can add an
to patch 1 and send it to Andrew whenever you're ready (better CC Ingo as well). If not, please discuss! :)
Thanks,
Nick
Quoted 11 lines · Nick Piggin
kernel/cpuset.c static int validate_change(const struct · line 648
Quoted 6 lines
kernel/cpuset.c +6
-0+ +lock_cpu_hotplug (); +partition_sched_domains (&pspan, &cspan); +unlock_cpu_hotplug (); +} +I don't think the cpu hotplug lock isn't supposed to provide synchronisation between readers (for example, it may be turned into an rwsem), but only between the thread and the cpu hotplug callbacks.
That should be ok
Quoted 5 lines
In that case, can you move this locking into
kernel/sched.c, and add the comment inpartition_sched_domainsthat the callers must take care of synchronisation (which without reading the code, I assume you're doing with the cpuset sem?).
I didnt want to do this as my next patch, which introduces hotplug support for dynamic sched domains, also calls partition_sched_domains. That code is called with the hotplug lock already held. (I am still testing that code, it should be out by this weekend)
However I will add a comment about the synchronization and yes currently it is taken care of by the cpuset sem
-Dinakar
o Patch3 has the ia64 changes similar to kernel/sched.c
o This patch compiles ok, but has not been tested
arch/ia64/kernel/domain.c +46 -31
@@ -27,7 +27,7 @@* * Should use nodemask_t. */ -static int __devinit find_next_best_node (int node,unsigned long *used_nodes) +static int find_next_best_node (int node,unsigned long *used_nodes) {int i, n, val, min_val, best_node =0 ;
@@ -66,7 +66,7 @@ static int __devinit find_next_best_node* should be one that prevents unnecessary balancing, but also spreads tasks * out optimally. */ -static cpumask_t __devinit sched_domain_node_span (int node) +static cpumask_t sched_domain_node_span (int node) {int i;cpumask_t span, nodemask;
@@ -96,7 +96,7 @@ static cpumask_t __devinit sched_domain_#ifdef CONFIG_SCHED_SMT static DEFINE_PER_CPU(struct sched_domain, cpu_domains);static struct sched_group sched_group_cpus[NR_CPUS ]; -static int __devinit cpu_to_cpu_group (int cpu) +static int cpu_to_cpu_group (int cpu) {return cpu; }
@@ -104,7 +104,7 @@ static int __devinit cpu_to_cpu_group(in static DEFINE_PER_CPU(struct sched_domain, phys_domains);static struct sched_group sched_group_phys[NR_CPUS ]; -static int __devinit cpu_to_phys_group (int cpu) +static int cpu_to_phys_group (int cpu) {#ifdef CONFIG_SCHED_SMT return first_cpu (cpu_sibling_map[cpu]);
@@ -125,44 +125,36 @@ static struct sched_group *sched_group_n static DEFINE_PER_CPU(struct sched_domain, allnodes_domains);static struct sched_group sched_group_allnodes[MAX_NUMNODES ]; -static int __devinit cpu_to_allnodes_group (int cpu) +static int cpu_to_allnodes_group (int cpu) {return cpu_to_node (cpu); }#endif /* -* Set up scheduler domains and groups. Callers must hold the hotplug lock. +* Build sched domains for a given set of cpus and attach the sched domains +* to the individual cpus */ -void __devinit arch_init_sched_domains (void ) +void build_sched_domains (const cpumask_t *cpu_map) {int i; -cpumask_t cpu_default_map; - -/* -* Setup mask for cpus without special case scheduling requirements. -* For now this just excludes isolated cpus, but could be used to -* exclude other special cases in the future. -*/ -cpus_complement (cpu_default_map, cpu_isolated_map); -cpus_and (cpu_default_map, cpu_default_map, cpu_online_map);/* -* Set up domains. Isolated domains just stay on the dummy domain. +* Set up domains for cpus specified by the cpu_map. */ -for_each_cpu_mask (i, cpu_default_map) { +for_each_cpu_mask (i, *cpu_map) {int group;struct sched_domain *sd =NULL , *p;cpumask_t nodemask =node_to_cpumask (cpu_to_node (i)); -cpus_and (nodemask, nodemask, cpu_default_map); +cpus_and (nodemask, nodemask, *cpu_map);#ifdef CONFIG_NUMA if (num_online_cpus () >SD_NODES_PER_DOMAIN *cpus_weight (nodemask)) { sd = &per_cpu (allnodes_domains, i); *sd =SD_ALLNODES_INIT ; - sd->span = cpu_default_map; + sd->span = *cpu_map; group =cpu_to_allnodes_group (i); sd->groups = &sched_group_allnodes[group]; p = sd;
@@ -173,7 +165,7 @@ void __devinit arch_init_sched_domains(v *sd =SD_NODE_INIT ; sd->span =sched_domain_node_span (cpu_to_node (i)); sd->parent = p; -cpus_and (sd->span, sd->span, cpu_default_map); +cpus_and (sd->span, sd->span, *cpu_map);#endif p = sd;
@@ -190,7 +182,7 @@ void __devinit arch_init_sched_domains(v group =cpu_to_cpu_group (i); *sd =SD_SIBLING_INIT ; sd->span = cpu_sibling_map[i]; -cpus_and (sd->span, sd->span, cpu_default_map); +cpus_and (sd->span, sd->span, *cpu_map); sd->parent = p; sd->groups = &sched_group_cpus[group];#endif
@@ -198,9 +190,9 @@ void __devinit arch_init_sched_domains(v#ifdef CONFIG_SCHED_SMT /* Set up CPU (sibling) groups */ -for_each_cpu_mask (i, cpu_default_map) { +for_each_cpu_mask (i, *cpu_map) {cpumask_t this_sibling_map = cpu_sibling_map[i]; -cpus_and (this_sibling_map, this_sibling_map, cpu_default_map); +cpus_and (this_sibling_map, this_sibling_map, *cpu_map);if (i !=first_cpu (this_sibling_map))continue ;
@@ -213,7 +205,7 @@ void __devinit arch_init_sched_domains(vfor (i =0 ; i <MAX_NUMNODES ; i++) {cpumask_t nodemask =node_to_cpumask (i); -cpus_and (nodemask, nodemask, cpu_default_map); +cpus_and (nodemask, nodemask, *cpu_map);if (cpus_empty (nodemask))continue ;
@@ -222,7 +214,7 @@ void __devinit arch_init_sched_domains(v }#ifdef CONFIG_NUMA -init_sched_build_groups (sched_group_allnodes, cpu_default_map, +init_sched_build_groups (sched_group_allnodes, *cpu_map, &cpu_to_allnodes_group);for (i =0 ; i <MAX_NUMNODES ; i++) {
@@ -233,12 +225,12 @@ void __devinit arch_init_sched_domains(vcpumask_t covered =CPU_MASK_NONE ;int j; -cpus_and (nodemask, nodemask, cpu_default_map); +cpus_and (nodemask, nodemask, *cpu_map);if (cpus_empty (nodemask))continue ; domainspan =sched_domain_node_span (i); -cpus_and (domainspan, domainspan, cpu_default_map); +cpus_and (domainspan, domainspan, *cpu_map); sg =kmalloc (sizeof (struct sched_group ),GFP_KERNEL ); sched_group_nodes[i] = sg;
@@ -266,7 +258,7 @@ void __devinit arch_init_sched_domains(vint n = (i + j) %MAX_NUMNODES ;cpus_complement (notcovered, covered); -cpus_and (tmp, notcovered, cpu_default_map); +cpus_and (tmp, notcovered, *cpu_map);cpus_and (tmp, tmp, domainspan);if (cpus_empty (tmp))break ;
@@ -293,7 +285,7 @@ void __devinit arch_init_sched_domains(v#endif /* Calculate CPU power for physical packages and nodes */ -for_each_cpu_mask (i, cpu_default_map) { +for_each_cpu_mask (i, *cpu_map) {int power;struct sched_domain *sd;#ifdef CONFIG_SCHED_SMT
@@ -359,13 +351,36 @@ next_sg:cpu_attach_domain (sd, i); } } +/* +* Set up scheduler domains and groups. Callers must hold the hotplug lock. +*/ +void arch_init_sched_domains (const cpumask_t *cpu_map) +{ +cpumask_t cpu_default_map; + +/* +* Setup mask for cpus without special case scheduling requirements. +* For now this just excludes isolated cpus, but could be used to +* exclude other special cases in the future. +*/ +cpus_complement (cpu_default_map, cpu_isolated_map); +cpus_and (cpu_default_map, cpu_default_map, *cpu_map); + +build_sched_domains (&cpu_default_map); +} -void __devinit arch_destroy_sched_domains (void ) +void arch_destroy_sched_domains (const cpumask_t *cpu_map) {#ifdef CONFIG_NUMA int i;for (i =0 ; i <MAX_NUMNODES ; i++) { +cpumask_t nodemask =node_to_cpumask (i);struct sched_group *oldsg, *sg = sched_group_nodes[i]; + +cpus_and (nodemask, nodemask, *cpu_map); +if (cpus_empty (nodemask)) +continue ; +if (sg ==NULL )continue ; sg = sg->next;
Looking good. Some minor comments on these three patches ...
* The name 'nodemask' for the cpumask_t of CPUs that are siblings to CPU i is a bit confusing (yes, that name was already there). How about something like 'siblings' ?
* I suspect that the following two lines:
cpus_complement(cpu_default_map, cpu_isolated_map); cpus_and(cpu_default_map, cpu_default_map, *cpu_map);
can be replaced with the one line:
cpus_andnot(cpu_default_map, *cpu_map, cpu_isolated_map);
* You have 'cpu-exclusive' in some places in the Documentation. I would mildly prefer to always spell this 'cpu_exclusive' (with underscore, not hyphen).
* I like how this design came out, as described in:
A cpuset that is cpu exclusive has a sched domain associated with it. The sched domain consists of all cpus in the current cpuset that are not part of any exclusive child cpusets.
Good work.
* Question - any idea how much of a performance hiccup a system will feel whenever someone changes the cpu_exclusive cpusets? Could this lead to a denial-of-service attack, if say some untrusted user were allowed modify privileges on some small cpuset that was cpu_exclusive, and they abused that privilege by turning on and off the cpu_exclusive property on their little cpuset (or creating/destroying an exclusive child):
cd /dev/cpuset/$(cat /proc/self/cpuset)
while true
do
for i in 0 1
do
echo $i > cpu_exclusive
done
done
If so, perhaps we should recommend that shared systems with untrusted users avoid allowing a cpu_exclusive cpuset to be modifiable, or to have a cpu_exclusive flag modifiable, by those untrusted users.
* The cpuset 'oldcs' in update_flag() seems to only be used for its cpu_exclusive flag. We could save some stack space on my favorite big honkin NUMA iron by just having a local variable for this 'old_cpu_exclusive' value, instead of the entire cpuset.
* Similarly, though with a bit less savings, one could replace 'oldcs' in update_cpumask() with just the old_cpus_allowed mask. Or, skip even that, and compute a boolean flag:
cpus_changed = cpus_equal(cs->cpus_allowed, trialcs.cpus_allowed);
before copying over the trialcs, so we only need one word of stack for the boolean, not possibly many words for a cpumask.
* Non-traditional code style:
}
else {
should be instead:
} else { * Is it the case that update_cpu_domains() is called with cpuset_sem held? Would it be a good idea to note in the comment for that routine:
* Call with cpuset_sem held. May nest a call to the
* lock_cpu_hotplug()/unlock_cpu_hotplug() pair.
I didn't callout the cpuset_sem lock precondition on many routines, but since this one can nest the cpu_hotplug lock, it might be worth calling it out, for the benefit of engineers who are passing through, needing to know how the hotplug lock nests with other semaphores.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401Quoted 5 lines · Paul Jackson
Looking good. Some minor comments on these three patches ...
* The name 'nodemask' for the
cpumask_tof CPUs that are siblings to CPU i is a bit confusing (yes, that name was already there). How about something like 'siblings' ?
Not sure which code you are referring to here ?? I dont see any nodemask referring to SMT siblings ?
Quoted 3 lines
can be replaced with the one line:
cpus_andnot(cpu_default_map, *cpu_map, cpu_isolated_map);
yeah, ok
Quoted 2 lines
I would mildly prefer to always spell this '
cpu_exclusive' (with underscore, not hyphen).
fine
Quoted 1 line
Good work.
Thanks !
Quoted 8 lines
* Question - any idea how much of a performance hiccup a system will feel whenever someone changes the
cpu_exclusivecpusets? Could this lead to a denial-of-service attack, if say some untrusted user were allowed modify privileges on some small cpuset that wascpu_exclusive, and they abused that privilege by turning on and off thecpu_exclusiveproperty on their little cpuset (or creating/destroying an exclusive child):
I tried your script and see that it makes absolutely no impact on top. The CPU on which it is running is mostly 100% idle. However I'll run more tests to confirm that it has no impact
Quoted 6 lines
* The cpuset 'oldcs' in
update_flag()seems to only be used for itscpu_exclusiveflag. We could save some stack space on my favorite big honkin NUMA iron by just having a local variable for this 'old_cpu_exclusive' value, instead of the entire cpuset.
Quoted 6 lines
* Similarly, though with a bit less savings, one could replace 'oldcs' in
update_cpumask()with just theold_cpus_allowedmask. Or, skip even that, and compute a boolean flag:cpus_changed = cpus_equal(cs->cpus_allowed, trialcs.cpus_allowed);before copying over the trialcs, so we only need one word of stack for the boolean, not possibly many words for a cpumask.
ok for both
Quoted 6 lines
* Non-traditional code style:
} else { should be instead: } else {
I dont know how that snuck back in, I'll change that
Quoted 9 lines
* Is it the case that
update_cpu_domains()is called withcpuset_semheld? Would it be a good idea to note in the comment for that routine:
* Call withcpuset_semheld. May nest a call to the
* lock_cpu_hotplug()/unlock_cpu_hotplug() pair.
I didn't callout thecpuset_semlock precondition on many routines, but since this one can nest thecpu_hotpluglock, it might be worth calling it out, for the benefit of engineers who are passing through, needing to know how the hotplug lock nests with other semaphores.
ok
I do feel with the above updates the patches can go into -mm. Appreciate all the review comments from everyone, Thanks
-Dinakar
Quoted 6 lines · Dinakar
Quoted 3 lines
* The name 'nodemask' for the
cpumask_tof CPUs that are siblings to CPU i is a bit confusing (yes, that name was already there). How about something like 'siblings' ?Not sure which code you are referring to here ?? I dont see any nodemask referring to SMT siblings ?
This comment was referring to lines such as the following, which appear a few places in your patch (though not lines you wrote, just nearby lines, in all but one case):
cpumask_t nodemask = node_to_cpumask(cpu_to_node(i));
I was thinking to change such a line to:
cpumask_t sibling = node_to_cpumask(cpu_to_node(i));
However, it is no biggie, and since it is not in your actual new code, probably should not be part of your patch anyway.
There is one place, arch_destroy_sched_domains(), where you added such a line, but there you should probably use the same 'nodemask' name as the other couple of places, unless and until these places change together.
So bottom line - nevermind this comment.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401Quoted 3 lines · Dinakar
I tried your script and see that it makes absolutely no impact on top. The CPU on which it is running is mostly 100% idle. However I'll run more tests to confirm that it has no impact
I have no particular intuition, one way or the other, on how much a dynamic reallocation of sched domains will impact the system. So once you are comfortable that this is not normally a problem (which you might already be), don't worry about it further on my account.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401Paul Jackson
Acked-byQuoted 1 line · Dinakar
I do feel with the above updates the patches can go into -mm.
Signature · 3 lines
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.650.933.1373, 1.925.600.0401