need_active_balance() gates the asymmetric packing based due to power
save logic, but for packing we don't care.
This marks the type of balanace we are attempting to do perform from
f_b_g() and stops need_active_balance() power save logic gating a
balance in the asymmetric packing case.
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
kernel/sched_fair.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
Index: linux-2.6-ozlabs/kernel/sched_fair.c
===================================================================
--- linux-2.6-ozlabs.orig/kernel/sched_fair.c
+++ linux-2.6-ozlabs/kernel/sched_fair.c
@@ -91,6 +91,13 @@ const_debug unsigned int sysctl_sched_mi
static const struct sched_class fair_sched_class;
+enum balance_type {
+ BALANCE_NONE = 0,
+ BALANCE_LOAD,
+ BALANCE_POWER,
+ BALANCE_PACKING
+};
+
/**************************************************************
* CFS operations on generic schedulable entities:
*/
@@ -2783,7 +2790,8 @@ static inline void calculate_imbalance(s
static struct sched_group *
find_busiest_group(struct sched_domain *sd, int this_cpu,
unsigned long *imbalance, enum cpu_idle_type idle,
- int *sd_idle, const struct cpumask *cpus, int *balance)
+ int *sd_idle, const struct cpumask *cpus, int *balance,
+ enum balance_type *bt)
{
struct sd_lb_stats sds;
@@ -2808,6 +2816,7 @@ find_busiest_group(struct sched_domain *
if (!(*balance))
goto ret;
+ *bt = BALANCE_PACKING;
if ((idle == CPU_IDLE || idle == CPU_NEWLY_IDLE) &&
check_asym_packing(sd, &sds, this_cpu, imbalance))
return sds.busiest;
@@ -2828,6 +2837,7 @@ find_busiest_group(struct sched_domain *
/* Looks like there is an imbalance. Compute it */
calculate_imbalance(&sds, this_cpu, imbalance);
+ *bt = BALANCE_LOAD;
return sds.busiest;
out_balanced:
@@ -2835,10 +2845,12 @@ out_balanced:
* There is no obvious imbalance. But check if we can do some balancing
* to ...This explanation lacks a how/why. So the problem is that need_active_balance() ends up returning false and prevents the active balance from pulling a task to a lower available SMT At the very least this wants more comments in the code. I'm not really charmed by having to add yet another variable to pass around that mess, --
Yeah, the current case only ever reads the balance type in the != BALANCE_POWER so a full enum might be overkill, but I though it might come in useful for someone else. Updated patch below. Mikey [PATCH 4/5] sched: fix need_active_balance() from preventing asymmetric packing need_active_balance() prevents a task being pulled onto a newly idle package in an attempt to completely free it so it can be powered down. Hence it returns false to load_balance() and prevents the active balance from occurring. Unfortunately, when asymmetric packing is enabled at the sibling level this power save logic is preventing the packing balance from moving a task to a lower idle thread. At the sibling level SD_SHARE_CPUPOWER and parent(SD_POWERSAVINGS_BALANCE) are enabled and the domain is also non-idle (since we have at least 1 task we are trying to move down). Hence the following code, prevents the an active balance from occurring: if (!sd_idle && sd->flags & SD_SHARE_CPUPOWER && !test_sd_parent(sd, SD_POWERSAVINGS_BALANCE)) return 0; To fix this, this patch classifies the type of balance we are attempting to perform into none, load, power and packing based on what function finds busiest in f_b_g(). This classification is then used by need_active_balance() to prevent the above power saving logic from stopping a balance due to asymmetric packing. This ensures tasks can be correctly moved down to lower sibling threads. Signed-off-by: Michael Neuling <mikey@neuling.org> --- kernel/sched_fair.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-) Index: linux-2.6-ozlabs/kernel/sched_fair.c =================================================================== --- linux-2.6-ozlabs.orig/kernel/sched_fair.c +++ linux-2.6-ozlabs/kernel/sched_fair.c @@ -91,6 +91,14 @@ const_debug unsigned int sysctl_sched_mi static const struct sched_class fair_sched_class; +/* Enum to classify the type of balance we are ...
