diff mbox series

[2/2] mm, numa: Migrate pages to local nodes quicker early in the lifetime of a task

Message ID 20181001100525.29789-3-mgorman@techsingularity.net (mailing list archive)
State New, archived
Headers show
Series Faster migration for automatic NUMA balancing | expand

Commit Message

Mel Gorman Oct. 1, 2018, 10:05 a.m. UTC
Automatic NUMA Balancing uses a multi-stage pass to decide whether a page
should migrate to a local node. This filter avoids excessive ping-ponging
if a page is shared or used by threads that migrate cross-node frequently.

Threads inherit both page tables and the preferred node ID from the
parent. This means that threads can trigger hinting faults earlier than
a new task which delays scanning for a number of seconds. As it can be
load balanced very early in its lifetime there can be an unnecessary delay
before it starts migrating thread-local data. This patch migrates private
pages faster early in the lifetime of a thread using the sequence counter
as an identifier of new tasks.

With this patch applied, STREAM performance is the same as 4.17 even though
processes are not spread cross-node prematurely. Other workloads showed
a mix of minor gains and losses. This is somewhat expected most workloads
are not very sensitive to the starting conditions of a process.

                         4.19.0-rc5             4.19.0-rc5                 4.17.0
                         numab-v1r1       fastmigrate-v1r1                vanilla
MB/sec copy     43298.52 (   0.00%)    47335.46 (   9.32%)    47219.24 (   9.06%)
MB/sec scale    30115.06 (   0.00%)    32568.12 (   8.15%)    32527.56 (   8.01%)
MB/sec add      32825.12 (   0.00%)    36078.94 (   9.91%)    35928.02 (   9.45%)
MB/sec triad    32549.52 (   0.00%)    35935.94 (  10.40%)    35969.88 (  10.51%)

Signed-off-by: Mel Gorman <mgorman@techsingularity.net>
---
 kernel/sched/fair.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Rik van Riel Oct. 1, 2018, 3:41 p.m. UTC | #1
On Mon, 2018-10-01 at 11:05 +0100, Mel Gorman wrote:
> With this patch applied, STREAM performance is the same as 4.17 even
> though
> processes are not spread cross-node prematurely. Other workloads
> showed
> a mix of minor gains and losses. This is somewhat expected most
> workloads
> are not very sensitive to the starting conditions of a process.
> 
>                          4.19.0-rc5             4.19.0-
> rc5                 4.17.0
>                          numab-v1r1       fastmigrate-
> v1r1                vanilla
> MB/sec copy     43298.52 (   0.00%)    47335.46
> (   9.32%)    47219.24 (   9.06%)
> MB/sec scale    30115.06 (   0.00%)    32568.12
> (   8.15%)    32527.56 (   8.01%)
> MB/sec add      32825.12 (   0.00%)    36078.94
> (   9.91%)    35928.02 (   9.45%)
> MB/sec triad    32549.52 (   0.00%)    35935.94
> (  10.40%)    35969.88 (  10.51%)
> 
> Signed-off-by: Mel Gorman <mgorman@techsingularity.net>

Reviewed-by: Rik van Riel <riel@surriel.com>
Srikar Dronamraju Oct. 2, 2018, 12:41 p.m. UTC | #2
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 25c7c7e09cbd..7fc4a371bdd2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1392,6 +1392,17 @@ bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
>  	int last_cpupid, this_cpupid;
>
>  	this_cpupid = cpu_pid_to_cpupid(dst_cpu, current->pid);
> +	last_cpupid = page_cpupid_xchg_last(page, this_cpupid);
> +
> +	/*
> +	 * Allow first faults or private faults to migrate immediately early in
> +	 * the lifetime of a task. The magic number 4 is based on waiting for
> +	 * two full passes of the "multi-stage node selection" test that is
> +	 * executed below.
> +	 */
> +	if ((p->numa_preferred_nid == -1 || p->numa_scan_seq <= 4) &&
> +	    (cpupid_pid_unset(last_cpupid) || cpupid_match_pid(p, last_cpupid)))
> +		return true;
>

This does have issues when using with workloads that access more shared faults
than private faults.

In such workloads, this change would spread the memory causing regression in
behaviour.

