All of lore.kernel.org
 help / color / mirror / Atom feed
From: morten.rasmussen@arm.com (Morten Rasmussen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 4/6] sched: get CPU's usage statistic
Date: Fri, 26 Sep 2014 16:58:03 +0100	[thread overview]
Message-ID: <20140926155803.GA23693@e103034-lin> (raw)
In-Reply-To: <CAKfTPtCzVRY4q184nqHSL7zFh1CvF009p8q8EPFCZxDFMx9TeQ@mail.gmail.com>

On Fri, Sep 26, 2014 at 01:17:43PM +0100, Vincent Guittot wrote:
> On 25 September 2014 21:05, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
> > On 23/09/14 17:08, Vincent Guittot wrote:
> >> Monitor the usage level of each group of each sched_domain level. The usage is
> >> the amount of cpu_capacity that is currently used on a CPU or group of CPUs.
> >> We use the utilization_load_avg to evaluate the usage level of each group.
> >>
> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> >> ---
> >>  kernel/sched/fair.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> index 2cf153d..4097e3f 100644
> >> --- a/kernel/sched/fair.c
> >> +++ b/kernel/sched/fair.c
> >> @@ -4523,6 +4523,17 @@ static int select_idle_sibling(struct task_struct *p, int target)
> >>       return target;
> >>  }
> >>
> >> +static int get_cpu_usage(int cpu)
> >> +{
> >> +     unsigned long usage = cpu_rq(cpu)->cfs.utilization_load_avg;
> >> +     unsigned long capacity = capacity_orig_of(cpu);
> >> +
> >> +     if (usage >= SCHED_LOAD_SCALE)
> >> +             return capacity + 1;
> >
> > Why you are returning rq->cpu_capacity_orig + 1 (1025) in case
> > utilization_load_avg is greater or equal than 1024 and not usage or
> > (usage * capacity) >> SCHED_LOAD_SHIFT too?
> 
> The usage can't be higher than the full capacity of the CPU because
> it's about the running time on this CPU. Nevertheless, usage can be
> higher than SCHED_LOAD_SCALE because of unfortunate rounding in
> avg_period and running_load_avg or just after migrating tasks until
> the average stabilizes with the new running time.

I fully agree that the cpu usage should be capped to capacity, but why
do you return capacity + 1? I would just return capacity, no?

Now that you have gotten rid of 'usage' everywhere else, shouldn't this
function be renamed to get_cpu_utilization()?

WARNING: multiple messages have this Message-ID (diff)
From: Morten Rasmussen <morten.rasmussen@arm.com>
To: Vincent Guittot <vincent.guittot@linaro.org>
Cc: Dietmar Eggemann <Dietmar.Eggemann@arm.com>,
	"peterz@infradead.org" <peterz@infradead.org>,
	"mingo@kernel.org" <mingo@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"preeti@linux.vnet.ibm.com" <preeti@linux.vnet.ibm.com>,
	"linux@arm.linux.org.uk" <linux@arm.linux.org.uk>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"riel@redhat.com" <riel@redhat.com>,
	"efault@gmx.de" <efault@gmx.de>,
	"nicolas.pitre@linaro.org" <nicolas.pitre@linaro.org>,
	"linaro-kernel@lists.linaro.org" <linaro-kernel@lists.linaro.org>,
	"daniel.lezcano@linaro.org" <daniel.lezcano@linaro.org>,
	"pjt@google.com" <pjt@google.com>,
	"bsegall@google.com" <bsegall@google.com>
Subject: Re: [PATCH v6 4/6] sched: get CPU's usage statistic
Date: Fri, 26 Sep 2014 16:58:03 +0100	[thread overview]
Message-ID: <20140926155803.GA23693@e103034-lin> (raw)
In-Reply-To: <CAKfTPtCzVRY4q184nqHSL7zFh1CvF009p8q8EPFCZxDFMx9TeQ@mail.gmail.com>

On Fri, Sep 26, 2014 at 01:17:43PM +0100, Vincent Guittot wrote:
> On 25 September 2014 21:05, Dietmar Eggemann <dietmar.eggemann@arm.com> wrote:
> > On 23/09/14 17:08, Vincent Guittot wrote:
> >> Monitor the usage level of each group of each sched_domain level. The usage is
> >> the amount of cpu_capacity that is currently used on a CPU or group of CPUs.
> >> We use the utilization_load_avg to evaluate the usage level of each group.
> >>
> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> >> ---
> >>  kernel/sched/fair.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> index 2cf153d..4097e3f 100644
> >> --- a/kernel/sched/fair.c
> >> +++ b/kernel/sched/fair.c
> >> @@ -4523,6 +4523,17 @@ static int select_idle_sibling(struct task_struct *p, int target)
> >>       return target;
> >>  }
> >>
> >> +static int get_cpu_usage(int cpu)
> >> +{
> >> +     unsigned long usage = cpu_rq(cpu)->cfs.utilization_load_avg;
> >> +     unsigned long capacity = capacity_orig_of(cpu);
> >> +
> >> +     if (usage >= SCHED_LOAD_SCALE)
> >> +             return capacity + 1;
> >
> > Why you are returning rq->cpu_capacity_orig + 1 (1025) in case
> > utilization_load_avg is greater or equal than 1024 and not usage or
> > (usage * capacity) >> SCHED_LOAD_SHIFT too?
> 
> The usage can't be higher than the full capacity of the CPU because
> it's about the running time on this CPU. Nevertheless, usage can be
> higher than SCHED_LOAD_SCALE because of unfortunate rounding in
> avg_period and running_load_avg or just after migrating tasks until
> the average stabilizes with the new running time.

I fully agree that the cpu usage should be capped to capacity, but why
do you return capacity + 1? I would just return capacity, no?

Now that you have gotten rid of 'usage' everywhere else, shouldn't this
function be renamed to get_cpu_utilization()?

  reply	other threads:[~2014-09-26 15:58 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-23 16:07 [PATCH v6 0/6] sched: consolidation of cpu_capacity Vincent Guittot
2014-09-23 16:07 ` Vincent Guittot
2014-09-23 16:08 ` [PATCH v6 1/6] sched: add per rq cpu_capacity_orig Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-09-23 16:08 ` [PATCH v6 2/6] sched: move cfs task on a CPU with higher capacity Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-09-23 16:08 ` [PATCH v6 3/6] sched: add utilization_avg_contrib Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-10-03 14:15   ` Peter Zijlstra
2014-10-03 14:15     ` Peter Zijlstra
2014-10-03 14:44     ` Vincent Guittot
2014-10-03 14:44       ` Vincent Guittot
2014-10-03 14:36   ` Peter Zijlstra
2014-10-03 14:36     ` Peter Zijlstra
2014-10-03 14:51     ` Vincent Guittot
2014-10-03 14:51       ` Vincent Guittot
2014-10-03 15:14       ` Peter Zijlstra
2014-10-03 15:14         ` Peter Zijlstra
2014-10-03 16:05         ` Morten Rasmussen
2014-10-03 16:05           ` Morten Rasmussen
2014-09-23 16:08 ` [PATCH v6 4/6] sched: get CPU's usage statistic Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-09-25 19:05   ` Dietmar Eggemann
2014-09-25 19:05     ` Dietmar Eggemann
2014-09-26 12:17     ` Vincent Guittot
2014-09-26 12:17       ` Vincent Guittot
2014-09-26 15:58       ` Morten Rasmussen [this message]
2014-09-26 15:58         ` Morten Rasmussen
2014-09-26 19:57       ` Dietmar Eggemann
2014-09-26 19:57         ` Dietmar Eggemann
2014-11-21  5:36       ` Wanpeng Li
2014-11-21  5:36         ` Wanpeng Li
2014-11-21 12:17         ` Vincent Guittot
2014-11-21 12:17           ` Vincent Guittot
2014-09-23 16:08 ` [PATCH v6 5/6] sched: replace capacity_factor by usage Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-09-24 17:48   ` Dietmar Eggemann
2014-09-24 17:48     ` Dietmar Eggemann
2014-09-25  8:35     ` Vincent Guittot
2014-09-25  8:35       ` Vincent Guittot
2014-09-25 19:19       ` Dietmar Eggemann
2014-09-25 19:19         ` Dietmar Eggemann
2014-09-26 12:39         ` Vincent Guittot
2014-09-26 12:39           ` Vincent Guittot
2014-09-26 14:00           ` Dietmar Eggemann
2014-09-26 14:00             ` Dietmar Eggemann
2014-09-25  8:38   ` Vincent Guittot
2014-09-25  8:38     ` Vincent Guittot
2014-09-29 13:39   ` Dietmar Eggemann
2014-09-29 13:39     ` Dietmar Eggemann
2014-10-02 16:57   ` Morten Rasmussen
2014-10-02 16:57     ` Morten Rasmussen
2014-10-03  7:24     ` Vincent Guittot
2014-10-03  7:24       ` Vincent Guittot
2014-10-03  9:35       ` Morten Rasmussen
2014-10-03  9:35         ` Morten Rasmussen
2014-10-03 12:50         ` Vincent Guittot
2014-10-03 12:50           ` Vincent Guittot
2014-11-23  0:22           ` Wanpeng Li
2014-11-23  0:22             ` Wanpeng Li
2014-11-24  8:26             ` Vincent Guittot
2014-11-24  8:26               ` Vincent Guittot
2014-10-03 15:38   ` Peter Zijlstra
2014-10-03 15:38     ` Peter Zijlstra
2014-10-06  8:55     ` Vincent Guittot
2014-10-06  8:55       ` Vincent Guittot
2014-09-23 16:08 ` [PATCH v6 6/6] sched: add SD_PREFER_SIBLING for SMT level Vincent Guittot
2014-09-23 16:08   ` Vincent Guittot
2014-09-24 12:27   ` Preeti U Murthy
2014-09-24 12:27     ` Preeti U Murthy
2014-09-25 12:10     ` Vincent Guittot
2014-09-25 12:10       ` 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=20140926155803.GA23693@e103034-lin \
    --to=morten.rasmussen@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.