From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755318AbbIHO1P (ORCPT ); Tue, 8 Sep 2015 10:27:15 -0400 Received: from eu-smtp-delivery-143.mimecast.com ([146.101.78.143]:38139 "EHLO eu-smtp-delivery-143.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755110AbbIHO1L convert rfc822-to-8bit (ORCPT ); Tue, 8 Sep 2015 10:27:11 -0400 Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig To: Vincent Guittot References: <1439569394-11974-1-git-send-email-morten.rasmussen@arm.com> <1439569394-11974-6-git-send-email-morten.rasmussen@arm.com> <55E8DD00.2030706@linaro.org> <55EDAF43.30500@arm.com> <55EDDD5A.70904@arm.com> <55EED99E.2040100@arm.com> Cc: Steve Muckle , Morten Rasmussen , "peterz@infradead.org" , "mingo@redhat.com" , "daniel.lezcano@linaro.org" , "yuyang.du@intel.com" , "mturquette@baylibre.com" , "rjw@rjwysocki.net" , Juri Lelli , "sgurrappadi@nvidia.com" , "pang.xunlei@zte.com.cn" , "linux-kernel@vger.kernel.org" From: Dietmar Eggemann Message-ID: <55EEF039.4040509@arm.com> Date: Tue, 8 Sep 2015 15:27:05 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: X-OriginalArrivalTime: 08 Sep 2015 14:27:05.0843 (UTC) FILETIME=[6FD0A430:01D0EA42] X-MC-Unique: xH5E4RkYTqeaAtrnEzJhPA-1 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/09/15 15:01, Vincent Guittot wrote: > On 8 September 2015 at 14:50, Dietmar Eggemann wrote: >> On 08/09/15 08:22, Vincent Guittot wrote: >>> On 7 September 2015 at 20:54, Dietmar Eggemann wrote: >>>> On 07/09/15 17:21, Vincent Guittot wrote: >>>>> On 7 September 2015 at 17:37, Dietmar Eggemann 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. [...]