Re: [Patch v3 1/6] sched/fair: Determine active load balance for SMT sched groups

[Tim Chen <tim.c.chen@linux.intel.com>]

> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index 87317634fab2..f636d6c09dc6 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -8279,6 +8279,11 @@ enum group_type {
> >  	 * more powerful CPU.
> >  	 */
> >  	group_misfit_task,
> > +	/*
> > +	 * Balance SMT group that's fully busy. Can benefit from migration
> > +	 * a task on SMT with busy sibling to another CPU on idle core.
> > +	 */
> > +	group_smt_balance,
> 
> Would it make sense to move smt_balance?, s.t. we get:
> 
> group_has_spare < group_smt_balance < group_fully_busy
> 
> Conceptually I would be more intuitive to me like this as the
> smt_balance groups are more busy than has_spare ones, but less busy
> then fully_busy ones.
> 
>  From a functional perspective I could also see some impact when
> update_sd_pick_busiest compares the group types. In that case we
> would remove tasks from fully busy groups before moving them
> from smt_balance groups. Not sure which way would be to prefer
> to increase overall throughput.
> 
> Since smt_balance is only selected if the group has SMT, this
> should still not pull the last task off of a non-SMT CPU.
> 
> 

I think you have similar concerns as Shrikanth on this patch.
Can you see if my fix to update_sd_pick_busiest() in my reply
to Shrikanth addresses what you have in mind.

Tim