linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Vincent Guittot <vincent.guittot@linaro.org>
To: Dietmar Eggemann <dietmar.eggemann@arm.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Morten Rasmussen <Morten.Rasmussen@arm.com>,
	Yuyang Du <yuyang.du@intel.com>, Paul Turner <pjt@google.com>,
	Ben Segall <bsegall@google.com>
Subject: Re: [PATCH v2] sched/cfs: make util/load_avg more stable
Date: Tue, 25 Apr 2017 14:40:01 +0200	[thread overview]
Message-ID: <CAKfTPtA4wpkEdtO4xVNuNbLNh=Mwqgjqewj2CmV_YMjz2ethTA@mail.gmail.com> (raw)
In-Reply-To: <afe5d97e-368e-84aa-5693-5c02d55836a2@arm.com>

On 25 April 2017 at 13:05, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
> On 19/04/17 17:54, Vincent Guittot wrote:
>> In the current implementation of load/util_avg, we assume that the ongoing
>> time segment has fully elapsed, and util/load_sum is divided by LOAD_AVG_MAX,
>> even if part of the time segment still remains to run. As a consequence, this
>> remaining part is considered as idle time and generates unexpected variations
>> of util_avg of a busy CPU in the range ]1002..1024[ whereas util_avg should
>
> Why do you use the square brackets the other way around? Just curious.

This refers to the very beginning and very end of time segment formulas below.
That being said, 1024 is not reachable because at very end we have :
 1024*MAX_LOAD_AVG*y+1024*1023 = 1023,9997

1002 is not reachable because at very beg we have
1024*MAX_LOAD_AVG*y+ 1024*0 = 1002,0577

But we are working with integer so [1002..1024[ is probably more correct

>
> 1002 stands for 1024*y^1 w/ y = 4008/4096 or y^32 = 0.5, right ? Might
> be worth mentioning.
>
>> stay at 1023.
>>
>> In order to keep the metric stable, we should not consider the ongoing time
>> segment when computing load/util_avg but only the segments that have already
>> fully elapsed. Bu to not consider the current time segment adds unwanted
>> latency in the load/util_avg responsivness especially when the time is scaled
>> instead of the contribution. Instead of waiting for the current time segment
>> to have fully elapsed before accounting it in load/util_avg, we can already
>> account the elapsed part but change the range used to compute load/util_avg
>> accordingly.
>>
>> At the very beginning of a new time segment, the past segments have been
>> decayed and the max value is MAX_LOAD_AVG*y. At the very end of the current
>> time segment, the max value becomes 1024(us) + MAX_LOAD_AVG*y which is equal
>> to MAX_LOAD_AVG. In fact, the max value is
>> sa->period_contrib + MAX_LOAD_AVG*y at any time in the time segment.
>>
>> Taking advantage of the fact that MAX_LOAD_AVG*y == MAX_LOAD_AVG-1024, the
>> range becomes [0..MAX_LOAD_AVG-1024+sa->period_contrib].
>>
>> As the elapsed part is already accounted in load/util_sum, we update the max
>> value according to the current position in the time segment instead of
>> removing its contribution.
>
> Removing its contribution stands for '- 1024' of 'LOAD_AVG_MAX - 1024'
> which was added in patch 1/2?

removing its contribution refers to  "- sa->period_contrib * weight"
and "- (running * sa->period_contrib << SCHED_CAPACITY_SHIFT))"  in
patch 1/2 of the previous version

>
>>
>> Suggested-by: Peter Zijlstra <peterz@infradead.org>
>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>> ---
>>
>> Fold both patches in one
>>
>>  kernel/sched/fair.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 3f83a35..c3b8f0f 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -3017,12 +3017,12 @@ ___update_load_avg(u64 now, int cpu, struct sched_avg *sa,
>>       /*
>>        * Step 2: update *_avg.
>>        */
>> -     sa->load_avg = div_u64(sa->load_sum, LOAD_AVG_MAX);
>> +     sa->load_avg = div_u64(sa->load_sum, LOAD_AVG_MAX - 1024 + sa->period_contrib);
>>       if (cfs_rq) {
>>               cfs_rq->runnable_load_avg =
>> -                     div_u64(cfs_rq->runnable_load_sum, LOAD_AVG_MAX);
>> +                     div_u64(cfs_rq->runnable_load_sum, LOAD_AVG_MAX - 1024 + sa->period_contrib);
>>       }
>> -     sa->util_avg = sa->util_sum / LOAD_AVG_MAX;
>> +     sa->util_avg = sa->util_sum / (LOAD_AVG_MAX - 1024 + sa->period_contrib);
>>
>>       return 1;
>>  }
>>

  reply	other threads:[~2017-04-25 12:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-19 16:29 [PATCH 0/2] sched/cfs: make util/load_avg stable Vincent Guittot
2017-04-19 16:29 ` [PATCH 1/2] sched/cfs: make util/load_avg more stable Vincent Guittot
2017-04-19 16:29 ` [PATCH 2/2] sched/cfs: take into account current time segment Vincent Guittot
2017-04-19 16:44 ` [PATCH 0/2] sched/cfs: make util/load_avg stable Vincent Guittot
2017-04-19 16:44   ` [PATCH 1/2] sched/cfs: make util/load_avg more stable Vincent Guittot
2017-04-19 16:44   ` [PATCH 2/2] sched/cfs: take into account current time segment Vincent Guittot
2017-04-19 16:54 ` [PATCH v2] sched/cfs: make util/load_avg more stable Vincent Guittot
2017-04-25 11:05   ` Dietmar Eggemann
2017-04-25 12:40     ` Vincent Guittot [this message]
2017-04-25 14:53       ` Dietmar Eggemann
2017-04-25 15:17         ` Vincent Guittot

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='CAKfTPtA4wpkEdtO4xVNuNbLNh=Mwqgjqewj2CmV_YMjz2ethTA@mail.gmail.com' \
    --to=vincent.guittot@linaro.org \
    --cc=Morten.Rasmussen@arm.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=yuyang.du@intel.com \
    --subject='Re: [PATCH v2] sched/cfs: make util/load_avg more stable' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).