All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yuyang Du <yuyang.du@intel.com>
To: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	mingo@kernel.org, linux-kernel@vger.kernel.org,
	bsegall@google.com, pjt@google.com, morten.rasmussen@arm.com,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com
Subject: Re: [RFC PATCH 1/2] sched: Clean up SD_BALANCE_WAKE flags in sched domain build-up
Date: Thu, 2 Jun 2016 06:41:06 +0800	[thread overview]
Message-ID: <20160601224106.GB18670@intel.com> (raw)
In-Reply-To: <1464846623.3766.49.camel@gmail.com>

On Thu, Jun 02, 2016 at 07:50:23AM +0200, Mike Galbraith wrote:
> > > Nope, those two have different meanings.  We pass SD_BALANCE_WAKE to
> > > identify a ttwu() wakeup, just as we pass SD_BALANCE_FORK to say we're
> > > waking a child.  SD_WAKE_AFFINE means exactly what it says, but is only
> > > applicable to ttwu() wakeups.
> >  
> > I don't disagree, but want to add that, SD_WAKE_AFFINE has no meaning that is so
> > special and so important for anyone to use the flag to tune anything. If you want
> > to do any SD_BALANCE_*, waker CPU is a valid candidate if allowed, that is it.
> 
> That flag lets the user specifically tell us that he doesn't want us to
> bounce his tasks around the box, cache misses be damned.  The user may
> _know_ that say cross node migrations hurt his load more than help, and
> not want us to do that, thus expresses himself by turning the flag off
> at whatever level.  People do that.  You can force them to take other
> measures, but why do that?
 
Agreed, and with this patch, just disable SD_BALANCE_WAKE.

> > IIUC your XXX mark and your comment "Prefer wake_affine over balance flags", you
> > said the same thing: SD_WAKE_AFFINE should be part of SD_BALANCE_WAKE, and should
> > be part of all SD_BALANCE_* flags,
> 
> Peter wrote that, but I don't read it the way you do.  I read as if the
> user wants the benefits of affine wakeups, he surely doesn't want us to
> send the wakee off to god know where on every wakeup _instead_ of
> waking affine, he wants to balance iff he can't have an affine wakeup.

That is another matter within SD_BALANCE_WAKE we may further define: how
much effort to scan or how frequent bouncing etc the user wants. This is now
defined by SD_WAKE_AFFINE flag, which I certainly don't think is good.
 
> > > If wake_wide() says we do not want an affine wakeup, but you apply
> > > SD_WAKE_AFFINE meaning to SD_BALANCE_WAKE and turn it on in ->flags,
> > > we'll give the user a free sample of full balance cost, no?
> >  
> > Yes, and otherwise we don't select anything? That is just bad engough whether worse
> > or not. So the whole fuss I made is really that this is a right thing to start with. :)
> 
> Nope, leaving tasks where they were is not a bad thing.  Lots of stuff
> likes the scheduler best when it leaves them the hell alone :)  That
> works out well all around, balance cycles are spent in userspace
> instead, scheduler produces wins by doing nothing, perfect.
> 
Again, agreed, and with this patch, just disable SD_BALANCE_WAKE. :)

  reply	other threads:[~2016-06-02  6:38 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-31  1:11 [RFC PATCH 0/2] Remove and replace SD_WAKE_AFFINE with SD_BALANCE_WAKE Yuyang Du
2016-05-31  1:11 ` [RFC PATCH 1/2] sched: Clean up SD_BALANCE_WAKE flags in sched domain build-up Yuyang Du
2016-05-31  9:21   ` Peter Zijlstra
2016-05-31  1:31     ` Yuyang Du
2016-05-31 10:41       ` Peter Zijlstra
2016-05-31 18:00         ` Yuyang Du
2016-06-01  5:07       ` Mike Galbraith
2016-06-01  0:01         ` Yuyang Du
2016-06-01  8:32           ` Vincent Guittot
2016-06-01  1:03             ` Yuyang Du
2016-06-01  9:24               ` Vincent Guittot
2016-06-01 19:35                 ` Yuyang Du
2016-06-02  6:56                   ` Vincent Guittot
2016-06-01 23:19                     ` Yuyang Du
2016-06-01  9:36           ` Mike Galbraith
2016-06-01 20:03             ` Yuyang Du
2016-06-02  5:50               ` Mike Galbraith
2016-06-01 22:41                 ` Yuyang Du [this message]
2016-06-02  6:44                   ` Mike Galbraith
2016-05-31  1:11 ` [RFC PATCH 2/2] sched: Remove SD_WAKE_AFFINE flag and replace it with SD_BALANCE_WAKE Yuyang Du
2016-05-31  9:23   ` Peter Zijlstra
2016-05-31  1:34     ` Yuyang Du

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=20160601224106.GB18670@intel.com \
    --to=yuyang.du@intel.com \
    --cc=bsegall@google.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=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.