All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sched/cache: Fix a thread aggregation conflict when there is one runnable task
@ 2026-07-27  1:29 Chen Yu
  2026-07-27  6:41 ` Zhan Xusheng
  0 siblings, 1 reply; 3+ messages in thread
From: Chen Yu @ 2026-07-27  1:29 UTC (permalink / raw)
  To: Peter Zijlstra, Vincent Guittot
  Cc: K Prateek Nayak, Ingo Molnar, Juri Lelli, Tim Chen,
	Valentin Schneider, Mel Gorman, Steven Rostedt, Dietmar Eggemann,
	Ben Segall, linux-kernel, Chen Yu, Yi Lai

On systems with SD_ASYM_PACKING set at the PKG domain level (e.g.,
with ITMT enabled), the sched_asym() check in sched_balance_find_src_rq()
prevents pulling a single task from a higher-priority CPU to a
lower-priority CPU. This blocks migrate_llc_task migrations where a task
wants to move to its preferred LLC for cache locality.

For example, CPU 0 (highest priority) has a task preferring LLC3,
but the load balancer on CPU 120 (LLC3) cannot pull it because:
sched_asym(PKG_sd, cpu=0, dst_cpu=120) and nr_running == 1
The task remains stuck in the wrong LLC indefinitely.

Fix this by exempting migrate_llc_task from the sched_asym filter.

Fixes: e4c9a4cb244a ("sched/cache: Add migrate_llc_task migration type for cache-aware balancing")
Reported-by: Yi Lai <yi1.lai@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 kernel/sched/fair.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index d78467ec6ee1..e6e6fa84c9e1 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -12986,8 +12986,12 @@ static struct rq *sched_balance_find_src_rq(struct lb_env *env,
 		 *
 		 * If balancing between cores, let lower priority CPUs help
 		 * SMT cores with more than one busy sibling.
+		 *
+		 * For migrate_llc_task, skip this check: cache locality
+		 * outweighs the asym priority.
 		 */
-		if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
+		if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1 &&
+		    env->migration_type != migrate_llc_task)
 			continue;
 
 		switch (env->migration_type) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] sched/cache: Fix a thread aggregation conflict when there is one runnable task
  2026-07-27  1:29 [PATCH] sched/cache: Fix a thread aggregation conflict when there is one runnable task Chen Yu
@ 2026-07-27  6:41 ` Zhan Xusheng
  2026-07-27  9:46   ` Chen, Yu C
  0 siblings, 1 reply; 3+ messages in thread
From: Zhan Xusheng @ 2026-07-27  6:41 UTC (permalink / raw)
  To: Chen Yu, Peter Zijlstra, Vincent Guittot
  Cc: K Prateek Nayak, Ingo Molnar, Juri Lelli, Tim Chen,
	Valentin Schneider, Mel Gorman, Steven Rostedt, Dietmar Eggemann,
	Ben Segall, Yi Lai, linux-kernel, Zhan Xusheng

On Mon, Jul 27, 2026 at 09:29:14AM +0800, Chen Yu wrote:
> -	if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
> +	if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1 &&
> +	    env->migration_type != migrate_llc_task)
>  		continue;

The fix looks correct for the ITMT case. I also convinced myself it does
not introduce a migrate_llc_task <-> asym-packing ping-pong: once the task
lands in its preferred LLC, a subsequent asym pull-back is a migrate_task,
which goes through can_migrate_task() -> migrate_degrades_llc() ->
can_migrate_llc_task() == mig_forbid (moving away from the preferred LLC),
so it returns 0 and the task is not pulled back (modulo the
nr_balance_failed >= cache_nice_tries + 1 escape). So it should not
oscillate. Nice.

One minor consistency question: the adjacent single-task filter just above,

	if (env->sd->flags & SD_ASYM_CPUCAPACITY &&
	    !capacity_greater(capacity_of(env->dst_cpu), capacity) &&
	    nr_running == 1)
		continue;

has the same nr_running == 1 shape and is not exempted for
migrate_llc_task. I don't think it is reachable on current hardware (it
would need both multiple LLCs, so migrate_llc_task can fire, and
asymmetric CPU capacity at that domain), so this is not a correctness
concern for the reported case. But since the two filters now treat
migrate_llc_task differently, was leaving the capacity one intentional
(capacity prioritized over cache locality on asym-capacity), or just out
of scope here? A word in the changelog would make the intent clear.

Thanks,
Xusheng

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] sched/cache: Fix a thread aggregation conflict when there is one runnable task
  2026-07-27  6:41 ` Zhan Xusheng