5 runs of on 2 socket/ 4 node power 8 box


Without this patch
./numa01.sh      Real:  382.82    454.29    422.31    29.72
./numa01.sh      Sys:   40.12     74.53     58.50     13.37
./numa01.sh      User:  34230.22  46398.84  40292.62  4915.93

With this patch
./numa01.sh      Real:  415.56    555.04    473.45    51.17    -10.8016%
./numa01.sh      Sys:   43.42     94.22     73.59     17.31    -20.5055%
./numa01.sh      User:  35271.95  56644.19  45615.72  7165.01  -11.6694%

Since we are looking at time, smaller numbers are better.

----------------------------------------
# cat numa01.sh
#! /bin/bash
# numa01.sh corresponds to 2 perf bench processes each having ncpus/2 threads
# 50 loops of 3G process memory.

THREADS=${THREADS:-$(($(getconf _NPROCESSORS_ONLN)/2))}
perf bench numa mem --no-data_rand_walk -p 2 -t $THREADS -G 0 -P 3072 -T 0 -l 50 -c -s 2000 $@
----------------------------------------

I know this is a synthetic benchmark, but wonder if benchmarks run on vm
guest show similar behaviour when noticed from host.

SPECJbb did show some small loss and gains.

Our numa grouping is not fast enough. It can take sometimes several
iterations before all the tasks belonging to the same group end up being
part of the group. With the current check we end up spreading memory faster
than we should hence hurting the chance of early consolidation.

Can we restrict to something like this?

if (p->numa_scan_seq >=MIN && p->numa_scan_seq <= MIN+4 &&
    (cpupid_match_pid(p, last_cpupid)))
	return true;

meaning, we ran atleast MIN number of scans, and we find the task to be most likely
task using this page.
Mel Gorman Oct. 2, 2018, 1:54 p.m. UTC | #3
On Tue, Oct 02, 2018 at 06:11:49PM +0530, Srikar Dronamraju wrote:
> >
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 25c7c7e09cbd..7fc4a371bdd2 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -1392,6 +1392,17 @@ bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
> >  	int last_cpupid, this_cpupid;
> >
> >  	this_cpupid = cpu_pid_to_cpupid(dst_cpu, current->pid);
> > +	last_cpupid = page_cpupid_xchg_last(page, this_cpupid);
> > +
> > +	/*
> > +	 * Allow first faults or private faults to migrate immediately early in
> > +	 * the lifetime of a task. The magic number 4 is based on waiting for
> > +	 * two full passes of the "multi-stage node selection" test that is
> > +	 * executed below.
> > +	 */
> > +	if ((p->numa_preferred_nid == -1 || p->numa_scan_seq <= 4) &&
> > +	    (cpupid_pid_unset(last_cpupid) || cpupid_match_pid(p, last_cpupid)))
> > +		return true;
> >
> 
> This does have issues when using with workloads that access more shared faults
> than private faults.
> 

Not as such. It can have issues on workloads where memory is initialised
by one thread, then additional threads are created and access the same
memory. They are not necessarily shared once buffers are handed over. In
such a case, migrating quickly is the right thing to do. If it's truely
shared pages then there may be some unnecessary migrations early in the
lifetime of the task but it'll settle down quickly enough.

> In such workloads, this change would spread the memory causing regression in
> behaviour.
> 
> 5 runs of on 2 socket/ 4 node power 8 box
> 
> 
> Without this patch
> ./numa01.sh      Real:  382.82    454.29    422.31    29.72
> ./numa01.sh      Sys:   40.12     74.53     58.50     13.37
> ./numa01.sh      User:  34230.22  46398.84  40292.62  4915.93
> 
> With this patch
> ./numa01.sh      Real:  415.56    555.04    473.45    51.17    -10.8016%
> ./numa01.sh      Sys:   43.42     94.22     73.59     17.31    -20.5055%
> ./numa01.sh      User:  35271.95  56644.19  45615.72  7165.01  -11.6694%
> 
> Since we are looking at time, smaller numbers are better.
> 

Is it just numa01 that was affected for you? I ask because that particular
workload is an averse workload on any machine with more than sockets and
your machine description says it has 4 nodes. What it is testing is quite
specific to 2-node machines.

