mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mike Galbraith <efault@gmx.de>
To: Dima Zavin <dmitriyz@google.com>
Cc: "Peter Zijlstra" <peterz@infradead.org>,
	"Ingo Molnar" <mingo@elte.hu>, "Arve Hjønnevåg" <arve@google.com>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: broken behavior in cfs when moving threads between cgroups
Date: Wed, 29 Sep 2010 04:53:32 +0200	[thread overview]
Message-ID: <1285728812.7440.141.camel@marge.simson.net> (raw)
In-Reply-To: <AANLkTikjH1J_nCxZympo9v_VZbptWHBNJ8xh=pFXLRrQ@mail.gmail.com>

On Tue, 2010-09-28 at 15:57 -0700, Dima Zavin wrote:

> Hmm. Would we really want to give the sleeping task such a bump in the
> new group? Why not just start it off at min_vruntime and let
> place_entity do it's thing? Something like...

No, I wouldn't want to give it anything, just enter at zero lag..

> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c
> index a7be83c..4656231 100644
> --- a/kernel/sched_fair.c
> +++ b/kernel/sched_fair.c
> @@ -3622,10 +3622,15 @@ static void set_curr_task_fair(struct rq *rq)
>  static void moved_group_fair(struct task_struct *p, int on_rq)
>  {
>         struct cfs_rq *cfs_rq = task_cfs_rq(p);
> +       struct sched_entity *se = &p->se;
> 
>         update_curr(cfs_rq);
> -       if (!on_rq)
> +       if (on_rq) {
> +               se->vruntime = 0;
> +       } else {
> +               se->vruntime = cfs_rq->min_vruntime;
>                 place_entity(cfs_rq, &p->se, 1);
> +       }
>  }
>  #endif

(Why penalize the sleeper?)

..but, as you noted, moving out then _back_ at forced 0 lag would result
in bogus vruntime deltas, so lag must be preserved.  The sleeper's
vruntime has to be set to relative before it's cfs_rq is changed, then
back to absolute in moved_group_fair() I suppose.

	-Mike


  parent reply	other threads:[~2010-09-29  2:53 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-28  6:07 Dima Zavin
2010-09-28  8:17 ` Mike Galbraith
2010-09-28 22:57   ` Dima Zavin
2010-09-28 23:23     ` Dima Zavin
2010-09-29  2:53     ` Mike Galbraith [this message]
2010-09-29  6:42       ` Mike Galbraith
2010-09-29  6:45         ` Dima Zavin

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=1285728812.7440.141.camel@marge.simson.net \
    --to=efault@gmx.de \
    --cc=arve@google.com \
    --cc=dmitriyz@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.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

all inboxes | Powered by JetHome®