linux-kernel - Re: [PATCH] workqueue: Process rescuer work items one-by-one using a positional marker

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aRYSPyIp2LYzUkf-@slm.duckdns.org>
Date: Thu, 13 Nov 2025 07:15:43 -1000
From: Tejun Heo <tj@...nel.org>
To: Lai Jiangshan <jiangshanlai@...il.com>
Cc: linux-kernel@...r.kernel.org,
	Lai Jiangshan <jiangshan.ljs@...group.com>,
	ying chen <yc1082463@...il.com>
Subject: Re: [PATCH] workqueue: Process rescuer work items one-by-one using a
 positional marker

Hello, Lai.

On Fri, Nov 14, 2025 at 12:34:26AM +0800, Lai Jiangshan wrote:
> +static void insert_mayday_pos(struct pool_workqueue *pwq, struct work_struct *next)
> +{
> +	unsigned int work_flags;
> +	unsigned int work_color;
> +
> +	__set_bit(WORK_STRUCT_PENDING_BIT, work_data_bits(&pwq->mayday_pos_work));

Maybe use test_and_set_bit() here like normal work item?

> +static void remove_mayday_pos(struct pool_workqueue *pwq)
> +{
> +	list_del_init(&pwq->mayday_pos_work.entry);
> +	pwq_dec_nr_in_flight(pwq, *work_data_bits(&pwq->mayday_pos_work));
> +	INIT_WORK(&pwq->mayday_pos_work, mayday_pos_func);

and maybe we can init the work item once and

>  static void send_mayday(struct work_struct *work)
>  {
>  	struct pool_workqueue *pwq = get_work_pwq(work);
> @@ -2992,6 +3037,9 @@ static void send_mayday(struct work_struct *work)
>  	if (!wq->rescuer)
>  		return;
>  
> +	if (!work_pending(&pwq->mayday_pos_work))

drop this conditional too?

Also, I wonder whether it'd be simpler to think about if we just exclude the
work item from flush color management. e.g. we can just flag the work item
and then make work scanning skip them, so that they really are just markers;
then, we don't have to worry about colors or othre states at all. We just
insert the marker and use it purely as iteration marker.

> +				/* reset the position and handle the assigned work */
> +				if (list_next_entry(&pwq->mayday_pos_work, entry) != n) {
> +					remove_mayday_pos(pwq);
> +					insert_mayday_pos(pwq, n);

and this would become simple list_move_tail().

Thanks.

-- 
tejun