> SPECJbb did show some small loss and gains.
> 

That almost always shows small gains and losses so that's not too
surprising.

> Our numa grouping is not fast enough. It can take sometimes several
> iterations before all the tasks belonging to the same group end up being
> part of the group. With the current check we end up spreading memory faster
> than we should hence hurting the chance of early consolidation.
> 
> Can we restrict to something like this?
> 
> if (p->numa_scan_seq >=MIN && p->numa_scan_seq <= MIN+4 &&
>     (cpupid_match_pid(p, last_cpupid)))
> 	return true;
> 
> meaning, we ran atleast MIN number of scans, and we find the task to be most likely
> task using this page.
> 

What's MIN? Assuming it's any type of delay, note that this will regress
STREAM again because it's very sensitive to the starting state.
Srikar Dronamraju Oct. 2, 2018, 5:30 p.m. UTC | #4
> > 
> > This does have issues when using with workloads that access more shared faults
> > than private faults.
> > 
> 
> Not as such. It can have issues on workloads where memory is initialised
> by one thread, then additional threads are created and access the same
> memory. They are not necessarily shared once buffers are handed over. In
> such a case, migrating quickly is the right thing to do. If it's truely
> shared pages then there may be some unnecessary migrations early in the
> lifetime of the task but it'll settle down quickly enough.
> 

Do you have a workload recommendation to try for shared fault accesses.
I will try to get a DayTrader run in a day or two. There JVM and db threads
act on the same memory, I presume it might show some insights.

> Is it just numa01 that was affected for you? I ask because that particular
> workload is an averse workload on any machine with more than sockets and
> your machine description says it has 4 nodes. What it is testing is quite
> specific to 2-node machines.
> 

Agree, 

Some variations of numa01.sh where I have one process having threads equal
to number of cpus does regress but not as much as numa01.

./numa03.sh      Real:  484.84    555.51    518.59    22.91    -5.84277%
./numa03.sh      Sys:   44.41     64.40     53.24     6.65     -11.3824%
./numa03.sh      User:  51328.77  59429.39  55366.62  2744.39  -9.47912%


> > SPECJbb did show some small loss and gains.
> > 
> 
> That almost always shows small gains and losses so that's not too
> surprising.
> 

Okay.

> > Our numa grouping is not fast enough. It can take sometimes several
> > iterations before all the tasks belonging to the same group end up being
> > part of the group. With the current check we end up spreading memory faster
> > than we should hence hurting the chance of early consolidation.
> > 
> > Can we restrict to something like this?
> > 
> > if (p->numa_scan_seq >=MIN && p->numa_scan_seq <= MIN+4 &&
> >     (cpupid_match_pid(p, last_cpupid)))
> > 	return true;
> > 
> > meaning, we ran atleast MIN number of scans, and we find the task to be most likely
> > task using this page.
> > 
> 


> What's MIN? Assuming it's any type of delay, note that this will regress
> STREAM again because it's very sensitive to the starting state.
> 

I was thinking of MIN as 3 to give a chance for things to settle.
but that might not help STREAM as you pointed out.

Do you have a hint on which commit made STREAM regress?

if we want to prioritize STREAM like workloads (i.e private faults) one simpler
fix could be to change the quadtraic equation

from:
	if (!cpupid_pid_unset(last_cpupid) &&
				cpupid_to_nid(last_cpupid) != dst_nid)
		return false;
to:
	if (!cpupid_pid_unset(last_cpupid) &&
				cpupid_to_nid(last_cpupid) == dst_nid)
		return true;

i.e to say if the group tasks likely consolidated to a node or the task was
moved to a different node but access were private, just move the memory.

The drawback though is we keep pulling memory everytime the task moves
across nodes. (which is probably restricted for long running tasks to some
extent by your fix)
Mel Gorman Oct. 2, 2018, 6:22 p.m. UTC | #5
On Tue, Oct 02, 2018 at 11:00:05PM +0530, Srikar Dronamraju wrote:
> > > 
> > > This does have issues when using with workloads that access more shared faults
> > > than private faults.
> > > 
> > 
> > Not as such. It can have issues on workloads where memory is initialised
> > by one thread, then additional threads are created and access the same
> > memory. They are not necessarily shared once buffers are handed over. In
> > such a case, migrating quickly is the right thing to do. If it's truely
> > shared pages then there may be some unnecessary migrations early in the
> > lifetime of the task but it'll settle down quickly enough.
> > 
> 
> Do you have a workload recommendation to try for shared fault accesses.

