Skip to contentSkip to conversation

sched: Calculate effective load even if local weight is 0

PATCH In review Mel Gorman 5 participants 8 messages Jan 6, 2014 – Jan 17, 2014 Scheduler

lore
Share & tools
lore

Mel Gorman

author reviewer
#0 lore raw

PATCH 1/1 sched: Calculate effective load even if local weight is 0

(Rik, you authored this patch so it should be sent from you and needs a signed-off assuming people are ok with the changelog.)

Thomas Hellstrom bisected a regression where erratic 3D performance is experienced on virtual machines as measured by glxgears. It identified commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) as the problem which had modified the behaviour of effective_load.

Effective load calculates the difference to the system-wide load if a scheduling entity was moved to another CPU. The task group is not heavier as a result of the move but overall system load can increase/decrease as a result of the change. Commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) changed effective_load to make it suitable for calculating if a particular NUMA node was compute overloaded. To reduce the cost of the function, it assumed that a current sched entity weight of 0 was uninteresting but that is not the case.

wake_affine uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by task_numa_compare since commit fb13c7ee (sched/numa: Use a system-wide search to find swap/migration candidates), this patch simply restores the historical behaviour.

[[email protected]: Wrote changelog]

Should-be-signed-off-and-authored-by-Rik

---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/sched/fair.c +1 -1
index c7395d9..e64b079 100644
@@ -3923,7 +3923,7 @@ static long effective_load(struct task_group *tg, int cpu, long wl, long wg)
 {
 	struct sched_entity *se = tg->se[cpu];
 
-	if (!tg->parent || !wl)	/* the trivial, non-cgroup case */
+	if (!tg->parent)	/* the trivial, non-cgroup case */
 		return wl;
 
 	for_each_sched_entity(se) {
maintainer
#1 lore raw
Quoted 2 lines · Mel Gorman

(Rik, you authored this patch so it should be sent from you and needs a signed-off assuming people are ok with the changelog.)

Where did all this take place? I saw the initial email I think but then nothing..

Not that I was reading much email the past two weeks anyway :-)

author reviewer
#2 lore raw
Quoted 8 lines · Peter Zijlstra
Quoted 2 lines · Mel Gorman

(Rik, you authored this patch so it should be sent from you and needs a signed-off assuming people are ok with the changelog.)

Where did all this take place? I saw the initial email I think but then nothing..

Not that I was reading much email the past two weeks anyway :-)

Much of the discussion was on the bugzilla entry
https://bugzilla.kernel.org/show_bug.cgi?id=67601 which is rare but it happens. In retrospect, I should also have included a blurb saying that the patch resolved the bug but it's not that big a deal.

Signature · 2 lines
Mel Gorman
SUSE Labs
maintainer
#3 lore raw
In reply to Mel Gorman
Quoted 7 lines · Mel Gorman

wake_affine uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by task_numa_compare since commit fb13c7ee (sched/numa: Use a system-wide search to find swap/migration candidates), this patch simply restores the historical behaviour.

Urgh.. yeah. We should merge this. Rik you ok with me adding a from and sob from you?

#4 lore raw
Quoted 11 lines · Peter Zijlstra
Quoted 7 lines · Mel Gorman

wake_affine uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by task_numa_compare since commit fb13c7ee (sched/numa: Use a system-wide search to find swap/migration candidates), this patch simply restores the historical behaviour.

Urgh.. yeah. We should merge this. Rik you ok with me adding a from and sob from you?

Yes, please do :)

Signature · 1 line
All rights reversed
#5 lore raw
In reply to Mel Gorman
sched: Calculate effective load even if local weight is 0
Commit-ID:  9722c2dac708e9468cc0dc30218ef76946ffbc9d
Gitweb:     http://git.kernel.org/tip/9722c2dac708e9468cc0dc30218ef76946ffbc9d
Author:     Rik van Riel <[email protected]>
AuthorDate: Mon, 6 Jan 2014 11:39:12 +0000
Committer:  Ingo Molnar <[email protected]>
CommitDate: Sun, 12 Jan 2014 09:22:15 +0100

sched: Calculate effective load even if local weight is 0

Thomas Hellstrom bisected a regression where erratic 3D performance is experienced on virtual machines as measured by glxgears. It identified commit 58d081b5 ("sched/numa: Avoid overloading CPUs on a preferred NUMA node") as the problem which had modified the behaviour of effective_load.

Effective load calculates the difference to the system-wide load if a scheduling entity was moved to another CPU. The task group is not heavier as a result of the move but overall system load can increase/decrease as a result of the change. Commit 58d081b5 ("sched/numa: Avoid overloading CPUs on a preferred NUMA node") changed effective_load to make it suitable for calculating if a particular NUMA node was compute overloaded. To reduce the cost of the function, it assumed that a current sched entity weight of 0 was uninteresting but that is not the case.

