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))
next prev 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®