All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
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: Mon, 7 Sep 2015 19:54:18 +0100	[thread overview]
Message-ID: <55EDDD5A.70904@arm.com> (raw)
In-Reply-To: <CAKfTPtBmgc=7JMRcTL9VYdHxb7qgXBDFc62rb-jnSVCePwJNsg@mail.gmail.com>

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?

I always thought that scale_load_down() takes care of that.

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 ?

I would vote for removing this SCHED_LOAD_RESOLUTION thing completely so that we can
assume that load/util and capacity are always using 1024/10.

Cheers,

-- Dietmar

> 
> 
> Regards,
> Vincent
> 
> 
>>
>> LOAD (UTIL) and CAPACITY have the same SCALE and SHIFT values because
>> SCHED_LOAD_RESOLUTION is always defined to 0. scale_load() and
>> scale_load_down() are also NOPs so this area is probably
>> worth a separate clean-up.
>> Beyond that, I'm not sure if the current functionality is
>> broken if we use different SCALE and SHIFT values for LOAD and CAPACITY?
>>

[...]


  reply	other threads:[~2015-09-07 18:54 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 [this message]
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
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=55EDDD5A.70904@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=Juri.Lelli@arm.com \
    --cc=Morten.Rasmussen@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --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=vincent.guittot@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.