Re: [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts

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

On Tue, 2026-11-03 at 01:31 +0000, Qais Yousef wrote:
> On 11/2/26 02:15, Zhan Xusheng wrote:
> > From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> > 
> > invalid_llc_nr() uses fits_capacity() to compare a thread count with a
> > CPU count, only for its 20% margin.  Qais's series changes
> > fits_capacity() to take a CPU number as its second argument, and this
> > call would then pass the CPU count as a CPU number without a warning.
> > 
> > Give the check its own helper.  x * 5 < y * 4 is the same comparison as
> > x * 1280 < y * 1024, so there is no functional change.
> > 
> > Link: https://lore.kernel.org/all/20260504020003.71306-5-qyousef@layalina.io/
> > Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> Thanks for the patch. LGTM, although I hope we can do better.
> 
> Reviewed-by: Qais Yousef <qyousef@layalina.io>
> 
> > ---
> > This keeps the 20% margin.  Qais would rather not add hardcoded
> > margins, and llc_aggr_tolerance already scales the LLC size here, so
> > the margin could go, but that changes when a process stops being
> > aggregated.  Tim, Chen Yu, would you drop it?
> 
> Tim, Chen,
> 
> From capacity comparisons, we had issues that we move too slowly or too fast in
> some cases, that's why I was trying to remove this magic number.
> 
> I'd be surprised if this gives a global win.
> 
> Is this the best we can do? What is the problem you're working around here?

Qais,

The 20% margin is a deliberately conservative margin for the default
llc_aggr_tolerance. With no knowledge of whether a process's threads
share data, we only aggregate a process when its threads fit well within
the cores of one LLC, so workloads that don't benefit from aggregation
are not hurt by packing them into a single LLC.

Where aggregation is known to help, the user should raise
llc_aggr_tolerance (or, later, use hints like schedqos or the prctl
interface we proposed) rather than rely on a smaller margin. The
tolerance scales the limit (0.8x cores at the default, 1.6x at 2, and so
on), so the margin only sets how conservative the starting point is.

Thanks.

Tim 

> 
> I see we have other magic like llc_aggr_tolerance and llc_imb_pct etc.
> 
> > 
> >  kernel/sched/fair.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> > index cfc982205b98..dfd6528d18ee 100644
> > --- a/kernel/sched/fair.c
> > +++ b/kernel/sched/fair.c
> > @@ -1469,6 +1469,12 @@ static bool exceed_llc_capacity(struct sched_cache_group *grp, int cpu)
> >  	return false;
> >  }
> >  
> > +/* Are the running threads (in SMT siblings) below ~80% of the LLC's CPUs? */
> > +static inline bool fits_llc_nr(u64 nr_threads, unsigned int nr_cpus)
> > +{
> > +	return nr_threads * 5 < (u64)nr_cpus * 4;
> > +}
> > +
> >  static bool invalid_llc_nr(struct sched_cache_group *grp, struct task_struct *p,
> >  			   int cpu)
> >  {
> > @@ -1485,8 +1491,8 @@ static bool invalid_llc_nr(struct sched_cache_group *grp, struct task_struct *p,
> >  	if (scale == INT_MAX)
> >  		return false;
> >  
> > -	return !fits_capacity((READ_ONCE(grp->nr_running_avg) * cpu_smt_num_threads),
> > -			(scale * per_cpu(sd_llc_size, cpu)));
> > +	return !fits_llc_nr(READ_ONCE(grp->nr_running_avg) * cpu_smt_num_threads,
> > +			    scale * per_cpu(sd_llc_size, cpu));
> >  }
> >  
> >  /*
> > 
> > base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
>