From: bsegall@google.com
To: Yuyang Du <yuyang.du@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
Morten Rasmussen <morten.rasmussen@arm.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steve Muckle <steve.muckle@linaro.org>,
"mingo\@redhat.com" <mingo@redhat.com>,
"daniel.lezcano\@linaro.org" <daniel.lezcano@linaro.org>,
"mturquette\@baylibre.com" <mturquette@baylibre.com>,
"rjw\@rjwysocki.net" <rjw@rjwysocki.net>,
Juri Lelli <Juri.Lelli@arm.com>,
"sgurrappadi\@nvidia.com" <sgurrappadi@nvidia.com>,
"pang.xunlei\@zte.com.cn" <pang.xunlei@zte.com.cn>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Date: Mon, 21 Sep 2015 10:30:04 -0700 [thread overview]
Message-ID: <xm26d1xbader.fsf@sword-of-the-dawn.mtv.corp.google.com> (raw)
In-Reply-To: <20150921011652.GD11102@intel.com> (Yuyang Du's message of "Mon, 21 Sep 2015 09:16:52 +0800")
Yuyang Du <yuyang.du@intel.com> writes:
> On Thu, Sep 17, 2015 at 12:38:25PM +0200, Peter Zijlstra wrote:
>> On Fri, Sep 11, 2015 at 06:22:47PM +0100, Morten Rasmussen wrote:
>>
>> > While at it, should I include Yuyang's patch redefining the SCALE/SHIFT
>> > mess?
>>
>> I suspect his patch will fail to compile on ARM which uses
>> SCHED_CAPACITY_* outside of kernel/sched/*.
>>
>> But if you all (Ben, Yuyang, you) can agree on a patch simplifying these
>> things I'm not opposed to it.
>
> Yes, indeed. So SCHED_RESOLUTION_SHIFT has to be defined in include/linux/sched.h.
>
> With this, I think the codes still need some cleanup, and importantly
> documentation.
>
> But first, I think as load_sum and load_avg can afford NICE_0_LOAD with either high
> or low resolution. So we have no reason to have low resolution (10bits) load_avg
> when NICE_0_LOAD has high resolution (20bits), because load_avg = runnable% * load,
> as opposed to now we have load_avg = runnable% * scale_load_down(load).
>
> We get rid of all scale_load_down() for runnable load average?
Hmm, LOAD_AVG_MAX * prio_to_weight[0] is 4237627662, ie barely within a
32-bit unsigned long, but in fact LOAD_AVG_MAX * MAX_SHARES is already
going to give errors on 32-bit (even with the old code in fact). This
should probably be fixed... somehow (dividing by 4 for load_sum on
32-bit would work, though be ugly. Reducing MAX_SHARES by 2 bits on
32-bit might have made sense but would be a weird difference between 32
and 64, and could break userspace anyway, so it's presumably too late
for that).
64-bit has ~30 bits free, so this would be fine so long as SLR is 0 on
32-bit.
>
> --
>
> Subject: [PATCH] sched/fair: Generalize the load/util averages resolution
> definition
>
> The metric needs certain resolution to determine how much detail we
> can look into (or not losing detail by integer rounding), which also
> determines the range of the metrics.
>
> For instance, to increase the resolution of [0, 1] (two levels), one
> can multiply 1024 and get [0, 1024] (1025 levels).
>
> In sched/fair, a few metrics depend on the resolution: load/load_avg,
> util_avg, and capacity (frequency adjustment). In order to reduce the
> risks to make mistakes relating to resolution/range, we therefore
> generalize the resolution by defining a basic resolution constant
> number, and then formalize all metrics by depending on the basic
> resolution. The basic resolution is 1024 or (1 << 10). Further, one
> can recursively apply the basic resolution to increase the final
> resolution.
>
> Pointed out by Ben Segall, NICE_0's weight (visible to user) and load
> have independent resolution, but they must be well calibrated.
>
> Signed-off-by: Yuyang Du <yuyang.du@intel.com>
> ---
> include/linux/sched.h | 9 ++++++---
> kernel/sched/fair.c | 4 ----
> kernel/sched/sched.h | 15 ++++++++++-----
> 3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index bd38b3e..9b86f79 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -909,10 +909,13 @@ enum cpu_idle_type {
> CPU_MAX_IDLE_TYPES
> };
>
> +# define SCHED_RESOLUTION_SHIFT 10
> +# define SCHED_RESOLUTION_SCALE (1L << SCHED_RESOLUTION_SHIFT)
> +
> /*
> * Increase resolution of cpu_capacity calculations
> */
> -#define SCHED_CAPACITY_SHIFT 10
> +#define SCHED_CAPACITY_SHIFT SCHED_RESOLUTION_SHIFT
> #define SCHED_CAPACITY_SCALE (1L << SCHED_CAPACITY_SHIFT)
>
> /*
> @@ -1180,8 +1183,8 @@ struct load_weight {
> * 1) load_avg factors frequency scaling into the amount of time that a
> * sched_entity is runnable on a rq into its weight. For cfs_rq, it is the
> * aggregated such weights of all runnable and blocked sched_entities.
> - * 2) util_avg factors frequency and cpu scaling into the amount of time
> - * that a sched_entity is running on a CPU, in the range [0..SCHED_LOAD_SCALE].
> + * 2) util_avg factors frequency and cpu capacity scaling into the amount of time
> + * that a sched_entity is running on a CPU, in the range [0..SCHED_CAPACITY_SCALE].
> * For cfs_rq, it is the aggregated such times of all runnable and
> * blocked sched_entities.
> * The 64 bit load_sum can:
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 4df37a4..c61fd8e 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -2522,10 +2522,6 @@ static u32 __compute_runnable_contrib(u64 n)
> return contrib + runnable_avg_yN_sum[n];
> }
>
> -#if (SCHED_LOAD_SHIFT - SCHED_LOAD_RESOLUTION) != 10 || SCHED_CAPACITY_SHIFT != 10
> -#error "load tracking assumes 2^10 as unit"
> -#endif
> -
> #define cap_scale(v, s) ((v)*(s) >> SCHED_CAPACITY_SHIFT)
>
> /*
> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
> index 3845a71..31b4022 100644
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -53,18 +53,23 @@ static inline void update_cpu_load_active(struct rq *this_rq) { }
> * increased costs.
> */
> #if 0 /* BITS_PER_LONG > 32 -- currently broken: it increases power usage under light load */
> -# define SCHED_LOAD_RESOLUTION 10
> -# define scale_load(w) ((w) << SCHED_LOAD_RESOLUTION)
> -# define scale_load_down(w) ((w) >> SCHED_LOAD_RESOLUTION)
> +# define SCHED_LOAD_SHIFT (SCHED_RESOLUTION_SHIFT + SCHED_RESOLUTION_SHIFT)
> +# define scale_load(w) ((w) << SCHED_RESOLUTION_SHIFT)
> +# define scale_load_down(w) ((w) >> SCHED_RESOLUTION_SHIFT)
> #else
> -# define SCHED_LOAD_RESOLUTION 0
> +# define SCHED_LOAD_SHIFT (SCHED_RESOLUTION_SHIFT)
> # define scale_load(w) (w)
> # define scale_load_down(w) (w)
> #endif
>
> -#define SCHED_LOAD_SHIFT (10 + SCHED_LOAD_RESOLUTION)
> #define SCHED_LOAD_SCALE (1L << SCHED_LOAD_SHIFT)
>
> +/*
> + * NICE_0's weight (visible to user) and its load (invisible to user) have
> + * independent resolution, but they should be well calibrated. We use scale_load()
> + * and scale_load_down(w) to convert between them, the following must be true:
> + * scale_load(prio_to_weight[20]) == NICE_0_LOAD
> + */
> #define NICE_0_LOAD SCHED_LOAD_SCALE
> #define NICE_0_SHIFT SCHED_LOAD_SHIFT
I still think tying the scale_load shift to be the same as the
SCHED_CAPACITY/etc shift is silly, and tying the NICE_0_LOAD/SHIFT in is
worse. Honestly if I was going to change anything it would be to define
NICE_0_LOAD/SHIFT entirely separately from SCHED_LOAD_SCALE/SHIFT.
However I'm not sure if calculate_imbalance's use of SCHED_LOAD_SCALE is
actually a separate use of 1024*SLR-as-percentage or is basically
assuming most tasks are nice-0 or what. It sure /looks/ like it's
comparing values with different units - it's doing (nr_running * CONST -
group_capacity) and comparing to load, so it looks like both (ie
increasing load.weight of everything on your system by X% would change
load balancer behavior here).
Given that it might make sense to make it clear that capacity units and
nice-0-task units have to be the same thing due to load balancer
approximations (though they are still entirely separate from the
SCHED_LOAD_RESOLUTION multiplier).
next prev parent reply other threads:[~2015-09-21 17:30 UTC|newest]
Thread overview: 97+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-14 16:23 [PATCH 0/6] sched/fair: Compute capacity invariant load/utilization tracking Morten Rasmussen
2015-08-14 16:23 ` [PATCH 1/6] sched/fair: Make load tracking frequency scale-invariant Morten Rasmussen
2015-09-13 11:03 ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 2/6] sched/fair: Convert arch_scale_cpu_capacity() from weak function to #define Morten Rasmussen
2015-09-02 9:31 ` Vincent Guittot
2015-09-02 12:41 ` Vincent Guittot
2015-09-03 19:58 ` Dietmar Eggemann
2015-09-04 7:26 ` Vincent Guittot
2015-09-07 13:25 ` Dietmar Eggemann
2015-09-11 13:21 ` Dietmar Eggemann
2015-09-11 14:45 ` Vincent Guittot
2015-09-13 11:03 ` [tip:sched/core] " tip-bot for Morten Rasmussen
2015-08-14 16:23 ` [PATCH 3/6] sched/fair: Make utilization tracking cpu scale-invariant Morten Rasmussen
2015-08-14 23:04 ` Dietmar Eggemann
2015-09-04 7:52 ` Vincent Guittot
2015-09-13 11:04 ` [tip:sched/core] sched/fair: Make utilization tracking CPU scale-invariant tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 4/6] sched/fair: Name utilization related data and functions consistently Morten Rasmussen
2015-09-04 9:08 ` Vincent Guittot
2015-09-11 16:35 ` Dietmar Eggemann
2015-09-13 11:04 ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig Morten Rasmussen
2015-09-03 23:51 ` Steve Muckle
2015-09-07 15:37 ` Dietmar Eggemann
2015-09-07 16:21 ` Vincent Guittot
2015-09-07 18:54 ` Dietmar Eggemann
2015-09-07 19:47 ` Peter Zijlstra
2015-09-08 12:47 ` Dietmar Eggemann
2015-09-08 7:22 ` Vincent Guittot
2015-09-08 12:26 ` Peter Zijlstra
2015-09-08 12:52 ` Peter Zijlstra
2015-09-08 14:06 ` Vincent Guittot
2015-09-08 14:35 ` Morten Rasmussen
2015-09-08 14:40 ` Vincent Guittot
2015-09-08 14:31 ` Morten Rasmussen
2015-09-08 15:33 ` Peter Zijlstra
2015-09-09 22:23 ` bsegall
2015-09-10 11:06 ` Morten Rasmussen
2015-09-10 11:11 ` Vincent Guittot
2015-09-10 12:10 ` Morten Rasmussen
2015-09-11 0:50 ` Yuyang Du
2015-09-10 17:23 ` bsegall
2015-09-08 16:53 ` Morten Rasmussen
2015-09-09 9:43 ` Peter Zijlstra
2015-09-09 9:45 ` Peter Zijlstra
2015-09-09 11:13 ` Morten Rasmussen
2015-09-11 17:22 ` Morten Rasmussen
2015-09-17 9:51 ` Peter Zijlstra
2015-09-17 10:38 ` Peter Zijlstra
2015-09-21 1:16 ` Yuyang Du
2015-09-21 17:30 ` bsegall [this message]
2015-09-21 23:39 ` Yuyang Du
2015-09-22 17:18 ` bsegall
2015-09-22 23:22 ` Yuyang Du
2015-09-23 16:54 ` bsegall
2015-09-24 0:22 ` Yuyang Du
2015-09-30 12:52 ` Peter Zijlstra
2015-09-11 7:46 ` Leo Yan
2015-09-11 10:02 ` Morten Rasmussen
2015-09-11 14:11 ` Leo Yan
2015-09-09 19:07 ` Yuyang Du
2015-09-10 10:06 ` Peter Zijlstra
2015-09-08 13:39 ` Vincent Guittot
2015-09-08 14:10 ` Peter Zijlstra
2015-09-08 15:17 ` Vincent Guittot
2015-09-08 12:50 ` Dietmar Eggemann
2015-09-08 14:01 ` Vincent Guittot
2015-09-08 14:27 ` Dietmar Eggemann
2015-09-09 20:15 ` Yuyang Du
2015-09-10 10:07 ` Peter Zijlstra
2015-09-11 0:28 ` Yuyang Du
2015-09-11 10:31 ` Morten Rasmussen
2015-09-11 17:05 ` bsegall
2015-09-11 18:24 ` Yuyang Du
2015-09-14 17:36 ` bsegall
2015-09-14 12:56 ` Morten Rasmussen
2015-09-14 17:34 ` bsegall
2015-09-14 22:56 ` Yuyang Du
2015-09-15 17:11 ` bsegall
2015-09-15 18:39 ` Yuyang Du
2015-09-16 17:06 ` bsegall
2015-09-17 2:31 ` Yuyang Du
2015-09-15 8:43 ` Morten Rasmussen
2015-09-16 15:36 ` Peter Zijlstra
2015-09-08 11:44 ` Peter Zijlstra
2015-09-13 11:04 ` [tip:sched/core] " tip-bot for Dietmar Eggemann
2015-08-14 16:23 ` [PATCH 6/6] sched/fair: Initialize task load and utilization before placing task on rq Morten Rasmussen
2015-09-13 11:05 ` [tip:sched/core] " tip-bot for Morten Rasmussen
2015-08-16 20:46 ` [PATCH 0/6] sched/fair: Compute capacity invariant load/utilization tracking Peter Zijlstra
2015-08-17 11:29 ` Morten Rasmussen
2015-08-17 11:48 ` Peter Zijlstra
2015-08-31 9:24 ` Peter Zijlstra
2015-09-02 9:51 ` Dietmar Eggemann
2015-09-07 12:42 ` Peter Zijlstra
2015-09-07 13:21 ` Peter Zijlstra
2015-09-07 13:23 ` Peter Zijlstra
2015-09-07 14:44 ` Dietmar Eggemann
2015-09-13 11:06 ` [tip:sched/core] sched/fair: Defer calling scaling functions tip-bot for Dietmar Eggemann
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=xm26d1xbader.fsf@sword-of-the-dawn.mtv.corp.google.com \
--to=bsegall@google.com \
--cc=Juri.Lelli@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=morten.rasmussen@arm.com \
--cc=mturquette@baylibre.com \
--cc=pang.xunlei@zte.com.cn \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=sgurrappadi@nvidia.com \
--cc=steve.muckle@linaro.org \
--cc=vincent.guittot@linaro.org \
--cc=yuyang.du@intel.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.