LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] sched: Calculate effective load even if local weight is 0
@ 2014-01-06 11:39 Mel Gorman
  2014-01-06 12:34 ` Peter Zijlstra
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Mel Gorman @ 2014-01-06 11:39 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Thomas Hellstrom, Rik van Riel, linux-kernel

(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.

[mgorman@suse.de: Wrote changelog]
Reported-and-tested-by: Thomas Hellstrom <thellstrom@vmware.com>
Should-be-signed-off-and-authored-by-Rik
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c7395d9..e64b079 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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) {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-06 11:39 [PATCH] sched: Calculate effective load even if local weight is 0 Mel Gorman
@ 2014-01-06 12:34 ` Peter Zijlstra
  2014-01-06 13:09   ` Mel Gorman
  2014-01-06 12:35 ` Peter Zijlstra
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2014-01-06 12:34 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Thomas Hellstrom, Rik van Riel, linux-kernel

On Mon, Jan 06, 2014 at 11:39:12AM +0000, Mel Gorman wrote:
> (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 :-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-06 11:39 [PATCH] sched: Calculate effective load even if local weight is 0 Mel Gorman
  2014-01-06 12:34 ` Peter Zijlstra
@ 2014-01-06 12:35 ` Peter Zijlstra
  2014-01-06 14:00   ` Rik van Riel
  2014-01-12 18:42 ` [tip:sched/urgent] " tip-bot for Rik van Riel
  2014-01-13  7:52 ` [PATCH] " Preeti Murthy
  3 siblings, 1 reply; 8+ messages in thread
From: Peter Zijlstra @ 2014-01-06 12:35 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Thomas Hellstrom, Rik van Riel, linux-kernel

On Mon, Jan 06, 2014 at 11:39:12AM +0000, Mel Gorman wrote:
> 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?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-06 12:34 ` Peter Zijlstra
@ 2014-01-06 13:09   ` Mel Gorman
  0 siblings, 0 replies; 8+ messages in thread
From: Mel Gorman @ 2014-01-06 13:09 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Thomas Hellstrom, Rik van Riel, linux-kernel

On Mon, Jan 06, 2014 at 01:34:20PM +0100, Peter Zijlstra wrote:
> On Mon, Jan 06, 2014 at 11:39:12AM +0000, Mel Gorman wrote:
> > (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.

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-06 12:35 ` Peter Zijlstra
@ 2014-01-06 14:00   ` Rik van Riel
  0 siblings, 0 replies; 8+ messages in thread
From: Rik van Riel @ 2014-01-06 14:00 UTC (permalink / raw)
  To: Peter Zijlstra, Mel Gorman; +Cc: Thomas Hellstrom, linux-kernel

On 01/06/2014 07:35 AM, Peter Zijlstra wrote:
> On Mon, Jan 06, 2014 at 11:39:12AM +0000, Mel Gorman wrote:
>> 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 :)

Signed-off-by: Rik van Riel <riel@redhat.com>


-- 
All rights reversed

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tip:sched/urgent] sched: Calculate effective load even if local weight is 0
  2014-01-06 11:39 [PATCH] sched: Calculate effective load even if local weight is 0 Mel Gorman
  2014-01-06 12:34 ` Peter Zijlstra
  2014-01-06 12:35 ` Peter Zijlstra
@ 2014-01-12 18:42 ` tip-bot for Rik van Riel
  2014-01-13  7:52 ` [PATCH] " Preeti Murthy
  3 siblings, 0 replies; 8+ messages in thread
From: tip-bot for Rik van Riel @ 2014-01-12 18:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, peterz, thellstrom, riel, mgorman, tglx

Commit-ID:  9722c2dac708e9468cc0dc30218ef76946ffbc9d
Gitweb:     http://git.kernel.org/tip/9722c2dac708e9468cc0dc30218ef76946ffbc9d
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Mon, 6 Jan 2014 11:39:12 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
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.

Reported-and-tested-by: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
[ Wrote changelog]
Signed-off-by: Mel Gorman <mgorman@suse.de>
Signed-off-by: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20140106113912.GC6178@suse.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index c7395d9..e64b079 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -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) {

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-06 11:39 [PATCH] sched: Calculate effective load even if local weight is 0 Mel Gorman
                   ` (2 preceding siblings ...)
  2014-01-12 18:42 ` [tip:sched/urgent] " tip-bot for Rik van Riel
@ 2014-01-13  7:52 ` Preeti Murthy
  2014-01-17 14:56   ` Mel Gorman
  3 siblings, 1 reply; 8+ messages in thread
From: Preeti Murthy @ 2014-01-13  7:52 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Peter Zijlstra, Thomas Hellstrom, Rik van Riel, linux-kernel,
	Preeti U Murthy

Hi,

On Mon, Jan 6, 2014 at 5:09 PM, Mel Gorman <mgorman@suse.de> wrote:
> (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

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


> 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.
>
> [mgorman@suse.de: Wrote changelog]
> Reported-and-tested-by: Thomas Hellstrom <thellstrom@vmware.com>
> Should-be-signed-off-and-authored-by-Rik
> ---
>  kernel/sched/fair.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index c7395d9..e64b079 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] sched: Calculate effective load even if local weight is 0
  2014-01-13  7:52 ` [PATCH] " Preeti Murthy
@ 2014-01-17 14:56   ` Mel Gorman
  0 siblings, 0 replies; 8+ messages in thread
From: Mel Gorman @ 2014-01-17 14:56 UTC (permalink / raw)
  To: Preeti Murthy
  Cc: Peter Zijlstra, Thomas Hellstrom, Rik van Riel, linux-kernel,
	Preeti U Murthy

On Mon, Jan 13, 2014 at 01:22:40PM +0530, Preeti Murthy wrote:
> Hi,
> 
> On Mon, Jan 6, 2014 at 5:09 PM, Mel Gorman <mgorman@suse.de> wrote:
> > (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
> 
> 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.
> 

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

-- 
Mel Gorman
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-06 11:39 [PATCH] sched: Calculate effective load even if local weight is 0 Mel Gorman
2014-01-06 12:34 ` Peter Zijlstra
2014-01-06 13:09   ` Mel Gorman
2014-01-06 12:35 ` Peter Zijlstra
2014-01-06 14:00   ` Rik van Riel
2014-01-12 18:42 ` [tip:sched/urgent] " tip-bot for Rik van Riel
2014-01-13  7:52 ` [PATCH] " Preeti Murthy
2014-01-17 14:56   ` Mel Gorman

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git