Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1673685 > unrolled thread

[PATCH 0/4] NUMA improvements with task wakeup and load balancing

Started byriel@redhat.com
First post2017-06-23 19:00 +0200
Last post2017-06-24 09:30 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] NUMA improvements with task wakeup and load balancing riel@redhat.com - 2017-06-23 19:00 +0200
    [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing riel@redhat.com - 2017-06-23 19:00 +0200
      Re: [PATCH 1/4] sched,numa: override part of  migrate_degrades_locality when idle balancing Ingo Molnar <mingo@kernel.org> - 2017-06-24 09:00 +0200
        Re: [PATCH 1/4] sched,numa: override part of  migrate_degrades_locality when idle balancing Rik van Riel <riel@redhat.com> - 2017-06-25 01:50 +0200
      [tip:sched/core] sched/numa: Override part of  migrate_degrades_locality() when idle balancing tip-bot for Rik van Riel <tipbot@zytor.com> - 2017-06-24 09:30 +0200
    [PATCH 2/4] sched: simplify wake_affine for single socket case riel@redhat.com - 2017-06-23 19:00 +0200
      [tip:sched/core] sched/fair: Simplify wake_affine() for the single  socket case tip-bot for Rik van Riel <tipbot@zytor.com> - 2017-06-24 09:30 +0200

#1673685 — [PATCH 0/4] NUMA improvements with task wakeup and load balancing

Fromriel@redhat.com
Date2017-06-23 19:00 +0200
Subject[PATCH 0/4] NUMA improvements with task wakeup and load balancing
Message-ID<tVA65-4Fr-3@gated-at.bofh.it>
With these patches, and Peter Zijlstra's select_idle_sibling
scalability improvement, Jirka has seen these performance
gains on a 4.11 kernel:

NAS shows improvements in range 20-100%
SPECjbb2005 shows improvements around 6-8% in the single instance mode
SPECjvm2008 - improvements around 10%

Unfortunately the full set of tests takes about a week to
run, so numbers are not broken out for individual patches.

We have done previous runs with other scheduler changes,
which did not work out - they showed improvements on some
workloads, and regressions on others.

4.11 performance still lags behind 3.10 for some workloads.
I am trying to figure out why, and close that gap.

Diffstat:

 fair.c |  271 +++++++++++++++++++++--------------------------------------------
 1 file changed, 88 insertions(+), 183 deletions(-)

[toc] | [next] | [standalone]

#1673687 — [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing

Fromriel@redhat.com
Date2017-06-23 19:00 +0200
Subject[PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing
Message-ID<tVA66-4Fr-21@gated-at.bofh.it>
In reply to#1673685
From: Rik van Riel <riel@redhat.com>

Several tests in the NAS benchmark seem to run a lot slower with
NUMA balancing enabled, than with NUMA balancing disabled. The
slower run time corresponds with increased idle time.

Overriding the final test of migrate_degrades_locality (but still
doing the other NUMA tests first) seems to improve performance
of those benchmarks.

Reported-by: Jirka Hladky <jhladky@redhat.com>
Signed-off-by: Rik van Riel <riel@surriel.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/sched/fair.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2a0e71034e36..2180c8591e16 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6635,6 +6635,10 @@ static int migrate_degrades_locality(struct task_struct *p, struct lb_env *env)
 	if (dst_nid == p->numa_preferred_nid)
 		return 0;
 
+	/* Leaving a core idle is often worse than degrading locality. */
+	if (env->idle != CPU_NOT_IDLE)
+		return -1;
+
 	if (numa_group) {
 		src_faults = group_faults(p, src_nid);
 		dst_faults = group_faults(p, dst_nid);
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1674036 — Re: [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing

FromIngo Molnar <mingo@kernel.org>
Date2017-06-24 09:00 +0200
SubjectRe: [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing
Message-ID<tVNcZ-4u6-3@gated-at.bofh.it>
In reply to#1673687
* riel@redhat.com <riel@redhat.com> wrote:

> From: Rik van Riel <riel@redhat.com>
> 
> Several tests in the NAS benchmark seem to run a lot slower with
> NUMA balancing enabled, than with NUMA balancing disabled. The
> slower run time corresponds with increased idle time.
> 
> Overriding the final test of migrate_degrades_locality (but still
> doing the other NUMA tests first) seems to improve performance
> of those benchmarks.
> 
> Reported-by: Jirka Hladky <jhladky@redhat.com>
> Signed-off-by: Rik van Riel <riel@surriel.com>
> Signed-off-by: Rik van Riel <riel@redhat.com>

Note, I removed the first SOB from the commit, assuming that you only meant to 
include the second one?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1674157 — Re: [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing

FromRik van Riel <riel@redhat.com>
Date2017-06-25 01:50 +0200
SubjectRe: [PATCH 1/4] sched,numa: override part of migrate_degrades_locality when idle balancing
Message-ID<tW2Yp-5Q1-9@gated-at.bofh.it>
In reply to#1674036
On Sat, 2017-06-24 at 08:58 +0200, Ingo Molnar wrote:
> * riel@redhat.com <riel@redhat.com> wrote:
> 
> > From: Rik van Riel <riel@redhat.com>
> > 
> > Several tests in the NAS benchmark seem to run a lot slower with
> > NUMA balancing enabled, than with NUMA balancing disabled. The
> > slower run time corresponds with increased idle time.
> > 
> > Overriding the final test of migrate_degrades_locality (but still
> > doing the other NUMA tests first) seems to improve performance
> > of those benchmarks.
> > 
> > Reported-by: Jirka Hladky <jhladky@redhat.com>
> > Signed-off-by: Rik van Riel <riel@surriel.com>
> > Signed-off-by: Rik van Riel <riel@redhat.com>
> 
> Note, I removed the first SOB from the commit, assuming that you only
> meant to 
> include the second one?

Thank you. It looks like I used the wrong email address
in the SOB I wrote in the patch, and git added the second
one automatically. The second one is indeed the one I want.

[toc] | [prev] | [next] | [standalone]

#1674047 — [tip:sched/core] sched/numa: Override part of migrate_degrades_locality() when idle balancing

Fromtip-bot for Rik van Riel <tipbot@zytor.com>
Date2017-06-24 09:30 +0200
Subject[tip:sched/core] sched/numa: Override part of migrate_degrades_locality() when idle balancing
Message-ID<tVNG2-4Tg-15@gated-at.bofh.it>
In reply to#1673687
Commit-ID:  739294fb03f590401bbd7faa6d31a507e3ffada5
Gitweb:     http://git.kernel.org/tip/739294fb03f590401bbd7faa6d31a507e3ffada5
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 23 Jun 2017 12:55:27 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 24 Jun 2017 08:57:46 +0200

sched/numa: Override part of migrate_degrades_locality() when idle balancing

Several tests in the NAS benchmark seem to run a lot slower with
NUMA balancing enabled, than with NUMA balancing disabled. The
slower run time corresponds with increased idle time.

Overriding the final test of migrate_degrades_locality (but still
doing the other NUMA tests first) seems to improve performance
of those benchmarks.

Reported-by: Jirka Hladky <jhladky@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/20170623165530.22514-2-riel@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/fair.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 694c258..6e0c052 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6688,6 +6688,10 @@ static int migrate_degrades_locality(struct task_struct *p, struct lb_env *env)
 	if (dst_nid == p->numa_preferred_nid)
 		return 0;
 
+	/* Leaving a core idle is often worse than degrading locality. */
+	if (env->idle != CPU_NOT_IDLE)
+		return -1;
+
 	if (numa_group) {
 		src_faults = group_faults(p, src_nid);
 		dst_faults = group_faults(p, dst_nid);

[toc] | [prev] | [next] | [standalone]

#1673688 — [PATCH 2/4] sched: simplify wake_affine for single socket case

Fromriel@redhat.com
Date2017-06-23 19:00 +0200
Subject[PATCH 2/4] sched: simplify wake_affine for single socket case
Message-ID<tVA66-4Fr-23@gated-at.bofh.it>
In reply to#1673685
From: Rik van Riel <riel@redhat.com>

Then this_cpu and prev_cpu are in the same socket, select_idle_sibling
will do its thing regardless of the return value of wake_affine. Just
return true and don't look at all the other things.

Signed-off-by: Rik van Riel <riel@redhat.com>
---
 kernel/sched/fair.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 2180c8591e16..949de24e36bd 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5373,6 +5373,13 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p,
 	this_load = target_load(this_cpu, idx);
 
 	/*
+	 * Common case: CPUs are in the same socket, and select_idle_sibling
+	 * will do its thing regardless of what we return.
+	 */
+	if (cpus_share_cache(prev_cpu, this_cpu))
+		return true;
+
+	/*
 	 * If sync wakeup then subtract the (maximum possible)
 	 * effect of the currently running task from the load
 	 * of the current CPU:
@@ -5960,11 +5967,15 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int sd_flag, int wake_f
 
 	if (affine_sd) {
 		sd = NULL; /* Prefer wake_affine over balance flags */
-		if (cpu != prev_cpu && wake_affine(affine_sd, p, prev_cpu, sync))
+		if (cpu == prev_cpu)
+			goto pick_cpu;
+
+		if (wake_affine(affine_sd, p, prev_cpu, sync))
 			new_cpu = cpu;
 	}
 
 	if (!sd) {
+ pick_cpu:
 		if (sd_flag & SD_BALANCE_WAKE) /* XXX always ? */
 			new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);
 
-- 
2.9.4

[toc] | [prev] | [next] | [standalone]

#1674043 — [tip:sched/core] sched/fair: Simplify wake_affine() for the single socket case

Fromtip-bot for Rik van Riel <tipbot@zytor.com>
Date2017-06-24 09:30 +0200
Subject[tip:sched/core] sched/fair: Simplify wake_affine() for the single socket case
Message-ID<tVNG2-4Tg-3@gated-at.bofh.it>
In reply to#1673688
Commit-ID:  7d894e6e34a5cdd12309c7e4a3f830277ad4b7bf
Gitweb:     http://git.kernel.org/tip/7d894e6e34a5cdd12309c7e4a3f830277ad4b7bf
Author:     Rik van Riel <riel@redhat.com>
AuthorDate: Fri, 23 Jun 2017 12:55:28 -0400
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 24 Jun 2017 08:57:52 +0200

sched/fair: Simplify wake_affine() for the single socket case

Then 'this_cpu' and 'prev_cpu' are in the same socket, select_idle_sibling()
will do its thing regardless of the return value of wake_affine().

Just return true and don't look at all the other things.

Signed-off-by: Rik van Riel <riel@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: jhladky@redhat.com
Cc: linux-kernel@vger.kernel.org
Link: http://lkml.kernel.org/r/20170623165530.22514-3-riel@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/fair.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 6e0c052..fe19016 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5420,6 +5420,13 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p,
 	this_load = target_load(this_cpu, idx);
 
 	/*
+	 * Common case: CPUs are in the same socket, and select_idle_sibling()
+	 * will do its thing regardless of what we return:
+	 */
+	if (cpus_share_cache(prev_cpu, this_cpu))
+		return true;
+
+	/*
 	 * If sync wakeup then subtract the (maximum possible)
 	 * effect of the currently running task from the load
 	 * of the current CPU:
@@ -6007,11 +6014,15 @@ select_task_rq_fair(struct task_struct *p, int prev_cpu, int sd_flag, int wake_f
 
 	if (affine_sd) {
 		sd = NULL; /* Prefer wake_affine over balance flags */
-		if (cpu != prev_cpu && wake_affine(affine_sd, p, prev_cpu, sync))
+		if (cpu == prev_cpu)
+			goto pick_cpu;
+
+		if (wake_affine(affine_sd, p, prev_cpu, sync))
 			new_cpu = cpu;
 	}
 
 	if (!sd) {
+ pick_cpu:
 		if (sd_flag & SD_BALANCE_WAKE) /* XXX always ? */
 			new_cpu = select_idle_sibling(p, prev_cpu, new_cpu);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web