Dinakar,
(1) Does this patch look right to you?
(2) I don't understand this code:
* When do we ever create sched domains for cpusets
that -are- cpu_exclusive? All I see here are
calls to partition_sched_domains() with various
permutations of pspan and cspan that are the
cpus from various non-exclusive cpusets.
* Why do we return (setting up no sched domains
at this time) if the current cpuset's cpus
covers all the non exclusive cpus of our parent,
but continue on to make a sched domain just for
our parent if there are other non-exclusive cpus
in our sibling cpusets?
====
Remove a useless line from the sched domain setup code in cpusets.
When I removed the 'is_removed()' flag test from the sched domain
setup code in cpusets, as part of my July 23, 2006 patch:
Cpuset: fix ABBA deadlock with cpu hotplug lock
I failed to notice that this opened the door to a little bit of code
simplification. A line of code that had to cover for the possibility
that a cpuset marked cpu_exclusive was marked for removal could
be eliminated. In the code section visible in this patch, it is
now the case that cur->cpus_allowed is always a subset of pspan,
so it is always a no-op to cpus_or() cur->cpus_allowed into pspan.
Signed-off-by: Paul Jackson <[email protected]>
---
kernel/cpuset.c | 1 -
1 files changed, 1 deletion(-)
--- 2.6.19-rc1-mm1.orig/kernel/cpuset.c 2006-10-13 21:31:16.000000000 -0700
+++ 2.6.19-rc1-mm1/kernel/cpuset.c 2006-10-13 21:32:20.000000000 -0700
@@ -783,7 +783,6 @@ static void update_cpu_domains(struct cp
cpus_andnot(pspan, pspan, c->cpus_allowed);
}
if (!is_cpu_exclusive(cur)) {
- cpus_or(pspan, pspan, cur->cpus_allowed);
if (cpus_equal(pspan, cur->cpus_allowed))
return;
cspan = CPU_MASK_NONE;
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
On Fri, Oct 13, 2006 at 09:55:17PM -0700, Paul Jackson wrote:
> Dinakar,
>
> (1) Does this patch look right to you?
>
> (2) I don't understand this code:
>
> * When do we ever create sched domains for cpusets
> that -are- cpu_exclusive? All I see here are
> calls to partition_sched_domains() with various
> permutations of pspan and cspan that are the
> cpus from various non-exclusive cpusets.
>
> * Why do we return (setting up no sched domains
> at this time) if the current cpuset's cpus
> covers all the non exclusive cpus of our parent,
> but continue on to make a sched domain just for
> our parent if there are other non-exclusive cpus
> in our sibling cpusets?
>
> ====
>
> Remove a useless line from the sched domain setup code in cpusets.
>
> When I removed the 'is_removed()' flag test from the sched domain
> setup code in cpusets, as part of my July 23, 2006 patch:
>
> Cpuset: fix ABBA deadlock with cpu hotplug lock
>
> I failed to notice that this opened the door to a little bit of code
> simplification. A line of code that had to cover for the possibility
> that a cpuset marked cpu_exclusive was marked for removal could
> be eliminated. In the code section visible in this patch, it is
> now the case that cur->cpus_allowed is always a subset of pspan,
> so it is always a no-op to cpus_or() cur->cpus_allowed into pspan.
>
> Signed-off-by: Paul Jackson <[email protected]>
>
> ---
>
> kernel/cpuset.c | 1 -
> 1 files changed, 1 deletion(-)
>
> --- 2.6.19-rc1-mm1.orig/kernel/cpuset.c 2006-10-13 21:31:16.000000000 -0700
> +++ 2.6.19-rc1-mm1/kernel/cpuset.c 2006-10-13 21:32:20.000000000 -0700
> @@ -783,7 +783,6 @@ static void update_cpu_domains(struct cp
> cpus_andnot(pspan, pspan, c->cpus_allowed);
> }
> if (!is_cpu_exclusive(cur)) {
> - cpus_or(pspan, pspan, cur->cpus_allowed);
> if (cpus_equal(pspan, cur->cpus_allowed))
> return;
> cspan = CPU_MASK_NONE;
I dont think this is a valid optimization. What we are checking here
is if a previously exclusive cpuset has been changed to a non-exclusive one
(echo 0 > cpu_exclusive), we then OR all the cpus in the current cpuset
to the parent cpuset. We then rebuild a sched domain to include all of the cpus
in the current cpuset and those in the parent not part of exclusive children
So I dont see why this can be done away with
-Dinakar
Dinakar wrote:
> I dont think this is a valid optimization.
I probably don't agree, but I think there might be a better way to deal
with this line of code - remove the entire routine ;).
I have a patch set I am about to send in to lkml for comment, that
removes the entire mechanism connecting cpusets to sched domains,
replacing it with a much simpler (even I can understand) mechanism to
update the isolated_cpu_map on the fly.
If that flies, then this present patch becomes obsolete and irrelevant.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401
> > if (!is_cpu_exclusive(cur)) {
> > - cpus_or(pspan, pspan, cur->cpus_allowed);
> > if (cpus_equal(pspan, cur->cpus_allowed))
> > return;
> > cspan = CPU_MASK_NONE;
>
>
> I dont think this is a valid optimization. What we are checking here
> is if a previously exclusive cpuset has been changed to a non-exclusive one
> (echo 0 > cpu_exclusive), we then OR all the cpus in the current cpuset
> to the parent cpuset. We then rebuild a sched domain to include all of the cpus
> in the current cpuset and those in the parent not part of exclusive children
Not that it matters, but I don't think you need to OR in the current cpuset
cpus to the parents cpus (pspan) here, because they are already in pspan.
Look at the surrounding code:
pspan = par->cpus_allowed;
list_for_each_entry(c, &par->children, sibling) {
if (is_cpu_exclusive(c))
cpus_andnot(pspan, pspan, c->cpus_allowed);
}
if (!is_cpu_exclusive(cur)) {
cpus_or(pspan, pspan, cur->cpus_allowed);
if (cpus_equal(pspan, cur->cpus_allowed))
return;
cspan = CPU_MASK_NONE;
'pspan' starts out with all the parents cpus, which must be a superset
of currents cpus (cur->cpus_allowed.)
Then we subtract (cpus_andnot) from pspan the cpus in the cpu_exclusive
siblings of current. Since current is not cpu_exclusive, and since the
cpus of its cpu_exclusive sibling cpusets cannot overlap with currents
cpus, we could not have subtracted any current cpu from pspan.
So pspan is still a superset of currents cpus.
So OR'ing in currents cpus to pspan is a no-op.
--
I won't rest till it's the best ...
Programmer, Linux Scalability
Paul Jackson <[email protected]> 1.925.600.0401