* [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts
@ 2026-11-02 2:15 Zhan Xusheng
2026-11-03 1:31 ` Qais Yousef
2026-11-03 2:27 ` Chen Yu
0 siblings, 2 replies; 4+ messages in thread
From: Zhan Xusheng @ 2026-11-02 2:15 UTC (permalink / raw)
To: peterz, mingo
Cc: qyousef, tim.c.chen, yu.c.chen, vincent.guittot, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
linux-kernel, zhanxusheng
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>
---
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?
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
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts
2026-11-02 2:15 [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts Zhan Xusheng
@ 2026-11-03 1:31 ` Qais Yousef
2026-11-03 2:24 ` Chen Yu
2026-11-03 2:27 ` Chen Yu
1 sibling, 1 reply; 4+ messages in thread
From: Qais Yousef @ 2026-11-03 1:31 UTC (permalink / raw)
To: Zhan Xusheng, peterz, mingo
Cc: tim.c.chen, yu.c.chen, vincent.guittot, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
linux-kernel, zhanxusheng
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?
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts
2026-11-03 1:31 ` Qais Yousef
@ 2026-11-03 2:24 ` Chen Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chen Yu @ 2026-11-03 2:24 UTC (permalink / raw)
To: Qais Yousef
Cc: Zhan Xusheng, peterz, mingo, tim.c.chen, vincent.guittot,
juri.lelli, dietmar.eggemann, rostedt, bsegall, mgorman,
vschneid, linux-kernel, zhanxusheng
Hi Qais,
On Tue, Nov 03, 2026 at 01:31:17AM +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?
>
> I see we have other magic like llc_aggr_tolerance and llc_imb_pct etc.
>
The original idea was to avoid threads overaggregating on their preferred LLC.
There are several metrics used to avoid this overaggregation: the first one is
utilization, the second one is the number of active threads, and the third one
is the LLC cache size.
1. llc_overaggr_pct is used for utilization. The default value is 50%, which means
that if the utilization of the preferred LLC exceeds 50%, we slow down the aggregation.
2. invalid_llc_nr() leverages fits_capacity() to skip any process that has too many
active threads, and we don't want it to become saturated because every CPU in this LLC
has one runnable thread. fits_capacity() was reused only because it happened to be a good
fit for this case, and we don't want introduce extra tuning knobs like llc_overaggr_pct,
so 80% was chosen simply because fits_capacity() uses that value.
2. exceed_llc_capacity() checks whether a task's memory footprint exceeds the LLC cache
size. This is where llc_aggr_tolerance is involved.
So for 1, 2, and 3, they address different aspects that prevent overaggregation. We can not
simply remove one of them and let the rest avoid overaggregation. For example, the process
might have very low utilization (frequently doing context switch) but have a large number of
active threads. In this case, we don't want to do aggregation for this process.
After the fine-grained control cache-aware scheduling is introduced, we allow the user tune
these parameters per group aggressively. But for the global settings, we don't want to bring
any performance regression, so these parameters are just safeguards for a global conservative
strategy.
For Xusheng's patch, it looks good to me at the moment.
thanks,
Chenyu
> >
> > 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts
2026-11-02 2:15 [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts Zhan Xusheng
2026-11-03 1:31 ` Qais Yousef
@ 2026-11-03 2:27 ` Chen Yu
1 sibling, 0 replies; 4+ messages in thread
From: Chen Yu @ 2026-11-03 2:27 UTC (permalink / raw)
To: Zhan Xusheng
Cc: peterz, mingo, qyousef, tim.c.chen, vincent.guittot, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid,
linux-kernel, zhanxusheng
Hi Xusheng,
On Mon, Nov 02, 2026 at 10:15:36AM +0800, 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>
> ---
> 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?
>
As replied to Qais in another thread, I'm not sure llc_aggr_tolerance
can cover the thread number case, as the former is only for LLC size
scaling,
for this patch,
Reviewed-by: Chen Yu <yu.c.chen@intel.com>
thanks,
Chenyu
> 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
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-03 2:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02 2:15 [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts Zhan Xusheng
2026-11-03 1:31 ` Qais Yousef
2026-11-03 2:24 ` Chen Yu
2026-11-03 2:27 ` Chen Yu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®