wake_affine() uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by task_numa_compare since commit fb13c7ee (sched/numa: Use a system-wide search to find swap/migration candidates), this patch simply restores the historical behaviour.

---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/sched/fair.c +1 -1
index c7395d9..e64b079 100644
@@ -3923,7 +3923,7 @@ static long effective_load(struct task_group *tg, int cpu, long wl, long wg)
 {
 	struct sched_entity *se = tg->se[cpu];
 
-	if (!tg->parent || !wl)	/* the trivial, non-cgroup case */
+	if (!tg->parent)	/* the trivial, non-cgroup case */
 		return wl;
 
 	for_each_sched_entity(se) {
#6 lore raw
In reply to Mel Gorman

Hi,

Quoted 17 lines · Mel Gorman

(Rik, you authored this patch so it should be sent from you and needs a signed-off assuming people are ok with the changelog.)

Thomas Hellstrom bisected a regression where erratic 3D performance is experienced on virtual machines as measured by glxgears. It identified commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) as the problem which had modified the behaviour of effective_load.

Effective load calculates the difference to the system-wide load if a scheduling entity was moved to another CPU. The task group is not heavier as a result of the move but overall system load can increase/decrease as a result of the change. Commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) changed effective_load to make it suitable for calculating if a particular NUMA node was compute overloaded. To reduce the cost of the function, it assumed that a current sched entity weight of 0 was uninteresting but that is not the case.

Quoted 4 lines

wake_affine uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by

Would it be worth mentioning that besides sync wakeups, wake_affine() uses a weight of 0 for the sched entity, for effective load calculation on the prev_cpu as well?
This is so as to find the effect of moving this task away from the prev_cpu. Here
too we are interested in calculating the effective load of the root task group of this
sched entity on the prev_cpu and the below restored check will be relevant.

Without the below check the difference in the loads of the wake affine CPU and the
prev_cpu can get messed up.

Thanks

Regards
Preeti U Murthy

Quoted 29 lines

task_numa_compare since commit fb13c7ee (sched/numa: Use a system-wide search to find swap/migration candidates), this patch simply restores the historical behaviour.

[[email protected]: Wrote changelog]

Should-be-signed-off-and-authored-by-Rik

---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/sched/fair.c +1 -2
index c7395d9..e64b079 100644
@@ -3923,7 +3923,7 @@ static long effective_load(struct task_group *tg, int cpu, long wl, long wg)
 {
        struct sched_entity *se = tg->se[cpu];
 
-       if (!tg->parent || !wl) /* the trivial, non-cgroup case */
+       if (!tg->parent)        /* the trivial, non-cgroup case */
                return wl;
 
        for_each_sched_entity(se) {
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author reviewer
#7 lore raw
Quoted 34 lines · Preeti Murthy

Hi,

Quoted 17 lines · Mel Gorman

(Rik, you authored this patch so it should be sent from you and needs a signed-off assuming people are ok with the changelog.)

Thomas Hellstrom bisected a regression where erratic 3D performance is experienced on virtual machines as measured by glxgears. It identified commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) as the problem which had modified the behaviour of effective_load.

Effective load calculates the difference to the system-wide load if a scheduling entity was moved to another CPU. The task group is not heavier as a result of the move but overall system load can increase/decrease as a result of the change. Commit 58d081b5 (sched/numa: Avoid overloading CPUs on a preferred NUMA node) changed effective_load to make it suitable for calculating if a particular NUMA node was compute overloaded. To reduce the cost of the function, it assumed that a current sched entity weight of 0 was uninteresting but that is not the case.

Quoted 4 lines

wake_affine uses a weight of 0 for sync wakeups on the grounds that it is assuming the waking task will sleep and not contribute to load in the near future. In this case, we still want to calculate the effective load of the sched entity hierarchy. As effective_load is no longer used by

Would it be worth mentioning that besides sync wakeups, wake_affine() uses a weight of 0 for the sched entity, for effective load calculation on the prev_cpu as well?
This is so as to find the effect of moving this task away from the prev_cpu. Here
too we are interested in calculating the effective load of the root task group of this
sched entity on the prev_cpu and the below restored check will be relevant.

Quoted 4 lines

Without the below check the difference in the loads of the wake affine CPU and the
prev_cpu can get messed up.

I was too slow getting to this mail unfortunately. The patch is already merged upstream with the changelog as-is.

Signature · 2 lines
Mel Gorman
SUSE Labs