On 30 August 2014 19:50, Preeti U Murthy preeti@linux.vnet.ibm.com wrote:
Hi Vincent,
On 08/26/2014 04:36 PM, Vincent Guittot wrote:
If the CPU is used for handling lot of IRQs, trig a load balance to check if it's worth moving its tasks on another CPU that has more capacity.
As a sidenote, this will note generate more spurious ilb because we already trig an ilb if there is more than 1 busy cpu. If this cpu is the only one that has a task, we will trig the ilb once for migrating the task.
The nohz_kick_needed function has been cleaned up a bit while adding the new test
Signed-off-by: Vincent Guittot vincent.guittot@linaro.org
kernel/sched/fair.c | 69 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 49 insertions(+), 20 deletions(-)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 18db43e..60ae1ce 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -6049,6 +6049,14 @@ static bool update_sd_pick_busiest(struct lb_env *env, return true; }
/*
* The group capacity is reduced probably because of activity from other
* sched class or interrupts which use part of the available capacity
*/
if ((sg->sgc->capacity_orig * 100) > (sgs->group_capacity *
env->sd->imbalance_pct))
Wouldn't the check on avg_load let us know if we are packing more tasks in this group than its capacity ? Isn't that the metric we are more interested in?
With this patch, we don't want to pack but we prefer to spread the task on another CPU than the one which handles the interruption if latter uses a significant part of the CPU capacity.
return true;
return false;
}
@@ -6534,13 +6542,23 @@ static int need_active_balance(struct lb_env *env) struct sched_domain *sd = env->sd;
if (env->idle == CPU_NEWLY_IDLE) {
int src_cpu = env->src_cpu; /* * ASYM_PACKING needs to force migrate tasks from busy but * higher numbered CPUs in order to pack all tasks in the * lowest numbered CPUs. */
if ((sd->flags & SD_ASYM_PACKING) && env->src_cpu > env->dst_cpu)
if ((sd->flags & SD_ASYM_PACKING) && src_cpu > env->dst_cpu)
return 1;
/*
* If the CPUs share their cache and the src_cpu's capacity is
* reduced because of other sched_class or IRQs, we trig an
* active balance to move the task
*/
if ((capacity_orig_of(src_cpu) * 100) > (capacity_of(src_cpu) *
sd->imbalance_pct)) return 1; }
@@ -6643,6 +6661,8 @@ static int load_balance(int this_cpu, struct rq *this_rq,
schedstat_add(sd, lb_imbalance[idle], env.imbalance);
env.src_cpu = busiest->cpu;
ld_moved = 0; if (busiest->nr_running > 1) { /*
@@ -6652,7 +6672,6 @@ static int load_balance(int this_cpu, struct rq *this_rq, * correctly treated as an imbalance. */ env.flags |= LBF_ALL_PINNED;
env.src_cpu = busiest->cpu; env.src_rq = busiest; env.loop_max = min(sysctl_sched_nr_migrate, busiest->nr_running);
@@ -7359,10 +7378,12 @@ static void nohz_idle_balance(struct rq *this_rq, enum cpu_idle_type idle)
/*
- Current heuristic for kicking the idle load balancer in the presence
- of an idle cpu is the system.
- of an idle cpu in the system.
- This rq has more than one task.
- At any scheduler domain level, this cpu's scheduler group has multiple
busy cpu's exceeding the group's capacity.
- This rq has at least one CFS task and the capacity of the CPU is
significantly reduced because of RT tasks or IRQs.
- At parent of LLC scheduler domain level, this cpu's scheduler group has
*/
multiple busy cpu.
- For SD_ASYM_PACKING, if the lower numbered cpu's in the scheduler
domain span are idle.
@@ -7372,9 +7393,10 @@ static inline int nohz_kick_needed(struct rq *rq) struct sched_domain *sd; struct sched_group_capacity *sgc; int nr_busy, cpu = rq->cpu;
bool kick = false; if (unlikely(rq->idle_balance))
return 0;
return false; /* * We may be recently in ticked or tickless idle mode. At the first
@@ -7388,38 +7410,45 @@ static inline int nohz_kick_needed(struct rq *rq) * balancing. */ if (likely(!atomic_read(&nohz.nr_cpus)))
return 0;
return false; if (time_before(now, nohz.next_balance))
return 0;
return false; if (rq->nr_running >= 2)
Will this check ^^ not catch those cases which this patch is targeting?
This patch is not about how many tasks are running but if the capacity of the CPU is reduced because of side activity like interruptions which are only visible in the capacity value (with IRQ_TIME_ACCOUNTING config) but not in nr_running. Even if the capacity is reduced because of RT tasks, nothing ensures that the RT task will be running when the tick fires
Regards, Vincent
Regards Preeti U Murthy
goto need_kick;
return true; rcu_read_lock(); sd = rcu_dereference(per_cpu(sd_busy, cpu));
if (sd) { sgc = sd->groups->sgc; nr_busy = atomic_read(&sgc->nr_busy_cpus);
if (nr_busy > 1)
goto need_kick_unlock;
if (nr_busy > 1) {
kick = true;
goto unlock;
}
}
sd = rcu_dereference(per_cpu(sd_asym, cpu));
sd = rcu_dereference(rq->sd);
if (sd) {
if ((rq->cfs.h_nr_running >= 1) &&
((rq->cpu_capacity * sd->imbalance_pct) <
(rq->cpu_capacity_orig * 100))) {
kick = true;
goto unlock;
}
}
sd = rcu_dereference(per_cpu(sd_asym, cpu)); if (sd && (cpumask_first_and(nohz.idle_cpus_mask, sched_domain_span(sd)) < cpu))
goto need_kick_unlock;
kick = true;
+unlock: rcu_read_unlock();
return 0;
-need_kick_unlock:
rcu_read_unlock();
-need_kick:
return 1;
return kick;
} #else static void nohz_idle_balance(struct rq *this_rq, enum cpu_idle_type idle) { }