All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuyang Du <yuyang.du@intel.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Mike Galbraith <umgwanakikbuti@gmail.com>,
	Benjamin Segall <bsegall@google.com>,
	Paul Turner <pjt@google.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Matt Fleming <matt@codeblueprint.co.uk>
Subject: Re: [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks
Date: Tue, 21 Jun 2016 12:51:39 +0800	[thread overview]
Message-ID: <20160621045139.GE19934@intel.com> (raw)
In-Reply-To: <20160621084119.GN30154@twins.programming.kicks-ass.net>

On Tue, Jun 21, 2016 at 10:41:19AM +0200, Peter Zijlstra wrote:
> On Mon, Jun 20, 2016 at 03:49:34PM +0100, Dietmar Eggemann wrote:
> > On 20/06/16 13:35, Vincent Guittot wrote:
> 
> > > It will go through wake_up_new_task and post_init_entity_util_avg
> > > during its fork which is enough to set last_update_time. Then, it will
> > > use the switched_to_fair if the task becomes a fair one
> > 
> > Oh I see. We want to make sure that every task (even when forked as
> > !fair) has a last_update_time value != 0, when becoming fair one day.
> 
> Right, see 2 below. I need to write a bunch of comments explaining PELT
> proper, as well as document these things.
> 
> The things we ran into with these patches were that:
> 
>  1) You need to update the cfs_rq _before_ any entity attach/detach
>     (and might need to update_tg_load_avg when update_cfs_rq_load_avg()
>     returns true).

This is intrinsically an additional update, not a fix to anything. I
don't think it is a must, but I am fine with it.
 
>  2) (fair) entities are always attached, switched_from/to deal with !fair.
 
Yes, me too.

>  3) cpu migration is the only exception and uses the last_update_time=0
>     thing -- because refusal to take second rq->lock.

Task's last_update_time means this task is detached from fair queue. This
(re)definition is by all means much better than migrating. No?

> Which is why I dislike Yuyang's patches, they create more exceptions
> instead of applying existing rules (albeit undocumented).
> 
> Esp. 1 is important, because while for mathematically consistency you
> don't actually need to do this, you only need the entities to be
> up-to-date with the cfs rq when you attach/detach, but that forgets the
> temporal aspect of _when_ you do this.
 
Yes, temporally at any instant the avgs are outdated. But, I can have it,
and what if I have it?

I am thinking about document this really well, like "An art of load tracking:
accuracy, overhead, and usefulness", seriously.

  reply	other threads:[~2016-06-21 12:48 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-17 12:01 [PATCH 0/4] sched/fair: Fix PELT wobblies Peter Zijlstra
2016-06-17 12:01 ` [PATCH 1/4] sched: Optimize fork() paths Peter Zijlstra
2016-06-17 12:01 ` [PATCH 2/4] sched/fair: Fix PELT integrity for new groups Peter Zijlstra
2016-06-17 13:51   ` Vincent Guittot
2016-06-17 12:01 ` [PATCH 3/4] sched,cgroup: Fix cpu_cgroup_fork() Peter Zijlstra
2016-06-17 13:58   ` Vincent Guittot
2016-06-17 14:06     ` Peter Zijlstra
2016-06-17 12:01 ` [PATCH 4/4] sched,fair: Fix PELT integrity for new tasks Peter Zijlstra
2016-06-17 14:09   ` Vincent Guittot
2016-06-17 14:28     ` Peter Zijlstra
2016-06-17 16:02       ` Peter Zijlstra
2016-06-17 16:14         ` Vincent Guittot
2016-06-17 16:18         ` Peter Zijlstra
2016-06-19 22:55           ` Yuyang Du
2016-06-20  9:23           ` Vincent Guittot
2016-06-20  9:52             ` Peter Zijlstra
2016-06-20 10:07               ` Vincent Guittot
2016-06-21 11:43             ` Peter Zijlstra
2016-06-21 12:36               ` Vincent Guittot
2016-06-21 12:47                 ` Peter Zijlstra
2016-06-21 12:56                   ` Vincent Guittot
2016-06-20 11:35           ` Dietmar Eggemann
2016-06-20 12:35             ` Vincent Guittot
2016-06-20 14:49               ` Dietmar Eggemann
2016-06-21  8:41                 ` Peter Zijlstra
2016-06-21  4:51                   ` Yuyang Du [this message]
2016-06-24 13:03                     ` Peter Zijlstra
2016-08-09 23:19                       ` Yuyang Du
2016-06-21 13:17                   ` Peter Zijlstra
2016-06-21 13:29                     ` Vincent Guittot
2016-06-22 11:46                       ` Peter Zijlstra
2016-06-23 15:35                   ` Dietmar Eggemann
2016-06-23 17:15                     ` Peter Zijlstra
2016-06-20 14:27             ` Peter Zijlstra
2016-06-23 11:19   ` Peter Zijlstra
2016-08-01  7:30   ` Wanpeng Li
2016-08-01  9:31     ` Mike Galbraith
2016-08-01  9:56       ` Wanpeng Li
2016-08-01 11:52         ` Mike Galbraith

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=20160621045139.GE19934@intel.com \
    --to=yuyang.du@intel.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=umgwanakikbuti@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.