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

Message-ID: <aRcvMPAkGKqTaWJn@slm.duckdns.org>
Date: Fri, 14 Nov 2025 03:31:28 -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,

On Fri, Nov 14, 2025 at 10:41:04AM +0800, Lai Jiangshan wrote:
> > 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.
> 
> I think it should be processable by the normal workers, so that we don't
> have to add special code in the fast path in worker_thread(), and it should work
> like a normal work item or the wq_barrier.

I don't know. I don't think a single unlikely() test is going to be
noticeable.

> The ability of processable by the normal workers has an additional benefit:
> if it is processed by a normal worker rather than the rescuer, this
> indicates that the pool has made forward progress by normal workers
> and  the pwq does not need to be rescued until the next mayday event.

Is that correct tho? A worker thread executes any work item on the list and
the pool may be shared by any number of workqueues, so the cursor can be
cleared without any of the work items that belong to the rescuer's workqueue
being executed, no?

> It intended to update the positional work color to avoid back-to-back
> work items to
> stall the flush_workqueue().
> 
> But it did it wrong. Maybe this is better:
> 
> if (get_work_color(*work_data_bits(&pwq->mayday_pos_work)) != pwq->work_color) {
> {
>   remove_mayday_pos(pwq);
>   insert_mayday_pos(pwq, n);
> } else {
>   list_move_tail(&pwq->mayday_pos_work.entry, &n->entry);
> }

The colored flush mechanism is already complex enough and difficult wrap
one's head around. I don't really like adding more complications there.

Thanks.

-- 
tejun