From: tim.c.chen@linux.intel.com (Tim Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 03/12] sched: fix avg_load computation
Date: Thu, 04 Sep 2014 09:26:51 -0700 [thread overview]
Message-ID: <1409848011.31585.35.camel@schen9-desk2.jf.intel.com> (raw)
In-Reply-To: <CAKfTPtCd02JLyd335WUpU5RRUYmS=Ni9S0U_U1uDsYjs59oQ1A@mail.gmail.com>
On Thu, 2014-09-04 at 09:17 +0200, Vincent Guittot wrote:
> On 4 September 2014 01:43, Tim Chen <tim.c.chen@linux.intel.com> wrote:
> > On Wed, 2014-09-03 at 13:09 +0200, Vincent Guittot wrote:
> >> On 30 August 2014 14:00, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote:
> >> > Hi Vincent,
> >> >
> >> > On 08/26/2014 04:36 PM, Vincent Guittot wrote:
> >> >> The computation of avg_load and avg_load_per_task should only takes into
> >> >> account the number of cfs tasks. The non cfs task are already taken into
> >> >> account by decreasing the cpu's capacity and they will be tracked in the
> >> >> CPU's utilization (group_utilization) of the next patches
> >> >>
> >> >> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> >> >> ---
> >> >> kernel/sched/fair.c | 4 ++--
> >> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> >> >> index 87b9dc7..b85e9f7 100644
> >> >> --- a/kernel/sched/fair.c
> >> >> +++ b/kernel/sched/fair.c
> >> >> @@ -4092,7 +4092,7 @@ static unsigned long capacity_of(int cpu)
> >> >> static unsigned long cpu_avg_load_per_task(int cpu)
> >> >> {
> >> >> struct rq *rq = cpu_rq(cpu);
> >> >> - unsigned long nr_running = ACCESS_ONCE(rq->nr_running);
> >> >> + unsigned long nr_running = ACCESS_ONCE(rq->cfs.h_nr_running);
> >> >> unsigned long load_avg = rq->cfs.runnable_load_avg;
> >> >>
> >> >> if (nr_running)
> >> >> @@ -5985,7 +5985,7 @@ static inline void update_sg_lb_stats(struct lb_env *env,
> >> >> load = source_load(i, load_idx);
> >> >>
> >> >> sgs->group_load += load;
> >> >> - sgs->sum_nr_running += rq->nr_running;
> >> >> + sgs->sum_nr_running += rq->cfs.h_nr_running;
> >> >>
> >> >> if (rq->nr_running > 1)
> >> >> *overload = true;
> >> >>
> >> >
> >> > Why do we probe rq->nr_running while we do load balancing? Should not we
> >> > be probing cfs_rq->nr_running instead? We are interested after all in
> >> > load balancing fair tasks right? The reason I ask this is, I was
> >> > wondering if we need to make the above similar change in more places in
> >> > load balancing.
> >>
> >> Hi Preeti,
> >>
> >> Yes, we should probably the test rq->cfs.h_nr_running > 0 before
> >> setting overload.
> >>
> >
> > The overload indicator is used for knowing when we can totally avoid
> > load balancing to a cpu that is about to go idle.
> > We can avoid load balancing when no cpu has more than 1 task. So if you
> > have say just one fair task and multiple deadline tasks on a cpu,
> > and another cpu about to go idle, you should turn on normal load
> > balancing in the idle path by setting overload to true.
>
> The newly idle load balancing can only affect CFS tasks so triggering
> a load_balance because a cpu is overloaded by rt tasks only, will not
> change anything.
>
> >
> > So setting overload should be set based on rq->nr_running and not on
> > rq->cfs.h_nr_running.
>
> We should probably use both values like below
>
> if ((rq->nr_running > 1) && ( rq->cfs.h_nr_running > 0))
Yes, this modification is the correct one that takes care of the
condition I objected to previously.
Thanks.
Tim
next prev parent reply other threads:[~2014-09-04 16:26 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-26 11:06 [PATCH v5 00/12] sched: consolidation of cpu_capacity Vincent Guittot
2014-08-26 11:06 ` [PATCH v5 01/12] sched: fix imbalance flag reset Vincent Guittot
2014-08-26 11:06 ` [PATCH v5 02/12] sched: remove a wake_affine condition Vincent Guittot
2014-08-26 11:06 ` [PATCH v5 03/12] sched: fix avg_load computation Vincent Guittot
2014-08-30 12:00 ` Preeti U Murthy
2014-09-03 11:09 ` Vincent Guittot
2014-09-03 23:43 ` Tim Chen
2014-09-04 7:17 ` Vincent Guittot
2014-09-04 16:26 ` Tim Chen [this message]
2014-09-05 11:10 ` Preeti U Murthy
2014-08-26 11:06 ` [PATCH v5 04/12] sched: Allow all archs to set the capacity_orig Vincent Guittot
2014-08-27 13:12 ` Kamalesh Babulal
2014-08-30 17:07 ` Preeti U Murthy
2014-09-01 8:05 ` Vincent Guittot
2014-09-03 8:41 ` Preeti U Murthy
2014-09-10 13:50 ` Peter Zijlstra
2014-09-10 14:22 ` Vincent Guittot
2014-09-11 10:36 ` Preeti U Murthy
2014-08-26 11:06 ` [PATCH v5 05/12] ARM: topology: use new cpu_capacity interface Vincent Guittot
2014-09-11 18:52 ` Nicolas Pitre
2014-08-26 11:06 ` [PATCH v5 06/12] sched: add per rq cpu_capacity_orig Vincent Guittot
2014-08-27 13:32 ` Kamalesh Babulal
2014-08-28 7:34 ` Vincent Guittot
2014-09-10 13:53 ` Peter Zijlstra
2014-09-10 14:19 ` Vincent Guittot
2014-09-11 19:02 ` Nicolas Pitre
2014-09-15 21:22 ` Vincent Guittot
2014-08-26 11:06 ` [PATCH v5 07/12] sched: test the cpu's capacity in wake affine Vincent Guittot
2014-09-10 14:19 ` Peter Zijlstra
2014-08-26 11:06 ` [PATCH v5 08/12] sched: move cfs task on a CPU with higher capacity Vincent Guittot
2014-08-30 17:50 ` Preeti U Murthy
2014-09-01 8:45 ` Vincent Guittot
2014-09-03 9:11 ` Preeti U Murthy
2014-09-03 11:44 ` Vincent Guittot
2014-09-03 12:26 ` Preeti U Murthy
2014-09-03 12:49 ` Vincent Guittot
2014-09-11 9:27 ` Peter Zijlstra
2014-09-05 12:06 ` Preeti U Murthy
2014-09-05 12:24 ` Vincent Guittot
2014-09-11 10:07 ` Peter Zijlstra
2014-09-11 11:20 ` Vincent Guittot
2014-09-11 10:13 ` Peter Zijlstra
2014-09-11 12:14 ` Vincent Guittot
2014-09-11 11:54 ` Peter Zijlstra
2014-08-26 11:06 ` [PATCH v5 09/12] sched: add usage_load_avg Vincent Guittot
2014-09-04 7:34 ` [PATCH v5 09/11] " Vincent Guittot
2014-09-11 11:17 ` Peter Zijlstra
2014-09-11 11:17 ` [PATCH v5 09/12] " Peter Zijlstra
2014-09-11 12:18 ` Vincent Guittot
2014-09-11 12:20 ` Vincent Guittot
2014-09-15 19:15 ` Morten Rasmussen
2014-09-15 22:33 ` Vincent Guittot
2014-08-26 11:06 ` [PATCH v5 10/12] sched: get CPU's utilization statistic Vincent Guittot
2014-09-11 12:34 ` Peter Zijlstra
2014-09-11 13:07 ` Vincent Guittot
2014-09-11 14:04 ` Peter Zijlstra
2014-09-11 19:17 ` Nicolas Pitre
2014-09-12 7:41 ` Vincent Guittot
2014-09-15 19:45 ` Morten Rasmussen
2014-09-16 22:43 ` Vincent Guittot
2014-09-15 19:28 ` Morten Rasmussen
2014-08-26 11:06 ` [PATCH v5 11/12] sched: replace capacity_factor by utilization Vincent Guittot
2014-09-11 15:39 ` Peter Zijlstra
2014-09-11 16:15 ` Peter Zijlstra
2014-09-11 17:26 ` Vincent Guittot
2014-09-14 19:41 ` Peter Zijlstra
2014-09-14 19:51 ` Peter Zijlstra
2014-09-15 11:42 ` Peter Zijlstra
2014-09-15 19:07 ` Nicolas Pitre
2014-09-15 20:01 ` Peter Zijlstra
2014-09-17 18:45 ` Morten Rasmussen
2014-09-17 18:58 ` Morten Rasmussen
2014-09-17 23:03 ` Peter Zijlstra
2014-09-15 22:14 ` Vincent Guittot
2014-09-15 22:18 ` Vincent Guittot
2014-09-17 22:25 ` Peter Zijlstra
2014-09-18 1:32 ` Vincent Guittot
2014-09-16 17:00 ` Dietmar Eggemann
2014-08-26 11:06 ` [PATCH v5 12/12] sched: add SD_PREFER_SIBLING for SMT level 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=1409848011.31585.35.camel@schen9-desk2.jf.intel.com \
--to=tim.c.chen@linux.intel.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 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).