From: Vincent Guittot <vincent.guittot@linaro.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Steve Muckle <steve.muckle@linaro.org>,
Morten Rasmussen <Morten.Rasmussen@arm.com>,
"peterz@infradead.org" <peterz@infradead.org>,
"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:01:55 +0200 [thread overview]
Message-ID: <CAKfTPtBNZL=VkuS3TVeQOptH+4aZR9aMyC2CfAxn8oSrmmFa2w@mail.gmail.com> (raw)
In-Reply-To: <55EED99E.2040100@arm.com>
On 8 September 2015 at 14:50, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
> On 08/09/15 08:22, Vincent Guittot wrote:
>> On 7 September 2015 at 20:54, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>>> On 07/09/15 17:21, Vincent Guittot wrote:
>>>> On 7 September 2015 at 17:37, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
>>>>> On 04/09/15 00:51, Steve Muckle wrote:
>>>>>> Hi Morten, Dietmar,
>>>>>>
>>>>>> On 08/14/2015 09:23 AM, Morten Rasmussen wrote:
>>>>>> ...
>>>>>>> + * cfs_rq.avg.util_avg is the sum of running time of runnable tasks plus the
>>>>>>> + * recent utilization of currently non-runnable tasks on a CPU. It represents
>>>>>>> + * the amount of utilization of a CPU in the range [0..capacity_orig] where
>>>>>>
>>>>>> I see util_sum is scaled by SCHED_LOAD_SHIFT at the end of
>>>>>> __update_load_avg(). If there is now an assumption that util_avg may be
>>>>>> used directly as a capacity value, should it be changed to
>>>>>> SCHED_CAPACITY_SHIFT? These are equal right now, not sure if they will
>>>>>> always be or if they can be combined.
>>>>>
>>>>> You're referring to the code line
>>>>>
>>>>> 2647 sa->util_avg = (sa->util_sum << SCHED_LOAD_SHIFT) / LOAD_AVG_MAX;
>>>>>
>>>>> in __update_load_avg()?
>>>>>
>>>>> Here we actually scale by 'SCHED_LOAD_SCALE/LOAD_AVG_MAX' so both values are
>>>>> load related.
>>>>
>>>> I agree with Steve that there is an issue from a unit point of view
>>>>
>>>> sa->util_sum and LOAD_AVG_MAX have the same unit so sa->util_avg is a
>>>> load because of << SCHED_LOAD_SHIFT)
>>>>
>>>> Before this patch , the translation from load to capacity unit was
>>>> done in get_cpu_usage with "* capacity) >> SCHED_LOAD_SHIFT"
>>>>
>>>> So you still have to change the unit from load to capacity with a "/
>>>> SCHED_LOAD_SCALE * SCHED_CAPACITY_SCALE" somewhere.
>>>>
>>>> sa->util_avg = ((sa->util_sum << SCHED_LOAD_SHIFT) /SCHED_LOAD_SCALE *
>>>> SCHED_CAPACITY_SCALE / LOAD_AVG_MAX = (sa->util_sum <<
>>>> SCHED_CAPACITY_SHIFT) / LOAD_AVG_MAX;
>>>
>>> I see the point but IMHO this will only be necessary if the SCHED_LOAD_RESOLUTION
>>> stuff gets re-enabled again.
>>>
>>> It's not really about utilization or capacity units but rather about using the same
>>> SCALE/SHIFT values for both sides, right?
>>
>> It's both a unit and a SCALE/SHIFT problem, SCHED_LOAD_SHIFT and
>> SCHED_CAPACITY_SHIFT are defined separately so we must be sure to
>> scale the value in the right range. In the case of cpu_usage which
>> returns sa->util_avg , it's the capacity range not the load range.
>
> Still don't understand why it's a unit problem. IMHO LOAD/UTIL and
> CAPACITY have no unit.
If you set 2 different values to SCHED_LOAD_SHIFT and
SCHED_CAPACITY_SHIFT for test purpose, you will see that util_avg will
not use the right range of value
If we don't take into account freq and cpu invariance in a 1st step
sa->util_sum is a load in the range [0..LOAD_AVG_MAX]. I say load
because of the max value
the current implementation of util_avg is
sa->util_avg = (sa->util_sum << SCHED_LOAD_SHIFT) / LOAD_AVG_MAX
so sa->util_avg is a load in the range [0..SCHED_LOAD_SCALE]
the current implementation of get_cpu_usage is
return (sa->util_avg * capacity_orig_of(cpu)) >> SCHED_LOAD_SHIFT
so the usage has the same unit and range as capacity of the cpu and
can be compared with another capacity value
Your patchset returns directly sa->util_avg which is a load to compare
it with capacity value
So you have to convert sa->util_avg from load to capacity so if you have
sa->util_avg = (sa->util_sum << SCHED_CAPACITY_SHIFT) / LOAD_AVG_MAX
sa->util_avg is now a capacity with the same range as you cpu thanks
to the cpu invariance factor that the patch 3 has added.
the << SCHED_CAPACITY_SHIFT above can be optimized with the >>
SCHED_CAPACITY_SHIFT included in
sa->util_sum += scale(contrib, scale_cpu);
as mentioned by Peter
At now, SCHED_CAPACITY_SHIFT is set to 10 as well as SCHED_LOAD_SHIFT
so using one instead of the other doesn't change the result but if
it's no more the case, we need to take care of the range/unit that we
use
Regards,
Vincent
>
> I agree that with the current patch-set we have a SHIFT/SCALE problem
> once SCHED_LOAD_RESOLUTION is set to != 0.
>
>>
>>>
>>> I always thought that scale_load_down() takes care of that.
>>
>> AFAIU, scale_load_down is a way to increase the resolution of the
>> load not to move from load to capacity
>
> IMHO, increasing the resolution of the load is done by re-enabling this
> define SCHED_LOAD_RESOLUTION 10 thing (or by setting
> SCHED_LOAD_RESOLUTION to something else than 0).
>
> I tried to figure out why we have this issue when comparing UTIL w/
> CAPACITY and not LOAD w/ CAPACITY:
>
> Both are initialized like that:
>
> 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;
>
> and we use 'se->on_rq * scale_load_down(se->load.weight)' as 'unsigned
> long weight' argument to call __update_load_avg() making sure the
> scaling differences between LOAD and CAPACITY are respected while
> updating sa->load_sum (and sa->load_avg).
>
> OTAH, we don't apply a scale_load_down for sa->util_[sum/avg] only a '<<
> SCHED_LOAD_SHIFT) / LOAD_AVG_MAX' on sa->util_avg.
> So changing '<< SCHED_LOAD_SHIFT' to '*
> scale_load_down(SCHED_LOAD_SCALE)' would be the logical thing to do.
>
> I agree that '<< SCHED_CAPACITY_SHIFT' would have the same effect but
> why using a CAPACITY related thing on the LOAD/UTIL side? The only
> reason would be the unit problem which I don't understand.
>
>>
>>>
>>> So shouldn't:
>>>
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index 3445d2fb38f4..b80f799aface 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -2644,7 +2644,7 @@ __update_load_avg(u64 now, int cpu, struct sched_avg *sa,
>>> 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 * scale_load_down(SCHED_LOAD_SCALE)) / LOAD_AVG_MAX;
>>> }
>>>
>>> return decayed;
>>>
>>> fix that issue in case SCHED_LOAD_RESOLUTION != 0 ?
>>
>>
>> No, but
>> sa->util_avg = (sa->util_sum << SCHED_CAPACITY_SHIFT) / LOAD_AVG_MAX;
>> will fix the unit issue.
>> I agree that i don't change the result because both SCHED_LOAD_SHIFT
>> and SCHED_CAPACITY_SHIFT are set to 10 but as mentioned above, they
>> are set separately so it can make the difference if someone change one
>> SHIFT value.
>
> SCHED_LOAD_SHIFT and SCHED_CAPACITY_SHIFT can be set separately but the
> way to change SCHED_LOAD_SHIFT is by re-enabling the define
> SCHED_LOAD_RESOLUTION 10 in kernel/sched/sched.h. I guess nobody wants
> to change SCHED_CAPACITY_[SHIFT/SCALE].
>
> Cheers,
>
> -- Dietmar
>
> [...]
>
next prev parent reply other threads:[~2015-09-08 14:02 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
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 [this message]
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='CAKfTPtBNZL=VkuS3TVeQOptH+4aZR9aMyC2CfAxn8oSrmmFa2w@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.