From: Vincent Guittot <vincent.guittot@linaro.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steve Muckle <steve.muckle@linaro.org>,
Morten Rasmussen <Morten.Rasmussen@arm.com>,
"mingo@redhat.com" <mingo@redhat.com>,
"daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>,
"yuyang.du@intel.com" <yuyang.du@intel.com>,
"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: Tue, 8 Sep 2015 16:06:36 +0200 [thread overview]
Message-ID: <CAKfTPtB_5qOx=6s9bbb+EDBt0mj_XEwgv5XdYR=S3rXPaerT-w@mail.gmail.com> (raw)
In-Reply-To: <20150908125205.GW18673@twins.programming.kicks-ass.net>
On 8 September 2015 at 14:52, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Sep 08, 2015 at 02:26:06PM +0200, Peter Zijlstra wrote:
>> On Tue, Sep 08, 2015 at 09:22:05AM +0200, Vincent Guittot wrote:
>> > No, but
>> > sa->util_avg = (sa->util_sum << SCHED_CAPACITY_SHIFT) / LOAD_AVG_MAX;
>> > will fix the unit issue.
>>
>> Tricky that, LOAD_AVG_MAX very much relies on the unit being 1<<10.
>>
>> And where load_sum already gets a factor 1024 from the weight
>> multiplication, util_sum does not get such a factor, and all the scaling
>> we do on it loose bits.
>>
>> So at the moment we go compute the util_avg value, we need to inflate
>> util_sum with an extra factor 1024 in order to make it work.
>>
>> And seeing that we do the shift up on sa->util_sum without consideration
>> of overflow, would it not make sense to add that factor before the
>> scaling and into the addition?
>>
>> Now, given all that, units are a complete mess here, and I'd not mind
>> something like:
>>
>> #if (SCHED_LOAD_SHIFT - SCHED_LOAD_RESOLUTION) != SCHED_CAPACITY_SHIFT
>> #error "something usefull"
>> #endif
>>
>> somewhere near here.
>
> Something like teh below..
>
> Another thing to ponder; the downside of scaled_delta_w is that its
> fairly likely delta is small and you loose all bits, whereas the weight
> is likely to be large can could loose a fwe bits without issue.
>
> That is, in fixed point scaling like this, you want to start with the
> biggest numbers, not the smallest, otherwise you loose too much.
>
> The flip side is of course that now you can share a multiplcation.
>
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -682,7 +682,7 @@ void init_entity_runnable_average(struct
> sa->load_avg = scale_load_down(se->load.weight);
> sa->load_sum = sa->load_avg * LOAD_AVG_MAX;
> sa->util_avg = scale_load_down(SCHED_LOAD_SCALE);
> - sa->util_sum = LOAD_AVG_MAX;
> + sa->util_sum = sa->util_avg * LOAD_AVG_MAX;
> /* when this task enqueue'ed, it will contribute to its cfs_rq's load_avg */
> }
>
> @@ -2515,6 +2515,10 @@ static u32 __compute_runnable_contrib(u6
> 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
so why don't we set SCHED_CAPACITY_SHIFT to SCHED_LOAD_SHIFT ?
> +
> #define cap_scale(v, s) ((v)*(s) >> SCHED_CAPACITY_SHIFT)
>
> /*
> @@ -2599,7 +2603,7 @@ __update_load_avg(u64 now, int cpu, stru
> }
> }
> if (running)
> - sa->util_sum += cap_scale(scaled_delta_w, scale_cpu);
> + sa->util_sum += scaled_delta_w * scale_cpu;
>
> delta -= delta_w;
>
> @@ -2623,7 +2627,7 @@ __update_load_avg(u64 now, int cpu, stru
> cfs_rq->runnable_load_sum += weight * contrib;
> }
> if (running)
> - sa->util_sum += cap_scale(contrib, scale_cpu);
> + sa->util_sum += contrib * scale_cpu;
> }
>
> /* Remainder of delta accrued against u_0` */
> @@ -2634,7 +2638,7 @@ __update_load_avg(u64 now, int cpu, stru
> cfs_rq->runnable_load_sum += weight * scaled_delta;
> }
> if (running)
> - sa->util_sum += cap_scale(scaled_delta, scale_cpu);
> + sa->util_sum += scaled_delta * scale_cpu;
>
> sa->period_contrib += delta;
>
> @@ -2644,7 +2648,7 @@ __update_load_avg(u64 now, int cpu, stru
> cfs_rq->runnable_load_avg =
> div_u64(cfs_rq->runnable_load_sum, LOAD_AVG_MAX);
> }
> - sa->util_avg = (sa->util_sum << SCHED_LOAD_SHIFT) / LOAD_AVG_MAX;
> + sa->util_avg = sa->util_sum / LOAD_AVG_MAX;
> }
>
> return decayed;
> @@ -2686,8 +2690,7 @@ static inline int update_cfs_rq_load_avg
> if (atomic_long_read(&cfs_rq->removed_util_avg)) {
> long r = atomic_long_xchg(&cfs_rq->removed_util_avg, 0);
> sa->util_avg = max_t(long, sa->util_avg - r, 0);
> - sa->util_sum = max_t(s32, sa->util_sum -
> - ((r * LOAD_AVG_MAX) >> SCHED_LOAD_SHIFT), 0);
> + sa->util_sum = max_t(s32, sa->util_sum - r * LOAD_AVG_MAX, 0);
looks good to me
> }
>
> decayed = __update_load_avg(now, cpu_of(rq_of(cfs_rq)), sa,
next prev parent reply other threads:[~2015-09-08 14:07 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 [this message]
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
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='CAKfTPtB_5qOx=6s9bbb+EDBt0mj_XEwgv5XdYR=S3rXPaerT-w@mail.gmail.com' \
--to=vincent.guittot@linaro.org \
--cc=Juri.Lelli@arm.com \
--cc=Morten.Rasmussen@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=dietmar.eggemann@arm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.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=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.