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 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.