Re: [PATCH 3/4] sched/fair: Consider SMT in ASYM_PACKING load balance

From: Ricardo Neri
Date: Fri Apr 09 2021 - 01:14:29 EST


On Thu, Apr 08, 2021 at 01:21:22PM +0200, Peter Zijlstra wrote:
> On Tue, Apr 06, 2021 at 04:17:51PM -0700, Ricardo Neri wrote:
> > On Tue, Apr 06, 2021 at 01:18:09PM +0200, Peter Zijlstra wrote:
> > > On Mon, Apr 05, 2021 at 09:11:07PM -0700, Ricardo Neri wrote:
> > > > +static bool cpu_group_is_smt(int cpu, struct sched_group *sg)
> > > > +{
> > > > +#ifdef CONFIG_SCHED_SMT
> > > > + if (!static_branch_likely(&sched_smt_present))
> > > > + return false;
> > > > +
> > > > + if (sg->group_weight == 1)
> > > > + return false;
> > > > +
> > > > + if (cpumask_weight(cpu_smt_mask(cpu)) == 1)
> > > > + return false;
> > >
> > > Please explain this condition. Why is it required?
> >
> > Thank you for your quick review Peter!
> >
> > Probably this is not required since the previous check verifies the
> > group weight, and the subsequent check makes sure that @sg matches the
> > SMT siblings of @cpu.
>
> So the thing is that cpumask_weight() can be fairly expensive, depending
> on how large the machine is.
>
> Now I suppose this mixing of SMT and !SMT cores is typical for 'small'
> machines (for now), but this is enabled for everything with ITMT on,
> which might very well include large systems.
>
> So yes, if it can go away, that'd be good.

Sure Peter, I think this check can be removed. I'll post a v2 with the
updates.

Thanks and BR,
Ricardo