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

Groups > linux.kernel > #1629973 > unrolled thread

[RFC PATCHSET] sched/fair: fix load balancer behavior when cgroup is in use

Back to article view | Back to linux.kernel

Contents

  [RFC PATCHSET] sched/fair: fix load balancer behavior when cgroup is  in use Tejun Heo <tj@kernel.org> - 2017-04-24 22:20 +0200
    [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq  sched_entities Tejun Heo <tj@kernel.org> - 2017-04-24 23:40 +0200
      Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq  sched_entities Peter Zijlstra <peterz@infradead.org> - 2017-04-24 23:50 +0200
        Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq  sched_entities Tejun Heo <tj@kernel.org> - 2017-04-25 01:00 +0200
      Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq  sched_entities Tejun Heo <tj@kernel.org> - 2017-04-25 23:20 +0200

#1629973 — [RFC PATCHSET] sched/fair: fix load balancer behavior when cgroup is in use

Hello,

We've noticed scheduling latency spike when cgroup is in use even when
the machine is idle enough with moderate scheduling frequency and
single level of cgroup nesting.  More details are in the patch
descriptions but here's a schbench run from the root cgroup.

 # ~/schbench -m 2 -t 16 -s 10000 -c 15000 -r 30
 Latency percentiles (usec)
	 50.0000th: 26
	 75.0000th: 62
	 90.0000th: 74
	 95.0000th: 86
	 *99.0000th: 887
	 99.5000th: 3692
	 99.9000th: 10832
	 min=0, max=13374

And here's one from inside a first level cgroup.

 # ~/schbench -m 2 -t 16 -s 10000 -c 15000 -r 30
 Latency percentiles (usec)
	 50.0000th: 31
	 75.0000th: 65
	 90.0000th: 71
	 95.0000th: 91
	 *99.0000th: 7288
	 99.5000th: 10352
	 99.9000th: 12496
	 min=0, max=13023

The p99 latency spike got tracked down to runnable_load_avg not being
propagated through nested cfs_rqs and thus load_balance() operating on
out-of-sync load information.  It ended up picking the wrong CPU as
load balance target often enough to significantly impact p99 latency.

This patchset fixes the issue by always propagating runnable_load_avg
so that, regardless of nesting, every cfs_rq's runnable_load_avg is
the sum of the scaled loads of all tasks queued below it.

As a side effect, this changes the load_avg behavior of sched_entities
associated cfs_rq's.  It doesn't seem wrong to me and I can't think of
a better / cleaner way, but if there is, please let me know.

This patchset is on top of v4.11-rc8 and contains the following two
patches.

 sched/fair: Fix how load gets propagated from cfs_rq to its sched_entity
 sched/fair: Always propagate runnable_load_avg

diffstat follows.

 kernel/sched/fair.c |   46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

Thanks.

-- 
tejun

[toc] | [next] | [standalone]

#1630043 — [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq sched_entities

Now that a cfs_rq sched_entity's load_avg always gets propagated from
the associated cfs_rq, there's no point in calling __update_load_avg()
on it.  The two mechanisms compete with each other and we'd be always
using a value close to the propagated one anyway.

Skip __update_load_avg() for cfs_rq sched_entities.  Also, relocate
propagate_entity_load_avg() to signify that propagation is the
counterpart to __update_load_avg() for cfs_rq sched_entities.  This
puts the propagation before update_cfs_rq_load_avg() which shouldn't
disturb anything.

Signed-off-by: Tejun Heo <tj@kernel.org>
Cc: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Turner <pjt@google.com>
---
Hello,

A follow-up patch.  This removes __update_load_avg() on cfs_rq se's as
the value is now constantly kept in sync from cfs_rq.  The patch
doesn't cause any noticable changes in tets.

Thanks.

 kernel/sched/fair.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -3293,20 +3293,22 @@ static inline void update_load_avg(struc
 	u64 now = cfs_rq_clock_task(cfs_rq);
 	struct rq *rq = rq_of(cfs_rq);
 	int cpu = cpu_of(rq);
-	int decayed;
+	int decayed = 0;
 
 	/*
 	 * Track task load average for carrying it to new CPU after migrated, and
 	 * track group sched_entity load average for task_h_load calc in migration
 	 */
-	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD)) {
-		__update_load_avg(now, cpu, &se->avg,
-			  se->on_rq * scale_load_down(se->load.weight),
-			  cfs_rq->curr == se, NULL);
+	if (entity_is_task(se)) {
+		if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
+			__update_load_avg(now, cpu, &se->avg,
+					  se->on_rq * scale_load_down(se->load.weight),
+					  cfs_rq->curr == se, NULL);
+	} else {
+		decayed |= propagate_entity_load_avg(se);
 	}
 
-	decayed  = update_cfs_rq_load_avg(now, cfs_rq, true);
-	decayed |= propagate_entity_load_avg(se);
+	decayed |= update_cfs_rq_load_avg(now, cfs_rq, true);
 
 	if (decayed && (flags & UPDATE_TG))
 		update_tg_load_avg(cfs_rq, 0);

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

#1630060 — Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq sched_entities

On Mon, Apr 24, 2017 at 02:35:28PM -0700, Tejun Heo wrote:
> -	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD)) {
> -		__update_load_avg(now, cpu, &se->avg,
> -			  se->on_rq * scale_load_down(se->load.weight),
> -			  cfs_rq->curr == se, NULL);
> +	if (entity_is_task(se)) {
> +		if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
> +			__update_load_avg(now, cpu, &se->avg,
> +					  se->on_rq * scale_load_down(se->load.weight),
> +					  cfs_rq->curr == se, NULL);

I've not looked at these patches yet, but you've been patching old code.
__update_load_avg() no longer exists (the conversion shouldn't be too
hard, its mostly been a restructure/rename thing).

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

#1630100 — Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq sched_entities

Hello, Peter.

On Mon, Apr 24, 2017 at 11:48:59PM +0200, Peter Zijlstra wrote:
> On Mon, Apr 24, 2017 at 02:35:28PM -0700, Tejun Heo wrote:
> > -	if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD)) {
> > -		__update_load_avg(now, cpu, &se->avg,
> > -			  se->on_rq * scale_load_down(se->load.weight),
> > -			  cfs_rq->curr == se, NULL);
> > +	if (entity_is_task(se)) {
> > +		if (se->avg.last_update_time && !(flags & SKIP_AGE_LOAD))
> > +			__update_load_avg(now, cpu, &se->avg,
> > +					  se->on_rq * scale_load_down(se->load.weight),
> > +					  cfs_rq->curr == se, NULL);
> 
> I've not looked at these patches yet, but you've been patching old code.
> __update_load_avg() no longer exists (the conversion shouldn't be too
> hard, its mostly been a restructure/rename thing).

Ah, sure.  The patchset still being RFC, I wanted to post the version
I was working with.  If you want the patchset refreshed now, please
let me know.

Thanks.

-- 
tejun

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

#1631001 — Re: [PATCH 3/2] sched/fair: Skip __update_load_avg() on cfs_rq sched_entities

On Mon, Apr 24, 2017 at 02:35:28PM -0700, Tejun Heo wrote:
> Now that a cfs_rq sched_entity's load_avg always gets propagated from
> the associated cfs_rq, there's no point in calling __update_load_avg()
> on it.  The two mechanisms compete with each other and we'd be always
> using a value close to the propagated one anyway.
> 
> Skip __update_load_avg() for cfs_rq sched_entities.  Also, relocate
> propagate_entity_load_avg() to signify that propagation is the
> counterpart to __update_load_avg() for cfs_rq sched_entities.  This
> puts the propagation before update_cfs_rq_load_avg() which shouldn't
> disturb anything.

Please ignore this patch.  As we don't propagate on decays, we still
need __update_load_avg() on runanble_load_avg so that it can decay on
its own.

Thanks.

-- 
tejun

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web