Re: [PATCH 6/7] sched: Rename root_domain->overload to should_idle_balance

[Valentin Schneider]

On 02/16/2018 09:49 AM, Peter Zijlstra wrote:
> On Fri, Feb 16, 2018 at 10:14:02AM +0100, Juri Lelli wrote:
>>> diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
>>> index 7d324b706e67..4215438667e5 100644
>>> --- a/kernel/sched/sched.h
>>> +++ b/kernel/sched/sched.h
>>> @@ -650,8 +650,12 @@ struct root_domain {
>>>  	cpumask_var_t span;
>>>  	cpumask_var_t online;
>>>  
>>> -	/* Indicate more than one runnable task for any CPU */
>>> -	bool overload;
>>> +	/*
>>> +	 * Indicate whether the idle balance can be used to solve
>>> +	 * imbalance within the root domain.
>>> +	 * e.g. There is more than one runnable task for any CPU
>>> +	 */
>>> +	bool should_idle_balance;
>>
>> Current name is however consistent with RT/DL's naming convention

I saw that it was already used elsewhere in fair but didn't know about
RT/DL, thanks for pointing that out.

> 
> Yeah, not a fan either. We've consistently used the term to mean
> nr_running>1. The thing to fix there is the stupid bool, not the name.
> 

So yeah the other thing that doesn't help here is that we're cramming 
several meanings into rq->rd->overload:
- is there an overloaded group
- is there a group with misfit task(s)

So it didn't make sense to keep it named "overload". Perhaps a better way of
handling this would be to keep exposing which is which instead of merging it
all in a bool. Something along those lines:

@update_sg_lb_stats():
[...]
 nr_running = rq->nr_running;
 if (nr_running > 1)
-    *overload = true;
+    sds->balance_status |= LB_STATUS_OVERLOAD

[...]

 if (env->sd->flags & SD_ASYM_CPUCAPACITY &&
 !sgs->group_misfit_task_load && rq->misfit_task_load) {
     sgs->group_misfit_task_load = rq->misfit_task_load;
-     *should_idle_balance = true;
+     sds->balance_status |= LB_STATUS_MISFIT
 }

@update_sd_lb_stats():

[...]
 if (!env->sd->parent) {
     /* update overload indicator if we are at root domain */
-     if (env->dst_rq->rd->overload != overload)
-         env->dst_rq->rd->overload = overload;
+     if (env->dst_rq->rd->balance_status != sds->balance_status)
+         env->dst_rq->rd->balance_status = sds->balance_status
 }