Re: [RFC PATCH v3 16/17] sched/cache: Drop stale preferred LLC checks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] sched/fair: migrate_degrades_llc() allows unrelated tasks in migrate_llc_task
--

commit a6df5e882c3ba89fef18f6405775608279e27938
Author: Jianyong Wu <wujianyong@hygon.cn>

sched/cache: Drop stale preferred LLC checks

This commit removes the NUMA preference check in get_pref_llc() and drops
the exact preferred_llc check in migrate_degrades_llc() so that
can_migrate_llc_task() determines whether an LLC migration is allowed.

> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index b93b6e6c9deae..5d11f9b7f685f 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
[ ... ]
> @@ -10898,15 +10880,6 @@ static bool migrate_degrades_llc(struct task_struct *p, struct lb_env *env)
>  	if (env->sd->nr_balance_failed >= env->sd->cache_nice_tries + 1)
>  		return false;
>  
> -	/*
> -	 * We know the env->src_cpu has some tasks prefer to
> -	 * run on env->dst_cpu, skip the tasks do not prefer
> -	 * env->dst_cpu, and find the one that prefers.
> -	 */
> -	if (env->migration_type == migrate_llc_task &&
> -	    READ_ONCE(p->preferred_llc) != llc_id(env->dst_cpu))
> -		return true;
> -
>  	if (can_migrate_llc_task(env->src_cpu,
>  				 env->dst_cpu, p) != mig_forbid)
>  		return false;

[Severity: Medium]
Does removing this check in migrate_degrades_llc() allow detach_tasks() to
pull unrelated tasks during a migrate_llc_task balance pass instead of the
task that actually prefers moving toward env->dst_cpu?

When a source group is classified as group_llc_balance, calculate_imbalance()
sets up a single-task migration pass:

kernel/sched/fair.c:calculate_imbalance() {
	if (busiest->group_type == group_llc_balance) {
		/* Move a task that prefer local LLC */
		env->migration_type = migrate_llc_task;
		env->imbalance = 1;
		return;
	}
}

And detach_tasks() stops as soon as a single task passes can_migrate_task():

kernel/sched/fair.c:detach_tasks() {
		case migrate_llc_task:
			env->imbalance--;
			break;
}

Without a check restricting migrate_llc_task to tasks that gain LLC affinity
by moving to env->dst_cpu, migrate_degrades_llc() relies solely on
can_migrate_llc_task(env->src_cpu, env->dst_cpu, p) != mig_forbid.

However, can_migrate_llc_task() returns mig_unrestricted for kernel threads
(!p->mm), tasks with mm->sc_stat.cpu < 0, single-threaded tasks matching
invalid_llc_nr(), and tasks matching exceed_llc_capacity():

kernel/sched/fair.c:can_migrate_llc_task() {
	mm = p->mm;
	if (!mm)
		return mig_unrestricted;

	cpu = READ_ONCE(mm->sc_stat.cpu);
	if (cpu < 0 || cpus_share_cache(src_cpu, dst_cpu))
		return mig_unrestricted;

	/* skip cache aware load balance for too many threads */
	if (invalid_llc_nr(mm, p, dst_cpu) ||
	    exceed_llc_capacity(mm, dst_cpu)) {
		if (READ_ONCE(mm->sc_stat.cpu) != -1)
			WRITE_ONCE(mm->sc_stat.cpu, -1);
		return mig_unrestricted;
	}

	to_pref = if_to_prefer(src_cpu, dst_cpu, llc_id(cpu));

	return can_migrate_llcs(src_cpu, dst_cpu,
			       p, to_pref);
}

In addition, when to_pref is false, can_migrate_llcs() can still return
mig_unrestricted or mig_llc. And when env->tol <= 1, update_sg_lb_stats()
and sched_balance_find_src_rq() only count affinity_score for tasks with
p->preferred_llc == llc_id(env->dst_cpu), whereas can_migrate_llc_task()
calls if_to_prefer() unconditionally.

If any such unrelated task precedes the target task on env->src_rq->cfs_tasks
(or if the target task is currently running on env->src_rq), won't
detach_tasks() detach the wrong task, decrement env->imbalance to 0, exit the
loop early, and also prevent need_active_balance() from migrating the running
LLC-preferring task?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029022036.4002331-1-wujianyong@hygon.cn?part=16