Re: [PATCH v2] sched/fair: do not scan twice in detach_tasks()

[Shijie Huang <shijie@amperemail.onmicrosoft.com>]

On 2025/7/18 13:47, Huang Shijie wrote:
> When detach_tasks() scans the src_cpu's task list, the task list
> may shrink during the scanning. For example, the task list
> may have four tasks at the beginning, it may becomes to two
> during the scanning in detach_tasks():
>      Task list at beginning : "ABCD"
>      Task list in scanning  : "CD"
>
>      (ABCD stands for differnt tasks.)
>
> In this scenario, the env->loop_max is still four, so
> detach_tasks() may scan twice for some tasks:
>      the scanning order maybe : "DCDC"
>
> In the Specjbb test, this issue can be catched many times.
> (Over 3,300,000 times in a 30-min Specjbb test)
sorry, I added an extra 0 here, it should be "330,000" times.
> The patch introduces "first_back" to record the first task which
> is put back to the task list. If we get a task which is equal to
> first_back, we break the loop, and avoid to scan twice for it.
>
> Signed-off-by: Huang Shijie <shijie@os.amperecomputing.com>
> ---
> v1 --> v2:
>      Add more comment from Valentin Schneider
>      v1: https://lore.kernel.org/all/20250707083636.38380-1-shijie@os.amperecomputing.com/
> ---
>   kernel/sched/fair.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 7e2963efe800..7cc9d50e3e11 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9443,6 +9443,7 @@ static int detach_tasks(struct lb_env *env)
>   {
>   	struct list_head *tasks = &env->src_rq->cfs_tasks;
>   	unsigned long util, load;
> +	struct task_struct *first_back = NULL;
>   	struct task_struct *p;
>   	int detached = 0;
>   
> @@ -9481,6 +9482,12 @@ static int detach_tasks(struct lb_env *env)
>   		}
>   
>   		p = list_last_entry(tasks, struct task_struct, se.group_node);
> +		/*
> +		 * We're back to an already visited task that couldn't be
> +		 * detached, we've seen all there is to see.
> +		 */
> +		if (p == first_back)
> +			break;
>   
>   		if (!can_migrate_task(p, env))
>   			goto next;
> @@ -9562,6 +9569,8 @@ static int detach_tasks(struct lb_env *env)
>   			schedstat_inc(p->stats.nr_failed_migrations_hot);
>   
>   		list_move(&p->se.group_node, tasks);
> +		if (!first_back)
> +			first_back = p;
>   	}
>   
>   	/*