linux-kernel - Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 8 Sep 2015 15:27:05 +0100
From:	Dietmar Eggemann <dietmar.eggemann@....com>
To:	Vincent Guittot <vincent.guittot@...aro.org>
Cc:	Steve Muckle <steve.muckle@...aro.org>,
	Morten Rasmussen <Morten.Rasmussen@....com>,
	"peterz@...radead.org" <peterz@...radead.org>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
	"yuyang.du@...el.com" <yuyang.du@...el.com>,
	"mturquette@...libre.com" <mturquette@...libre.com>,
	"rjw@...ysocki.net" <rjw@...ysocki.net>,
	Juri Lelli <Juri.Lelli@....com>,
	"sgurrappadi@...dia.com" <sgurrappadi@...dia.com>,
	"pang.xunlei@....com.cn" <pang.xunlei@....com.cn>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by
 capacity_orig



On 08/09/15 15:01, Vincent Guittot wrote:
> On 8 September 2015 at 14:50, Dietmar Eggemann <dietmar.eggemann@....com> wrote:
>> On 08/09/15 08:22, Vincent Guittot wrote:
>>> On 7 September 2015 at 20:54, Dietmar Eggemann <dietmar.eggemann@....com> wrote:
>>>> On 07/09/15 17:21, Vincent Guittot wrote:
>>>>> On 7 September 2015 at 17:37, Dietmar Eggemann <dietmar.eggemann@....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

No arguing here, I just called this a SHIFT/SCALE problem.

[...]

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/