public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Alex Shi <alex.shi@linaro.org>
Cc: mingo@redhat.com, morten.rasmussen@arm.com,
	vincent.guittot@linaro.org, daniel.lezcano@linaro.org,
	fweisbec@gmail.com, linux@arm.linux.org.uk, tony.luck@intel.com,
	fenghua.yu@intel.com, james.hogan@imgtec.com, jason.low2@hp.com,
	viresh.kumar@linaro.org, hanjun.guo@linaro.org,
	linux-kernel@vger.kernel.org, tglx@linutronix.de,
	akpm@linux-foundation.org, arjan@linux.intel.com, pjt@google.com,
	fengguang.wu@intel.com, linaro-kernel@lists.linaro.org,
	wangyun@linux.vnet.ibm.com, mgorman@suse.de
Subject: Re: [PATCH 04/11] sched: unify imbalance bias for target group
Date: Tue, 25 Feb 2014 15:14:07 +0100	[thread overview]
Message-ID: <20140225141407.GT9987@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <1393293054-11378-5-git-send-email-alex.shi@linaro.org>

On Tue, Feb 25, 2014 at 09:50:47AM +0800, Alex Shi wrote:
> Old code considers the bias in source/target_load already. but still
> use imbalance_pct as last check in idlest/busiest group finding. It is
> also a kind of redundant job. If we bias imbalance in source/target_load,
> we'd better not use imbalance_pct again.
> 
> After cpu_load array removed, it is nice time to unify the target bias
> consideration. So I remove the imbalance_pct from last check and add the
> live bias using.
> 
> On wake_affine, since all archs' wake_idx is 0, current logical is just
> want to prefer current cpu. so we follows this logical. Just renaming the
> target_load/source_load to wegithed_cpuload for more exact meaning.
> Thanks for reminding from Morten!
> 

So this patch is weird..

So the original bias in the source/target load is purely based on actual
load figures. It only pulls-down/pulls-up resp. the long term avg with a
shorter term average; iow. it allows the source to decrease faster and
the target to increase faster, giving a natural inertia (ie. a
resistance to movement).

Therefore this gives rise to a conservative imbalance.

Then at the end we use the imbalance_pct thing as a normal hysteresis
control to avoid the rapid state switching associated with a single
control point system.


You completely wreck that, you also don't give a coherent model back.


The movement of imbalance_pct into target_load() doesn't make sense to
me either; it's an (expensive) no-op afaict. Seeing how:

  100 * source_load() < imb_pct * target_load()

is very much equal to:

  source_load() < (imb_pct * target_load()) / 100;

Except you get to do that div all over the place.

It also completely muddles the fact that its a normal hysteresis
control. Not a load bias. A fixed bias can never replace the inertial
control we had; it doesn't make sense as a replacement.

Not to mention you seem to ignore all concerns wrt the use of longer
term averages for the bigger domains.

Now I'm all for removing code; and so far the numbers aren't bad; but I
don't like the complete muddle you make of things at all.

  reply	other threads:[~2014-02-25 14:15 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-25  1:50 [PATCH V4 0/11] sched: remove cpu_loads Alex Shi
2014-02-25  1:50 ` [PATCH 01/11] sched: shortcut to remove load_idx Alex Shi
2014-02-25  1:50 ` [PATCH 02/11] sched: remove rq->cpu_load[load_idx] array Alex Shi
2014-02-25 16:22   ` Srikar Dronamraju
2014-02-26  1:54     ` Alex Shi
2014-02-25  1:50 ` [PATCH 03/11] sched: clean up cpu_load update Alex Shi
2014-02-25  1:50 ` [PATCH 04/11] sched: unify imbalance bias for target group Alex Shi
2014-02-25 14:14   ` Peter Zijlstra [this message]
2014-02-26 15:16     ` Alex Shi
2014-03-02  1:44       ` Alex Shi
2014-03-12 10:36       ` Alex Shi
2014-02-25  1:50 ` [PATCH 05/11] sched: rewrite update_cpu_load_nohz Alex Shi
2014-02-25  1:50 ` [PATCH 06/11] sched: clean up source_load/target_load Alex Shi
2014-02-25  1:50 ` [PATCH 07/11] sched: replace source_load by weighted_cpuload Alex Shi
2014-02-25  1:50 ` [PATCH 08/11] sched: replace target_load by biased_load Alex Shi
2014-02-25  1:50 ` [PATCH 09/11] sched: remove rq->cpu_load and rq->nr_load_updates Alex Shi
2014-02-25  1:50 ` [PATCH 10/11] sched: rename update_*_cpu_load Alex Shi
2014-02-25  1:50 ` [PATCH 11/11] sched: clean up task_hot function Alex Shi

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=20140225141407.GT9987@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=alex.shi@linaro.org \
    --cc=arjan@linux.intel.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=fengguang.wu@intel.com \
    --cc=fenghua.yu@intel.com \
    --cc=fweisbec@gmail.com \
    --cc=hanjun.guo@linaro.org \
    --cc=james.hogan@imgtec.com \
    --cc=jason.low2@hp.com \
    --cc=linaro-kernel@lists.linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=pjt@google.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    --cc=wangyun@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