All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuyang Du <yuyang.du@intel.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Benjamin Segall <bsegall@google.com>,
	Paul Turner <pjt@google.com>,
	Morten Rasmussen <morten.rasmussen@arm.com>,
	Dietmar Eggemann <dietmar.eggemann@arm.com>,
	byungchul.park@lge.com
Subject: Re: [PATCH 2/2] sched/fair: Skip detach and attach load avgs for new group task
Date: Mon, 30 May 2016 09:33:37 +0800	[thread overview]
Message-ID: <20160530013336.GL18670@intel.com> (raw)
In-Reply-To: <CAKfTPtAV_Fic5RWzzvehT-yOQ1t+sxqrXUFZM1E7uDrwOS00og@mail.gmail.com>

Hi Vincent,

On Fri, May 27, 2016 at 03:37:11PM +0200, Vincent Guittot wrote:
> On 26 May 2016 at 21:44, Yuyang Du <yuyang.du@intel.com> wrote:
> > Hi Vincent,
> >
> > On Thu, May 26, 2016 at 01:50:56PM +0200, Vincent Guittot wrote:
> >> On 26 May 2016 at 03:14, Yuyang Du <yuyang.du@intel.com> wrote:
> >> > Vincent reported that the first task to a new task group's cfs_rq will
> >> > be attached in attach_task_cfs_rq() and once more when it is enqueued
> >> > (see https://lkml.org/lkml/2016/5/25/388).
> >> >
> >> > Actually, it is worse, attach_task_cfs_rq() is called for new task even
> >> > way before init_entity_runnable_average().
> >> >
> >> > Solve this by avoiding attach as well as detach new task's sched avgs
> >> > in task_move_group_fair(). To do it, we need to know whether the task
> >> > is forked or not, so we pass this info all the way from sched_move_task()
> >> > to attach_task_cfs_rq().
> >>
> >> Not sure that this is the right way to solve this problem because you
> >> continue to attach the task twice without detaching it in the mean
> >> time:
> >> - once during the copy of the process in cpu_cgroup_fork (you skip the
> >> attach of load average but the task is still attached to the local
> >> cpu)
> >
> > Sorry, the task's what is still attached, and how? You mean the vruntime
> > thingy? But the load/util avgs are not.
> 
> yes that's it. The sequence still looks weird IMHO.
> the detach is called for a newly forked task that is not fully init
> and has not been attached yet
> IIUC the fork sequence, we only need to set group at this point so you
> can skip completely the detach/attach_task_cfs_rq not only the
> detach/attach_entity_load_avg
 
Ok, I previously didn't attempt to touch the vruntime part, because I'm not
entirely familiar with it (and never attempted to).

Avoiding attach/detach new task in fork indeed makes sense, but I am not sure,
Peter, Byungchul?

Thanks,
Yuyang

      reply	other threads:[~2016-05-30  9:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-26  1:14 [PATCH 0/2] sched/fair: Skip sched avgs update for new group task before wake_up_new_task() Yuyang Du
2016-05-26  1:14 ` [PATCH 1/2] sched/fair: Clean up attach_entity_load_avg() Yuyang Du
2016-05-26  1:14 ` [PATCH 2/2] sched/fair: Skip detach and attach load avgs for new group task Yuyang Du
2016-05-26 11:50   ` Vincent Guittot
2016-05-26 19:44     ` Yuyang Du
2016-05-27 13:37       ` Vincent Guittot
2016-05-30  1:33         ` Yuyang Du [this message]

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=20160530013336.GL18670@intel.com \
    --to=yuyang.du@intel.com \
    --cc=bsegall@google.com \
    --cc=byungchul.park@lge.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=morten.rasmussen@arm.com \
    --cc=peterz@infradead.org \
    --cc=pjt@google.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.