All of lore.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 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.