All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tim Chen <tim.c.chen@linux.intel.com>
To: Zhan Xusheng <zhanxusheng1024@gmail.com>,
	peterz@infradead.org,  mingo@kernel.org
Cc: qyousef@layalina.io, yu.c.chen@intel.com,
	vincent.guittot@linaro.org,  juri.lelli@redhat.com,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	 bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
	 linux-kernel@vger.kernel.org, zhanxusheng@xiaomi.com
Subject: Re: [PATCH] sched/cache: Don't use fits_capacity() to compare thread and CPU counts
Date: Tue, 03 Nov 2026 10:30:59 -0800	[thread overview]
Message-ID: <fb4a9d54488c92206c0f983f7cc230d71db3b192.camel@linux.intel.com> (raw)
In-Reply-To: <20261102021536.1026143-1-zhanxusheng1024@gmail.com>

On Mon, 2026-11-02 at 10:15 +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?


Xusheng,

We tune the system based on a default llc_aggr_tolerance of 1,
where the number of running threads is 80% of the number of cores,
to prevent over-subscription which could lead to regressions.

This 80% factor is leaning on the conservative side to prevent
regressions for workloads where cache aware scheduling may
not help on multi-threaded process with little shared data.

We set the llc_aggr_tolerance as a default of 1 for more intuitive
tuning.  It varies from 

0 (no aggregation), 
1 (default with conservative aggregation)
100 (always aggregate)

Hope that makes sense.

Otherwise, the patch looks good to me.

Reviewed-by: Tim Chen <tim.c.chen@linux.intel.com>

Tim

> 
>  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

  parent reply	other threads:[~2026-11-03 18:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 21:07   ` Tim Chen
2026-11-03  2:27 ` Chen Yu
2026-11-03 18:30 ` Tim Chen [this message]
2026-11-03 18:34 ` Tim Chen
2026-11-09  8:38 ` [tip: sched/urgent] " tip-bot2 for Zhan Xusheng
2026-11-09  9:28 ` [tip: sched/core] " tip-bot2 for Zhan Xusheng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fb4a9d54488c92206c0f983f7cc230d71db3b192.camel@linux.intel.com \
    --to=tim.c.chen@linux.intel.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=qyousef@layalina.io \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=yu.c.chen@intel.com \
    --cc=zhanxusheng1024@gmail.com \
    --cc=zhanxusheng@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.