NAS parallelised with OMP tends to be ok but I haven't quantified if it's
perfect or a good example. I don't have an example of a workload that
is good at targetting the specific case where pages are shared between
tasks that tend to run on separate nodes. It would be somewhat of an
anti-pattern for any workload regardless of automatic NUMA balancing.

> > > <SNIP>
> > >
> > > Our numa grouping is not fast enough. It can take sometimes several
> > > iterations before all the tasks belonging to the same group end up being
> > > part of the group. With the current check we end up spreading memory faster
> > > than we should hence hurting the chance of early consolidation.
> > > 
> > > Can we restrict to something like this?
> > > 
> > > if (p->numa_scan_seq >=MIN && p->numa_scan_seq <= MIN+4 &&
> > >     (cpupid_match_pid(p, last_cpupid)))
> > > 	return true;
> > > 
> > > meaning, we ran atleast MIN number of scans, and we find the task to be most likely
> > > task using this page.
> > > 
> > 
> 
> 
> > What's MIN? Assuming it's any type of delay, note that this will regress
> > STREAM again because it's very sensitive to the starting state.
> > 
> 
> I was thinking of MIN as 3 to give a chance for things to settle.
> but that might not help STREAM as you pointed out.
> 

Probably not.

> Do you have a hint on which commit made STREAM regress?
> 

2c83362734da ("sched/fair: Consider SD_NUMA when selecting the most idle group to schedule on")

Reverting it hurts workloads that communicate immediately with new processes
or threads as workloads spread prematurely and then get pulled back just
after clone.

> if we want to prioritize STREAM like workloads (i.e private faults) one simpler
> fix could be to change the quadtraic equation
> 
> from:
> 	if (!cpupid_pid_unset(last_cpupid) &&
> 				cpupid_to_nid(last_cpupid) != dst_nid)
> 		return false;
> to:
> 	if (!cpupid_pid_unset(last_cpupid) &&
> 				cpupid_to_nid(last_cpupid) == dst_nid)
> 		return true;
> 
> i.e to say if the group tasks likely consolidated to a node or the task was
> moved to a different node but access were private, just move the memory.
> 
> The drawback though is we keep pulling memory everytime the task moves
> across nodes. (which is probably restricted for long running tasks to some
> extent by your fix)
> 

This has way more consequences as it changes the behaviour for the entire
lifetime of the workload. It could cause excessive migrations in the case
where a machine is almost fully utilised and getting load balanced or in
cases where tasks are pulled frequently cross-node (e.g. worker thread
model or a pipelined computation).

I'm only looking to address the case where the load balancer spreads a
workload early and the memory should move to the new node quickly. If it
turns out there are cases where that decision is wrong, it gets remedied
quickly but if your proposal is ever wrong, the system doesn't recover.
Srikar Dronamraju Oct. 3, 2018, 1:07 p.m. UTC | #6
* Srikar Dronamraju <srikar@linux.vnet.ibm.com> [2018-10-02 23:00:05]:

> I will try to get a DayTrader run in a day or two. There JVM and db threads
> act on the same memory, I presume it might show some insights.

I ran 2 runs of daytrader 7 with and without patch on a 2 node power9
PowerNv box.
https://github.com/WASdev/sample.daytrader7
In each run, has 8 JVMs.

Throughputs (Higher are better)
Without patch 19216.8 18900.7 Average: 19058.75
With patch    18644.5 18480.9 Average: 18562.70

