Messages in this thread | | | Subject | Re: [PATCH 6/7] sched: Rename root_domain->overload to should_idle_balance | From | Valentin Schneider <> | Date | Fri, 16 Feb 2018 11:59:10 +0000 |
| |
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 }
| |