From: Peter Zijlstra <peterz@infradead.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>,
LKML <linux-kernel@vger.kernel.org>,
Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>,
Gautham R Shenoy <ego@in.ibm.com>,
SureshSiddha <suresh.b.siddha@intel.com>,
"Pallipadi,Venkatesh" <venkatesh.pallipadi@intel.com>
Subject: [PATCH 7/6][RFC] sched: unify load_balance{,_newidle}()
Date: Wed, 23 Dec 2009 16:13:36 +0100 [thread overview]
Message-ID: <1261581216.4937.150.camel@laptop> (raw)
In-Reply-To: <20091217185021.684424629@chello.nl>
load_balance() and load_balance_newidle() look remarkably similar, one
key point they differ in is the condition on when to active balance.
So split out that logic into a separate function.
One side effect is that previously load_balance_newidle() used to fail and
return -1 under these conditions, whereas now it doesn't. I've not yet fully
figured out the whole -1 return case for either load_balance{,_newidle}().
It also differs in that sd->cache_nice_tries is now added on the
CPU_NEWLY_IDLE case.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
kernel/sched_fair.c | 115 ++++++++++++++++++++++++++--------------------------
1 file changed, 59 insertions(+), 56 deletions(-)
Index: linux-2.6/kernel/sched_fair.c
===================================================================
--- linux-2.6.orig/kernel/sched_fair.c
+++ linux-2.6/kernel/sched_fair.c
@@ -2816,6 +2816,39 @@ find_busiest_queue(struct sched_group *g
/* Working cpumask for load_balance and load_balance_newidle. */
static DEFINE_PER_CPU(cpumask_var_t, load_balance_tmpmask);
+static int need_active_balance(struct sched_domain *sd, int sd_idle, int idle)
+{
+ if (idle == CPU_NEWLY_IDLE) {
+ /*
+ * The only task running in a non-idle cpu can be moved to this
+ * cpu in an attempt to completely freeup the other CPU
+ * package.
+ *
+ * The package power saving logic comes from
+ * find_busiest_group(). If there are no imbalance, then
+ * f_b_g() will return NULL. However when sched_mc={1,2} then
+ * f_b_g() will select a group from which a running task may be
+ * pulled to this cpu in order to make the other package idle.
+ * If there is no opportunity to make a package idle and if
+ * there are no imbalance, then f_b_g() will return NULL and no
+ * action will be taken in load_balance_newidle().
+ *
+ * Under normal task pull operation due to imbalance, there
+ * will be more than one task in the source run queue and
+ * move_tasks() will succeed. ld_moved will be true and this
+ * active balance code will not be triggered.
+ */
+ if (!sd_idle && sd->flags & SD_SHARE_CPUPOWER &&
+ !test_sd_parent(sd, SD_POWERSAVINGS_BALANCE))
+ return 0;
+
+ if (sched_mc_power_savings < POWERSAVINGS_BALANCE_WAKEUP)
+ return 0;
+ }
+
+ return unlikely(sd->nr_balance_failed > sd->cache_nice_tries+2);
+}
+
/*
* Check this_cpu to ensure it is balanced within domain. Attempt to move
* tasks if there is an imbalance.
@@ -2902,8 +2935,7 @@ redo:
schedstat_inc(sd, lb_failed[idle]);
sd->nr_balance_failed++;
- if (unlikely(sd->nr_balance_failed > sd->cache_nice_tries+2)) {
-
+ if (need_active_balance(sd, sd_idle, idle)) {
raw_spin_lock_irqsave(&busiest->lock, flags);
/* don't kick the migration_thread, if the curr
@@ -3049,66 +3081,37 @@ redo:
int active_balance = 0;
schedstat_inc(sd, lb_failed[CPU_NEWLY_IDLE]);
- if (!sd_idle && sd->flags & SD_SHARE_CPUPOWER &&
- !test_sd_parent(sd, SD_POWERSAVINGS_BALANCE))
- return -1;
-
- if (sched_mc_power_savings < POWERSAVINGS_BALANCE_WAKEUP)
- return -1;
+ sd->nr_balance_failed++;
- if (sd->nr_balance_failed++ < 2)
- return -1;
+ if (need_active_balance(sd, sd_idle, CPU_NEWLY_IDLE)) {
+ double_lock_balance(this_rq, busiest);
- /*
- * The only task running in a non-idle cpu can be moved to this
- * cpu in an attempt to completely freeup the other CPU
- * package. The same method used to move task in load_balance()
- * have been extended for load_balance_newidle() to speedup
- * consolidation at sched_mc=POWERSAVINGS_BALANCE_WAKEUP (2)
- *
- * The package power saving logic comes from
- * find_busiest_group(). If there are no imbalance, then
- * f_b_g() will return NULL. However when sched_mc={1,2} then
- * f_b_g() will select a group from which a running task may be
- * pulled to this cpu in order to make the other package idle.
- * If there is no opportunity to make a package idle and if
- * there are no imbalance, then f_b_g() will return NULL and no
- * action will be taken in load_balance_newidle().
- *
- * Under normal task pull operation due to imbalance, there
- * will be more than one task in the source run queue and
- * move_tasks() will succeed. ld_moved will be true and this
- * active balance code will not be triggered.
- */
+ /*
+ * don't kick the migration_thread, if the curr
+ * task on busiest cpu can't be moved to this_cpu
+ */
+ if (!cpumask_test_cpu(this_cpu,
+ &busiest->curr->cpus_allowed)) {
+ double_unlock_balance(this_rq, busiest);
+ all_pinned = 1;
+ return ld_moved;
+ }
- /* Lock busiest in correct order while this_rq is held */
- double_lock_balance(this_rq, busiest);
+ if (!busiest->active_balance) {
+ busiest->active_balance = 1;
+ busiest->push_cpu = this_cpu;
+ active_balance = 1;
+ }
- /*
- * don't kick the migration_thread, if the curr
- * task on busiest cpu can't be moved to this_cpu
- */
- if (!cpumask_test_cpu(this_cpu, &busiest->curr->cpus_allowed)) {
double_unlock_balance(this_rq, busiest);
- all_pinned = 1;
- return ld_moved;
- }
-
- if (!busiest->active_balance) {
- busiest->active_balance = 1;
- busiest->push_cpu = this_cpu;
- active_balance = 1;
+ /*
+ * Should not call ttwu while holding a rq->lock
+ */
+ raw_spin_unlock(&this_rq->lock);
+ if (active_balance)
+ wake_up_process(busiest->migration_thread);
+ raw_spin_lock(&this_rq->lock);
}
-
- double_unlock_balance(this_rq, busiest);
- /*
- * Should not call ttwu while holding a rq->lock
- */
- raw_spin_unlock(&this_rq->lock);
- if (active_balance)
- wake_up_process(busiest->migration_thread);
- raw_spin_lock(&this_rq->lock);
-
} else
sd->nr_balance_failed = 0;
next prev parent reply other threads:[~2009-12-23 15:14 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-12-17 18:50 [PATCH 0/6] Some load-balancer cleanups Peter Zijlstra
2009-12-17 18:50 ` [PATCH 1/6] sched: Move load balance code into sched_fair.c Peter Zijlstra
2009-12-17 18:50 ` [PATCH 2/6] sched: Remove the sched_class load_balance methods Peter Zijlstra
2009-12-17 18:50 ` [PATCH 3/6] sched: Remove rq_iterator usage from load_balance_fair Peter Zijlstra
2009-12-17 18:50 ` [PATCH 4/6] sched: Remove rq_iterator from move_one_task Peter Zijlstra
2009-12-17 18:50 ` [PATCH 5/6] sched: Remove from fwd decls Peter Zijlstra
2009-12-17 18:50 ` [PATCH 6/6] sched: Add a lock break for PREEMPT=y Peter Zijlstra
2009-12-18 6:57 ` [PATCH 0/6] Some load-balancer cleanups Ingo Molnar
2009-12-18 9:37 ` Peter Zijlstra
2009-12-23 15:13 ` Peter Zijlstra [this message]
2009-12-24 4:43 ` [PATCH 7/6][RFC] sched: unify load_balance{,_newidle}() Mike Galbraith
2009-12-24 9:29 ` Peter Zijlstra
2009-12-24 10:01 ` Mike Galbraith
2009-12-24 10:09 ` Mike Galbraith
2009-12-24 10:16 ` Mike Galbraith
2009-12-24 10:16 ` Peter Zijlstra
2009-12-24 12:55 ` Peter Zijlstra
2009-12-24 17:43 ` Mike Galbraith
2009-12-23 15:13 ` [PATCH 8/6][RFC] sched: Remove load_balance_newidle() Peter Zijlstra
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=1261581216.4937.150.camel@laptop \
--to=peterz@infradead.org \
--cc=efault@gmx.de \
--cc=ego@in.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=suresh.b.siddha@intel.com \
--cc=svaidy@linux.vnet.ibm.com \
--cc=venkatesh.pallipadi@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox