From: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
To: Peter Zijlstra <peterz@infradead.org>,
Mike Galbraith <bitbucket@online.de>,
Paul Turner <pjt@google.com>, Ingo Molnar <mingo@kernel.org>
Cc: Michael Neuling <mikey@neuling.org>,
linux-kernel@vger.kernel.org, Anton Blanchard <anton@samba.org>,
Preeti U Murthy <preeti@linux.vnet.ibm.com>,
linuxppc-dev@lists.ozlabs.org
Subject: [PATCH 1/3] sched: Fix nohz_kick_needed to consider the nr_busy of the parent domain's group
Date: Mon, 21 Oct 2013 17:14:42 +0530 [thread overview]
Message-ID: <20131021114442.13291.99344.stgit@drishya> (raw)
In-Reply-To: <20131021114002.13291.31478.stgit@drishya>
From: Preeti U Murthy <preeti@linux.vnet.ibm.com>
In nohz_kick_needed() there are checks around the flags
SD_SHARE_PKG_RESOURCES which decide to initiate nohz_balance if the
domains with this flag set have more than one cpu busy. Therefore at
every domain, a check has to be made on nr_busy of that domain. This
means the sum of the nr_busy of each group in that domain needs to be
checked, since nr_busy is a parameter which is associated with
a group. However in the current implementation of nohz_kick_needed(),
the nr_busy is being checked for just the group to which the cpu that
has initiated this check belongs to. This will give us the wrong count
of the number of busy cpus in that domain.
The following commit which fixed the sgp->nr_busy_cpus computation
actually exposed the bug in nohz_kick_needed() which worked when
nr_busy was incorrectly > 1
25f55d9d01ad7a7ad248fd5af1d22675ffd202c5
sched: Fix init NOHZ_IDLE flag
To illustrate the scenario, consider a core, whose domain will have
the SD_SHARE_PKG_RESOURCES set. We want to kick nohz_idle_balance when
we find that more than one thread in the core is busy. With the
current implementation of nohz_kick_needed(), at this domain(sd), the
nr_busy will be 1 always since it returns this parameter for
sd->groups which encompasses a single thread, while we want this
parameter for sd->parent->groups which will rightly point to the
number of busy threads in the core.
This patch also ensures that the order of check for
SD_SHARE_PKG_RESOURCE comes before the check for ASYM_PACKING.
Priority is given to avoid more than one busy thread in a core as much
as possible before attempting asymmetric packing.
Signed-off-by: Preeti U Murthy <preeti@linux.vnet.ibm.com>
Signed-off-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>
---
kernel/sched/fair.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 7c70201..12f0eab 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -5807,12 +5807,19 @@ static inline int nohz_kick_needed(struct rq *rq, int cpu)
rcu_read_lock();
for_each_domain(cpu, sd) {
- struct sched_group *sg = sd->groups;
- struct sched_group_power *sgp = sg->sgp;
- int nr_busy = atomic_read(&sgp->nr_busy_cpus);
-
- if (sd->flags & SD_SHARE_PKG_RESOURCES && nr_busy > 1)
- goto need_kick_unlock;
+ struct sched_domain *sd_parent = sd->parent;
+ struct sched_group *sg;
+ struct sched_group_power *sgp;
+ int nr_busy;
+
+ if (sd_parent) {
+ sg = sd_parent->groups;
+ sgp = sg->sgp;
+ nr_busy = atomic_read(&sgp->nr_busy_cpus);
+
+ if (sd->flags & SD_SHARE_PKG_RESOURCES && nr_busy > 1)
+ goto need_kick_unlock;
+ }
if (sd->flags & SD_ASYM_PACKING && nr_busy != sg->group_weight
&& (cpumask_first_and(nohz.idle_cpus_mask,
next prev parent reply other threads:[~2013-10-21 11:44 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-21 11:44 [PATCH 0/3] sched: Fixes for task placement in SMT threads Vaidyanathan Srinivasan
2013-10-21 11:44 ` Vaidyanathan Srinivasan [this message]
2013-10-22 14:35 ` [PATCH 1/3] sched: Fix nohz_kick_needed to consider the nr_busy of the parent domain's group Kamalesh Babulal
2013-10-22 16:40 ` Preeti U Murthy
2013-10-22 22:11 ` Peter Zijlstra
2013-10-23 4:00 ` Preeti U Murthy
2013-10-23 4:21 ` Preeti U Murthy
2013-10-23 9:50 ` Preeti U Murthy
2013-10-23 15:28 ` Vincent Guittot
2013-10-24 8:07 ` Preeti U Murthy
2013-10-28 13:50 ` Peter Zijlstra
2013-10-29 3:30 ` Preeti U Murthy
2013-10-29 13:26 ` Peter Zijlstra
2013-10-21 11:44 ` [PATCH 2/3] sched: Fix asymmetric scheduling for POWER7 Vaidyanathan Srinivasan
2013-10-21 22:55 ` Michael Neuling
2013-10-22 22:18 ` Peter Zijlstra
2013-10-21 11:45 ` [PATCH 3/3] sched: Aggressive balance in domains whose groups share package resources Vaidyanathan Srinivasan
2013-10-22 22:23 ` Peter Zijlstra
2013-10-24 4:04 ` Preeti U Murthy
2013-10-25 13:19 ` Preeti U Murthy
2013-10-28 15:53 ` Peter Zijlstra
2013-10-29 5:35 ` Preeti U Murthy
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=20131021114442.13291.99344.stgit@drishya \
--to=svaidy@linux.vnet.ibm.com \
--cc=anton@samba.org \
--cc=bitbucket@online.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mikey@neuling.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
--cc=pjt@google.com \
--cc=preeti@linux.vnet.ibm.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 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).