mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Chen Yu <yu.c.chen@intel.com>
To: Zhan Xusheng <zhanxusheng1024@gmail.com>
Cc: <peterz@infradead.org>, <mingo@redhat.com>,
	<tim.c.chen@linux.intel.com>, <kayracizmeci@gmail.com>,
	<kprateek.nayak@amd.com>, <juri.lelli@redhat.com>,
	<vincent.guittot@linaro.org>, <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: Check reduced capacity before alb_break_llc()
Date: Thu, 29 Oct 2026 16:18:32 +0800	[thread overview]
Message-ID: <auMBWAGx04w5h80o@chenyu-dev> (raw)
In-Reply-To: <20261029023650.3404429-1-zhanxusheng1024@gmail.com>

Hi Xusheng,

On Thu, Oct 29, 2026 at 10:36:50AM +0800, Zhan Xusheng wrote:
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
> 
> need_active_balance() moves the only CFS task off a CPU whose capacity
> is reduced, for example by IRQ load, to an idle CPU with more.
> alb_break_llc() runs first and refuses to move a lone task off its
> preferred LLC, so if no other CPU in that LLC is idle the task stays on
> the reduced CPU.  Like asym packing, which was moved ahead of
> alb_break_llc() for the same reason, this is a performance case that
> cache locality should not override.
> 
> With two LLCs of two CPUs, the other CPU of the task's LLC busy and 60%
> of the task's CPU spent in a hardirq, a CPU-bound two-thread process
> stayed there for 20 seconds at 38% of its normal throughput.  With this
> change it moves to the idle LLC within a second, as it does with cache
> aware scheduling disabled.
> 
> Fixes: 714059f79ff0 ("sched/cache: Handle moving single tasks to/from their preferred LLC")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>  kernel/sched/fair.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index cfc982205b98..e313c6d212c6 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -13585,12 +13585,6 @@ static int need_active_balance(struct lb_env *env)
>  	if (asym_active_balance(env))
>  		return 1;
>  
> -	if (alb_break_llc(env))
> -		return 0;
> -
> -	if (imbalanced_active_balance(env))
> -		return 1;
> -
>  	/*
>  	 * The dst_cpu is idle and the src_cpu CPU has only 1 CFS task.
>  	 * It's worth migrating the task if the src_cpu's capacity is reduced
> @@ -13604,6 +13598,12 @@ static int need_active_balance(struct lb_env *env)
>  			return 1;
>  	}
>  
> +	if (alb_break_llc(env))
> +		return 0;
> +
> +	if (imbalanced_active_balance(env))
> +		return 1;
> +
>  	if (env->migration_type == migrate_misfit ||
>  	    env->migration_type == migrate_llc_task)
>  		return 1;
>

Moving the capacity check earlier seems to completely ignore the
task's LLC preference. It seems to be a trade-off between choosing
CPU capacity and cache re-population on a remote LLC, and cross-LLC
access latency due to cross-LLC migration. I don't know which one has
more impact, it looks to be workload-specific. May I know which
benchmark/platform you used to find this issue? I'd like to launch
hackbench/tbench/schbenh/netperf(or yours) to evaluate the impact.

May I know if the issue is because of the
if (env->src_rq->nr_running <= 1)
	return true
in alb_break_llc()?

If yes, to mitigate the possible impact, do you think tuning the
alb_break_llc() would be safer?

thanks,
Chenyu

patch1 introduces a helper to consider the capacity between src_cpu
and dst_cpu, no functional change:

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0f2364479f9d..35e6cc7a0fb4 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -13573,10 +13573,27 @@ imbalanced_active_balance(struct lb_env *env)
 	return 0;
 }
 
-static int need_active_balance(struct lb_env *env)
+static inline bool
+reduced_cap_alb(struct lb_env *env)
 {
 	struct sched_domain *sd = env->sd;
 
+	/*
+	 * The dst_cpu is idle and the src_cpu CPU has only 1 CFS task.
+	 * It's worth migrating the task if the src_cpu's capacity is reduced
+	 * because of other sched_class or IRQs if more capacity stays
+	 * available on dst_cpu.
+	 */
+	return env->idle &&
+	       (env->src_rq->cfs.h_nr_runnable == 1) &&
+	       check_cpu_capacity(env->src_rq, sd) &&
+	       (capacity_of(env->src_cpu)*sd->imbalance_pct < capacity_of(env->dst_cpu)*100);
+}
+
+static int need_active_balance(struct lb_env *env)
+{
+	bool enough_cap_src = !reduced_cap_alb(env);
+
 	if (alb_break_llc(env))
 		return 0;
 
@@ -13586,18 +13603,8 @@ static int need_active_balance(struct lb_env *env)
 	if (imbalanced_active_balance(env))
 		return 1;
 
-	/*
-	 * The dst_cpu is idle and the src_cpu CPU has only 1 CFS task.
-	 * It's worth migrating the task if the src_cpu's capacity is reduced
-	 * because of other sched_class or IRQs if more capacity stays
-	 * available on dst_cpu.
-	 */
-	if (env->idle &&
-	    (env->src_rq->cfs.h_nr_runnable == 1)) {
-		if ((check_cpu_capacity(env->src_rq, sd)) &&
-		    (capacity_of(env->src_cpu)*sd->imbalance_pct < capacity_of(env->dst_cpu)*100))
-			return 1;
-	}
+	if (!enough_cap_src)
+		return 1;
 
 	if (env->migration_type == migrate_misfit ||
 	    env->migration_type == migrate_llc_task)



patch2 tunes the abl_break_llc():

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 35e6cc7a0fb4..43ce929f8ad1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -10994,7 +10994,7 @@ static enum llc_mig can_migrate_llc_task(struct lb_env *env,
  * migrating the only runnable task away.
  */
 static inline bool
-alb_break_llc(struct lb_env *env)
+alb_break_llc(struct lb_env *env, bool enough_cap_src)
 {
 	if (!sched_cache_enabled())
 		return false;
@@ -11022,7 +11022,7 @@ alb_break_llc(struct lb_env *env)
 		if (env->migration_type == migrate_misfit)
 			return false;
 
-		if (env->src_rq->nr_running <= 1)
+		if (env->src_rq->nr_running <= 1 && enough_cap_src)
 			return true;
 
 		cur = rcu_dereference_all(env->src_rq->curr);
@@ -11098,7 +11098,7 @@ static inline bool get_llc_stats(int cpu, unsigned long *util,
 }
 
 static inline bool
-alb_break_llc(struct lb_env *env)
+alb_break_llc(struct lb_env *env, bool enough_cap_src)
 {
 	return false;
 }
@@ -13594,7 +13594,7 @@ static int need_active_balance(struct lb_env *env)
 {
 	bool enough_cap_src = !reduced_cap_alb(env);
 
-	if (alb_break_llc(env))
+	if (alb_break_llc(env, enough_cap_src))
 		return 0;
 
 	if (asym_active_balance(env))


  parent reply	other threads:[~2026-10-29  8:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29  2:36 Zhan Xusheng
2026-10-29  7:13 ` Kayra Cizmeci
2026-10-29  7:50   ` Zhan Xusheng
2026-10-29  8:13     ` Kayra Cizmeci
2026-10-29  8:18   ` Chen Yu
2026-10-29  8:18 ` Chen Yu [this message]
2026-10-29  8:55   ` Zhan Xusheng
2026-10-30 15:04     ` Chen Yu

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=auMBWAGx04w5h80o@chenyu-dev \
    --to=yu.c.chen@intel.com \
    --cc=bsegall@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=kayracizmeci@gmail.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tim.c.chen@linux.intel.com \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.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 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®