Re: [PATCH 2/6] sched/fair: Check whether active load balance is needed in busiest group

[Peter Zijlstra]

On Fri, May 05, 2023 at 03:29:45PM -0700, Tim Chen wrote:
> On Fri, 2023-05-05 at 14:16 +0200, Peter Zijlstra wrote:
> > 
> > > +static inline bool asym_active_balance_busiest(struct lb_env *env, struct sd_lb_stats *sds)
> > > +{
> > > +	/*
> > > +	 * Don't balance to a group without spare capacity.
> > > +	 *
> > > +	 * Skip non asymmetric sched group balancing. That check
> > > +	 * is handled by code path handling imbalanced load between
> > > +	 * similar groups.
> > > +	 */
> > > +	if (env->idle == CPU_NOT_IDLE ||
> > > +	    sds->local_stat.group_type != group_has_spare ||
> > > +	    !asymmetric_groups(sds->local, sds->busiest))
> > > +		return false;
> > > +
> > > +	/*
> > > +	 * For SMT source group, pull when there are two or more
> > > +	 * tasks over-utilizing a core.
> > > +	 */
> > > +	if (sds->busiest->flags & SD_SHARE_CPUCAPACITY &&
> > > +	    sds->busiest_stat.sum_h_nr_running > 1)
> > > +		return true;
> > > +
> > > +	return false;
> > > +}
> > 
> > This all seems to be mixing two 'asymmetric' things in the 'asym'
> > namespace :/ One being the SD_ASYM_PACKING and then the above SMT/no-SMT
> > core thing.
> 
> Yeah, I am kind of abusing the "asymmetric" word.  However, the above
> code does try to set things up for the aysm_active_balance() code
> later. Any suggestion on better names for "asymmetric_groups()" and
> and "asym_active_balance_busiest()"? 
> 
> Perhaps "hybrid_groups()" and "hybrid_active_balance_busiest()"?

As per the other subthread; I really don't think this whole SMT vs
non-SMT should be in any way shape or form be related to hybrid.
Offlining siblings should really get you the same topology.

(and if that currently is not the case, that should be fixed)

(and also; we should probably add group_flags to
 /debug/sched/domains/cpuN/domainM/ so we can actually see what's what,
 because this seems to be a bit of a blind spot).

That also suggests the hybrid naming suggestion is not a very good one.

And I'll blame it being nearly 2am for not being able to come up with a
good suggestion, but I'm thinking it ought to have SMT in the name
somehow.