@ 2026-07-27  9:46   ` Chen, Yu C
  0 siblings, 0 replies; 3+ messages in thread
From: Chen, Yu C @ 2026-07-27  9:46 UTC (permalink / raw)
  To: Zhan Xusheng
  Cc: K Prateek Nayak, Ingo Molnar, Juri Lelli, Tim Chen,
	Valentin Schneider, Mel Gorman, Steven Rostedt, Dietmar Eggemann,
	Ben Segall, Yi Lai, linux-kernel, Zhan Xusheng, Peter Zijlstra,
	Vincent Guittot, chen.yu@linux.dev

Hi Xusheng,

On 7/27/2026 2:41 PM, Zhan Xusheng wrote:
> On Mon, Jul 27, 2026 at 09:29:14AM +0800, Chen Yu wrote:
>> -	if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1)
>> +	if (sched_asym(env->sd, i, env->dst_cpu) && nr_running == 1 &&
>> +	    env->migration_type != migrate_llc_task)
>>   		continue;
> 
> The fix looks correct for the ITMT case. I also convinced myself it does
> not introduce a migrate_llc_task <-> asym-packing ping-pong: once the task
> lands in its preferred LLC, a subsequent asym pull-back is a migrate_task,
> which goes through can_migrate_task() -> migrate_degrades_llc() ->
> can_migrate_llc_task() == mig_forbid (moving away from the preferred LLC),
> so it returns 0 and the task is not pulled back (modulo the
> nr_balance_failed >= cache_nice_tries + 1 escape). So it should not
> oscillate. Nice.
> 

Thanks for checking the ping-pong angle, and I agree it does not oscillate.
I did a further investigation, and another reason it holds: with only one
running task, migrate_degrades_llc() is not what keeps the task in its
preferred LLC. In the regular load balance path can_migrate_task() already
bails at task_on_cpu() - since that task is curr. And in the active balance
path alb_break_llc() blocks it first:

if (env->src_rq->nr_running <= 1)
     return true;

so the task is not pulled back either way.

> One minor consistency question: the adjacent single-task filter just above,
> 
> 	if (env->sd->flags & SD_ASYM_CPUCAPACITY &&
> 	    !capacity_greater(capacity_of(env->dst_cpu), capacity) &&
> 	    nr_running == 1)
> 		continue;
> 
> has the same nr_running == 1 shape and is not exempted for
> migrate_llc_task. I don't think it is reachable on current hardware (it
> would need both multiple LLCs, so migrate_llc_task can fire, and
> asymmetric CPU capacity at that domain), so this is not a correctness
> concern for the reported case. But since the two filters now treat
> migrate_llc_task differently, was leaving the capacity one intentional
> (capacity prioritized over cache locality on asym-capacity), or just out
> of scope here? A word in the changelog would make the intent clear.
> 

Good point. OK, let me add description in changelog:
The adjacent SD_ASYM_CPUCAPACITY filter was left unchanged. In theory
it should have been handled in the same way, but it is not clear yet
whether any platform has both multiple LLCs and asymmetric CPU capacity,
so the asymmetric CPU capacity logic was not fine-tuned here and can be
revisited once such a platform shows up. In theory the priority follows the
order of enum group_type: the priority of group_llc_balance is lower than
that of group_overloaded and higher than that of all other group types.

thanks,
Chenyu

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-27  9:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-27  1:29 [PATCH] sched/cache: Fix a thread aggregation conflict when there is one runnable task Chen Yu
2026-07-27  6:41 ` Zhan Xusheng
2026-07-27  9:46   ` Chen, Yu C

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.