public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Ingo Molnar <mingo@redhat.com>,
	Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/8] sched: clean up move_task() and move_one_task()
Date: Wed, 20 Mar 2013 12:16:41 +0100	[thread overview]
Message-ID: <1363778201.3724.1.camel@laptop> (raw)
In-Reply-To: <20130320073355.GC11672@lge.com>


On Wed, 2013-03-20 at 16:33 +0900, Joonsoo Kim wrote:

> > Right, so I'm not so taken with this one. The whole load stuff really
> > is a balance heuristic that's part of move_tasks(), move_one_task()
> > really doesn't care about that.
> > 
> > So why did you include it? Purely so you didn't have to re-order the
> > tests? I don't see any reason not to flip a tests around.
> 
> I think that I'm not fully understand what you are concerning, because of
> my poor English. If possible, please elaborate on a problem in more detail.

OK, so your initial Changelog said it wanted to remove some code
duplication between move_tasks() and move_one_task(); but then you put
in the load heuristics and add a boolean argument to only enable those
for move_tasks() -- so clearly that wasn't duplicated.

So why move that code.. I proposed that this was due a reluctance to
re-arrange the various tests that stop the migration from happening.

Now you say:

> ... Just moving up can_migrate_task() above
> load evaluation code may raise side effect, because can_migrate_task() have
> other checking which is 'cache hottness'. I don't want a side effect. So
> embedding load evaluation to can_migrate_task() and re-order checking and
> makes load evaluation disabled for move_one_task().

Which pretty much affirms this. However I also said that I don't think
the order really matters that much; each test will cancel the migration
of this task; the order of these tests seem immaterial.

> If your recommandation is to move up can_mirate_task() above
> load evaluation code, yes, I can, and will do that. :)

I would actually propose 



  parent reply	other threads:[~2013-03-20 11:16 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-14  5:48 [PATCH 0/8] correct load_balance() Joonsoo Kim
2013-02-14  5:48 ` [PATCH 1/8] sched: change position of resched_cpu() in load_balance() Joonsoo Kim
2013-03-19 12:59   ` Peter Zijlstra
2013-02-14  5:48 ` [PATCH 2/8] sched: explicitly cpu_idle_type checking in rebalance_domains() Joonsoo Kim
2013-03-19 14:02   ` Peter Zijlstra
2013-03-20  6:48     ` Joonsoo Kim
2013-02-14  5:48 ` [PATCH 3/8] sched: don't consider other cpus in our group in case of NEWLY_IDLE Joonsoo Kim
2013-03-19 14:20   ` Peter Zijlstra
2013-03-20  6:52     ` Joonsoo Kim
2013-02-14  5:48 ` [PATCH 4/8] sched: clean up move_task() and move_one_task() Joonsoo Kim
2013-03-19 14:30   ` Peter Zijlstra
2013-03-20  7:33     ` Joonsoo Kim
2013-03-20 11:11       ` Peter Zijlstra
2013-03-20 13:42         ` JoonSoo Kim
2013-03-20 11:16       ` Peter Zijlstra [this message]
2013-03-20 12:07         ` Peter Zijlstra
2013-02-14  5:48 ` [PATCH 5/8] sched: move up affinity check to mitigate useless redoing overhead Joonsoo Kim
2013-02-14  5:48 ` [PATCH 6/8] sched: rename load_balance_tmpmask to load_balance_cpu_active Joonsoo Kim
2013-03-19 15:01   ` Peter Zijlstra
2013-03-20  7:35     ` Joonsoo Kim
2013-02-14  5:48 ` [PATCH 7/8] sched: prevent to re-select dst-cpu in load_balance() Joonsoo Kim
2013-03-19 15:05   ` Peter Zijlstra
2013-03-20  7:43     ` Joonsoo Kim
2013-03-20 12:38       ` Peter Zijlstra
2013-03-20 13:48         ` JoonSoo Kim
2013-02-14  5:48 ` [PATCH 8/8] sched: reset lb_env when redo " Joonsoo Kim
2013-03-19 15:21   ` Peter Zijlstra
2013-03-20  8:13     ` Joonsoo Kim
2013-02-25  4:56 ` [PATCH 0/8] correct load_balance() Joonsoo Kim
2013-03-19  5:11   ` Joonsoo Kim

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=1363778201.3724.1.camel@laptop \
    --to=peterz@infradead.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=vatsa@linux.vnet.ibm.com \
    /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