linux-kernel - Re: [PATCH v2] sched/fair: do not scan twice in detach

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8ce3cd68-1758-4efd-80fd-f60fb734e66b@amperemail.onmicrosoft.com>
Date: Fri, 18 Jul 2025 14:31:45 +0800
From: Shijie Huang <shijie@...eremail.onmicrosoft.com>
To: Huang Shijie <shijie@...amperecomputing.com>, mingo@...hat.com,
 peterz@...radead.org, juri.lelli@...hat.com, vincent.guittot@...aro.org,
 vschneid@...hat.com
Cc: patches@...erecomputing.com, cl@...ux.com,
 Shubhang@...amperecomputing.com, dietmar.eggemann@....com,
 rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] sched/fair: do not scan twice in detach_tasks()


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@...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;
>   	}
>   
>   	/*