Re: [PATCH] sched/cache: Avoid publishing partial LLC utilization

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

On Mon, 2026-10-12 at 23:10 +0800, Jemmy Wong wrote:
> Hi Prateek,
> 
> > On Oct 12, 2026, at 11:34 AM, K Prateek Nayak <kprateek.nayak@amd.com> wrote:
> > 
> > Hello Jemmy,
> > 
> > On 10/10/2026 5:04 PM, Jemmy Wong wrote:
> > > --- a/kernel/sched/fair.c
> > > +++ b/kernel/sched/fair.c
> > > @@ -12121,6 +12121,13 @@ static void record_sg_llc_stats(struct lb_env *env,
> > > if (env->sd->child != rcu_dereference_all(per_cpu(sd_llc, env->dst_cpu)))
> > > return;
> > > 
> > > + /*
> > > +  * Affinity retries can remove CPUs from env->cpus. Do not publish
> > > +  * partial utilization against the capacity of the whole group.
> > > +  */
> > > + if (!cpumask_subset(sched_group_span(group), env->cpus))
> > > + return;
> > > +
> > 
> > nit.
> > 
> > When sched_balance_rq() does a "redo" and arrives here, env->flags
> > will always have LBF_ALL_PINNED set.
> > 
> > That would be a much cheaper check compared to cpumask_subset() to
> > ensure env->cpus covers all CPUs.
> > 
> > Thoughts?
> > 
> > > /*
> > >  * At this point we know this group spans a LLC domain.
> > >  * Record the statistic of this group in its corresponding
> > > --
> > > 2.25.1
> > 
> > -- 
> > Thanks and Regards,
> > Prateek
> 
> The redo part is right, but LBF_ALL_PINNED reflects the previous
> busiest rq, not whether the group being recorded is fully covered.
> 
> For example, if cpu1 in LLC0 {cpu0, cpu1} is rejected as all-pinned,
> we remove it from env->cpus and redo. The next scan still carries
> LBF_ALL_PINNED while recording LLC1 {cpu2, cpu3}, which remains fully
> covered. The flag check would skip that update; cpumask_subset()
> would not.
> 
> It also misses the hotplug window: when cpu1 in LLC0 {cpu0, cpu1}
> becomes inactive, a new balance pass can exclude cpu1 from env->cpus
> before the domains are rebuilt. The group span and capacity still
> cover both CPUs, but utilization is summed only over cpu0.
> 
> LBF_ALL_PINNED is clear on this first pass, so the flag check would
> allow that partial publication; cpumask_subset() would prevent it.
> 
> So I would keep cpumask_subset().

+ Chen Yu who wrote this function.

The partial group issue you pointed out is valid. 
 
I think Pratek has a good point.
Using LBF_ALL_PINNED is a cheap check and can eliminate
a lot of un-needed cpumask checks. We can do that first
and only resort to cpumask_subset() when necessary.  Something like:

          /*
           * A redo repeats stats published on the first pass. Hotplug can
           * still leave env->cpus partial, so util would not match capacity.
           */                                                                                                      
          if ((env->flags & LBF_ALL_PINNED) ||                                                                                                                        
              !cpumask_subset(sched_group_span(group), env->cpus))                                                                                                    
                  return;                                                    

You should also add a Fixes tag.

Tim

> 
> Thanks,
> Jemmy