sched: fix a bug in sched domain degenerate
PATCH 1/1 sched: fix a bug in sched domain degenerate
(1) on i386 with SCHED_SMT and SCHED_MC enabled
# mount -t cgroup -o cpuset xxx /mnt # echo 0 > /mnt/cpuset.sched_load_balance # mkdir /mnt/0 # echo 0 > /mnt/0/cpuset.cpus # dmesg CPU0 attaching sched-domain: domain 0: span 0 level CPU groups: 0
(2) on i386 with SCHED_MC enabled but SCHED_SMT disabled
# same with (1) # dmesg CPU0 attaching NULL sched-domain.
The bug is some sched domains may be skipped unintentionally when doing sched domain degenerating.
--- kernel/sched.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
kernel/sched.c +4 -2
@@ -6875,15 +6875,17 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)struct sched_domain *tmp;/* Remove the sched domains which do not contribute to scheduling. */ -for (tmp = sd; tmp; tmp = tmp->parent) { +for (tmp = sd; tmp; ) {struct sched_domain *parent = tmp->parent;if (!parent)break ; +if (sd_parent_degenerate (tmp, parent)) { tmp->parent = parent->parent;if (parent->parent) parent->parent->child = tmp; - } + }else + tmp = tmp->parent; }if (sd &&sd_degenerate (sd)) {
Signature · 1 line
1.5.4.rc3
Ingo Molnar
AppliedQuoted 15 lines · Li Zefan
(1) on i386 with
SCHED_SMTandSCHED_MCenabled# mount -t cgroup -o cpuset xxx /mnt # echo 0 > /mnt/cpuset.sched_load_balance # mkdir /mnt/0 # echo 0 > /mnt/0/cpuset.cpus # dmesg CPU0 attaching sched-domain: domain 0: span 0 level CPU groups: 0(2) on i386 with
SCHED_MCenabled butSCHED_SMTdisabled# same with (1) # dmesg CPU0 attaching NULL sched-domain.
Quoted 7 lines
The bug is some sched domains may be skipped unintentionally when doing sched domain degenerating.
Signed-off-by Li Zefan <[email protected]>--- kernel/sched.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
applied to tip/sched/urgent, thanks!
Ingo
Hi Ingo,
I just read the modified changelog in the git-log, and it is wrong (or maybe my fix is wrong?), I should have explained the bug clearer. :(
I'm writing this mail to confirm if my thought and fix is right or not.
Quoted 8 lines
commit
f29c9b1ccb52904ee442a933cf3dee628f9f4e62Author: Li Zefan <[email protected]> Date: Thu Nov 6 09:45:16 2008 +0800sched: fix a bug in sched domain degenerate
Impact: re-add incorrectly eliminated sched domain layers
This statement is wrong..
Quoted 10 lines
(1) on i386 with
SCHED_SMTandSCHED_MCenabled # mount -t cgroup -o cpuset xxx /mnt # echo 0 >/mnt/cpuset.sched_load_balance# mkdir/mnt/0
# echo 0 >/mnt/0/cpuset.cpus
# dmesg
CPU0 attaching sched-domain:
domain 0: span 0 level CPU
groups: 0
I think this behavior is wrong.
Quoted 5 lines
(2) on i386 with
SCHED_MCenabled butSCHED_SMTdisabled # same with (1)
# dmesg
CPU0 attaching NULL sched-domain.
And this is right. CPU domain has only 1 cpu so it does not contribute to scheduling, so it can be removed.
Quoted 3 lines
The bug is that some sched domains may be skipped unintentionally when degenerating (optimizing) sched domains.
The bug is, some sched domains won't be checked in the for loop due to the bug, so they have no chance to be removed.
In the for loop, we check if the parents domains can be removed:
cur_ptr
|
v
SMT--->MC--->CPU--->NULL
(parent MC is checked and can be removed)
=>
cur_ptr
|
vSMT--->CPU--->NULL
(break out of the for loop, because cur_ptr->parent == NULL)
so CPU domain won't be checked. When we delete MC domain, the pointer should not move forwards, so the fix is:
cur_ptr
|
v
SMT--->CPU--->NULL
Quoted 28 lines
Signed-off-by Li Zefan <[email protected]>Acked-by Peter Zijlstra <[email protected]>Signed-off-by Ingo Molnar <[email protected]>kernel/sched.c +4
-2@@ -6877,15 +6877,17 @@ cpu_attach_domain(struct sched_domain *sd, struct root_domain *rd, int cpu)struct sched_domain *tmp;/* Remove the sched domains which do not contribute to scheduling. */ -for (tmp = sd; tmp; tmp = tmp->parent) { +for (tmp = sd; tmp; ) {struct sched_domain *parent = tmp->parent;if (!parent)break ; +if (sd_parent_degenerate (tmp, parent)) { tmp->parent = parent->parent;if (parent->parent) parent->parent->child = tmp; - } + }else + tmp = tmp->parent; }if (sd &&sd_degenerate (sd)) {
Quoted 13 lines · Li Zefan
Hi Ingo,
I just read the modified changelog in the git-log, and it is wrong (or maybe my fix is wrong?), I should have explained the bug clearer. :(
I'm writing this mail to confirm if my thought and fix is right or not.
Quoted 4 lines
commit
f29c9b1ccb52904ee442a933cf3dee628f9f4e62Author: Li Zefan <[email protected]> Date: Thu Nov 6 09:45:16 2008 +0800
Quoted 6 lines
Quoted 4 lines
sched: fix a bug in sched domain degenerate
Impact: re-add incorrectly eliminated sched domain layers
This statement is wrong..
that's OK, because the patch is correct :-)
Quoted 47 lines
Quoted 10 lines
(1) on i386 with
SCHED_SMTandSCHED_MCenabled # mount -t cgroup -o cpuset xxx /mnt # echo 0 >/mnt/cpuset.sched_load_balance# mkdir/mnt/0
# echo 0 >/mnt/0/cpuset.cpus
# dmesg
CPU0 attaching sched-domain:
domain 0: span 0 level CPU
groups: 0I think this behavior is wrong.
Quoted 5 lines
(2) on i386 with
SCHED_MCenabled butSCHED_SMTdisabled # same with (1)
# dmesg
CPU0 attaching NULL sched-domain.And this is right. CPU domain has only 1 cpu so it does not contribute to scheduling, so it can be removed.
Quoted 3 lines
The bug is that some sched domains may be skipped unintentionally when degenerating (optimizing) sched domains.
The bug is, some sched domains won't be checked in the for loop due to the bug, so they have no chance to be removed.
In the for loop, we check if the parents domains can be removed:
cur_ptr
|
v
SMT--->MC--->CPU--->NULL(parent MC is checked and can be removed)
=>
cur_ptr | vSMT--->CPU--->NULL
(break out of the for loop, because
cur_ptr->parent== NULL)
Quoted 7 lines
so CPU domain won't be checked. When we delete MC domain, the pointer should not move forwards, so the fix is:
cur_ptr
|
v
SMT--->CPU--->NULL
ah, ok - i misunderstood the direction of the fix. So it strengthens degeneration - which is a valid fix too. And the commit message remains there to shame my reading skills forever ;-)
Ingo