Difference being -2.6% regression
Srikar Dronamraju Oct. 3, 2018, 1:15 p.m. UTC | #7
> > if we want to prioritize STREAM like workloads (i.e private faults) one simpler
> > fix could be to change the quadtraic equation
> > 
> > from:
> > 	if (!cpupid_pid_unset(last_cpupid) &&
> > 				cpupid_to_nid(last_cpupid) != dst_nid)
> > 		return false;
> > to:
> > 	if (!cpupid_pid_unset(last_cpupid) &&
> > 				cpupid_to_nid(last_cpupid) == dst_nid)
> > 		return true;
> > 
> > i.e to say if the group tasks likely consolidated to a node or the task was
> > moved to a different node but access were private, just move the memory.
> > 
> > The drawback though is we keep pulling memory everytime the task moves
> > across nodes. (which is probably restricted for long running tasks to some
> > extent by your fix)
> > 
> 
> This has way more consequences as it changes the behaviour for the entire
> lifetime of the workload. It could cause excessive migrations in the case
> where a machine is almost fully utilised and getting load balanced or in
> cases where tasks are pulled frequently cross-node (e.g. worker thread
> model or a pipelined computation).
> 
> I'm only looking to address the case where the load balancer spreads a
> workload early and the memory should move to the new node quickly. If it
> turns out there are cases where that decision is wrong, it gets remedied
> quickly but if your proposal is ever wrong, the system doesn't recover.
> 

Agree.
Mel Gorman Oct. 3, 2018, 1:21 p.m. UTC | #8
On Wed, Oct 03, 2018 at 06:37:41PM +0530, Srikar Dronamraju wrote:
> * Srikar Dronamraju <srikar@linux.vnet.ibm.com> [2018-10-02 23:00:05]:
> 
> > I will try to get a DayTrader run in a day or two. There JVM and db threads
> > act on the same memory, I presume it might show some insights.
> 
> I ran 2 runs of daytrader 7 with and without patch on a 2 node power9
> PowerNv box.
> https://github.com/WASdev/sample.daytrader7
> In each run, has 8 JVMs.
> 
> Throughputs (Higher are better)
> Without patch 19216.8 18900.7 Average: 19058.75
> With patch    18644.5 18480.9 Average: 18562.70
> 
> Difference being -2.6% regression
> 

That's unfortunate.

How much does this workload normally vary between runs? If you monitor
migrations over time, is there an increase spike in migration early in
the lifetime of the workload?
Srikar Dronamraju Oct. 3, 2018, 2:08 p.m. UTC | #9
* Mel Gorman <mgorman@techsingularity.net> [2018-10-03 14:21:55]:

> On Wed, Oct 03, 2018 at 06:37:41PM +0530, Srikar Dronamraju wrote:
> > * Srikar Dronamraju <srikar@linux.vnet.ibm.com> [2018-10-02 23:00:05]:
> > 
> 

> That's unfortunate.
> 
> How much does this workload normally vary between runs? If you monitor
> migrations over time, is there an increase spike in migration early in
> the lifetime of the workload?
> 

The run to run variation has always been less than 1%.
I haven't monitored migrations over time. Will try to include it my next
run. Its a shared setup so I may not get the box immediately.
diff mbox series

Patch

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 25c7c7e09cbd..7fc4a371bdd2 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1392,6 +1392,17 @@  bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
 	int last_cpupid, this_cpupid;
 
 	this_cpupid = cpu_pid_to_cpupid(dst_cpu, current->pid);
+	last_cpupid = page_cpupid_xchg_last(page, this_cpupid);
+
+	/*
+	 * Allow first faults or private faults to migrate immediately early in
+	 * the lifetime of a task. The magic number 4 is based on waiting for
+	 * two full passes of the "multi-stage node selection" test that is
+	 * executed below.
+	 */
+	if ((p->numa_preferred_nid == -1 || p->numa_scan_seq <= 4) &&
+	    (cpupid_pid_unset(last_cpupid) || cpupid_match_pid(p, last_cpupid)))
+		return true;
 
 	/*
 	 * Multi-stage node selection is used in conjunction with a periodic
@@ -1410,7 +1421,6 @@  bool should_numa_migrate_memory(struct task_struct *p, struct page * page,
 	 * This quadric squishes small probabilities, making it less likely we
 	 * act on an unlikely task<->page relation.
 	 */
-	last_cpupid = page_cpupid_xchg_last(page, this_cpupid);
 	if (!cpupid_pid_unset(last_cpupid) &&
 				cpupid_to_nid(last_cpupid) != dst_nid)
 		return false;