public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Leo Yan <leo.yan@linaro.org>
To: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: Steve Muckle <steve.muckle@linaro.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	linux-kernel@vger.kernel.org, eas-dev@lists.linaro.org
Subject: Re: [PATCH RFC] sched/fair: let cpu's cfs_rq to reflect task migration
Date: Tue, 5 Apr 2016 14:56:44 +0800	[thread overview]
Message-ID: <20160405065644.GA29778@leoy-linaro> (raw)
In-Reply-To: <20160404084821.GA18516@e105550-lin.cambridge.arm.com>

On Mon, Apr 04, 2016 at 09:48:23AM +0100, Morten Rasmussen wrote:
> On Sat, Apr 02, 2016 at 03:11:54PM +0800, Leo Yan wrote:
> > On Fri, Apr 01, 2016 at 03:28:49PM -0700, Steve Muckle wrote:
> > > I think I follow - Leo please correct me if I mangle your intentions.
> > > It's an issue that Morten and Dietmar had mentioned to me as well.
> 
> Yes. We have been working on this issue for a while without getting to a
> nice solution yet.

Good to know this. This patch is mainly for discussion purpose.

[...]

> > > Leo I noticed you did not modify detach_entity_load_average(). I think
> > > this would be needed to avoid the task's stats being double counted for
> > > a while after switched_from_fair() or task_move_group_fair().
> 
> I'm afraid that the solution to problem is more complicated than that
> :-(
> 
> You are adding/removing a contribution from the root cfs_rq.avg which
> isn't part of the signal in the first place. The root cfs_rq.avg only
> contains the sum of the load/util of the sched_entities on the cfs_rq.
> If you remove the contribution of the tasks from there you may end up
> double-accounting for the task migration. Once due to you patch and then
> again slowly over time as the group sched_entity starts reflecting that
> the task has migrated. Furthermore, for group scheduling to make sense
> it has to be the task_h_load() you add/remove otherwise the group
> weighting is completely lost. Or am I completely misreading your patch?

Here have one thing want to confirm firstly: though CFS has maintained
task group's hierarchy, but between task group's cfs_rq.avg and root
cfs_rq.avg, CFS updates these signals independently rather than accouting
them by crossing the hierarchy.

So currently CFS decreases the group's cfs_rq.avg for task's migration,
but it don't iterate task group's hierarchy to root cfs_rq.avg. I
don't understand your meantioned the second accounting by "then again
slowly over time as the group sched_entity starts reflecting that the
task has migrated."

Another question is: does cfs_rq.avg _ONLY_ signal historic behavior but
not present behavior? so even the task has been migrated we still need
decay it slowly? Or this will be different between load and util?

> I don't think the slow response time for _load_ is necessarily a big
> problem. Otherwise we would have had people complaining already about
> group scheduling being broken. It is however a problem for all the
> initiatives that built on utilization.

Or maybe we need seperate utilization and load, these two signals
have different semantics and purpose.

Thanks,
Leo Yan

  parent reply	other threads:[~2016-04-05  6:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-01 16:38 [PATCH RFC] sched/fair: let cpu's cfs_rq to reflect task migration Leo Yan
2016-04-01 19:49 ` Peter Zijlstra
2016-04-01 22:28   ` Steve Muckle
2016-04-02  7:11     ` Leo Yan
2016-04-04  8:48       ` Morten Rasmussen
2016-04-04 18:30         ` Yuyang Du
2016-04-05  7:51           ` Morten Rasmussen
2016-04-05  0:15             ` Yuyang Du
2016-04-05 17:00               ` Dietmar Eggemann
2016-04-06  8:37                 ` Morten Rasmussen
2016-04-06 12:14                   ` Vincent Guittot
2016-04-06 18:53                   ` Dietmar Eggemann
2016-04-07 13:04                     ` Vincent Guittot
2016-04-07 20:30                       ` Dietmar Eggemann
2016-04-08  6:05                         ` Vincent Guittot
2016-04-05  6:56         ` Leo Yan [this message]
2016-04-05  9:13           ` Morten Rasmussen
2016-04-04  9:01 ` Morten Rasmussen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160405065644.GA29778@leoy-linaro \
    --to=leo.yan@linaro.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=eas-dev@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=steve.muckle@linaro.org \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox