lkml.org 
[lkml]   [2018]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [tip:sched/core] sched/numa: Delay retrying placement for automatic NUMA balance after wake_affine()
Hi Mel,

I do see performance improving with this commit 7347fc87df "sched/numa:
Delay retrying placement for automatic NUMA balance after wake_affine()"
even on powerpc where we have SD_WAKE_AFFINE *disabled* on numa sched
domains. Ideally this commit should not have affected powerpc machines.
That made me to look a bit deeper.

> @@ -1876,7 +1877,18 @@ static void numa_migrate_preferred(struct task_struct *p)
>
> /* Periodically retry migrating the task to the preferred node */
> interval = min(interval, msecs_to_jiffies(p->numa_scan_period) / 16);
> - p->numa_migrate_retry = jiffies + interval;
> + numa_migrate_retry = jiffies + interval;
> +
> + /*
> + * Check that the new retry threshold is after the current one. If
> + * the retry is in the future, it implies that wake_affine has
> + * temporarily asked NUMA balancing to backoff from placement.
> + */
> + if (numa_migrate_retry > p->numa_migrate_retry)
> + return;

The above check looks wrong. This check will most likely to be true,
numa_migrate_preferred() itself is called either when jiffies >
p->numa_migrate_retry or if the task's numa_preferred_nid has changed.

Hence we never end up calling task_numa_migrate() i.e we never go thro
the active cpu balancing path in numa balancing.

Reading the comments just above the check, makes me think the check
should have been

if (numa_migrate_retry < p->numa_migrate_retry)
return;

Here is perf stat output with 7347fc87df running perf bench numa mem
--no-data_rand_walk 96 -p 2 -t 48 -G 0 -P 3072 -T 0 -l 50 -c -s 1000

2,13,898 cs ( +- 2.65% )
10,228 migrations ( +- 14.61% )
21,86,406 faults ( +- 9.69% )
40,65,84,68,026 cache-misses ( +- 0.31% )
0 sched:sched_move_numa <---------------
0 sched:sched_stick_numa <---------------
0 sched:sched_swap_numa <---------------
1,41,780 migrate:mm_migrate_pages ( +- 24.11% )
0 migrate:mm_numa_migrate_ratelimit

778.331602169 seconds time elapsed


If you look at sched_move_numa, sched_stick_numa, sched_swap_numa
numbers, its very clear that we did try any active cpu migrations.

Same command with the commit reverted

2,38,685 cs ( +- 2.93% )
25,127 migrations ( +- 13.22% )
17,27,858 faults ( +- 2.61% )
34,77,06,21,298 cache-misses ( +- 0.61% )
560 sched:sched_move_numa ( +- 2.05% )
16 sched:sched_stick_numa ( +- 33.33% )
310 sched:sched_swap_numa ( +- 15.16% )
1,25,062 migrate:mm_migrate_pages ( +- 0.91% )
0 migrate:mm_numa_migrate_ratelimit

916.777315465 seconds time elapsed

(numbers are almost same with just that check commented/modified)

So we are seeing an improvement, but the improvement is because of
bypassing the active cpu balancing. Do we really want to by-pass this
code?

> +
> + /* Safe to try placing the task on the preferred node */
> + p->numa_migrate_retry = numa_migrate_retry;
>
> /* Success if task is already running on preferred CPU */
> if (task_node(p) == p->numa_preferred_nid)
> @@ -5759,6 +5771,48 @@ wake_affine_weight(struct sched_domain *sd, struct task_struct *p,

\
 
 \ /
  Last update: 2018-05-07 13:06    [W:0.291 / U:5.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site