linux-kernel - Re: [PATCH V3 6/7] workqueue: Limit number of processed works in rescuer per turn

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: <aSC9eGqstqHt9XSK@slm.duckdns.org>
Date: Fri, 21 Nov 2025 09:28:56 -1000
From: Tejun Heo <tj@...nel.org>
To: Lai Jiangshan <jiangshanlai@...il.com>
Cc: linux-kernel@...r.kernel.org, ying chen <yc1082463@...il.com>,
	Lai Jiangshan <jiangshan.ljs@...group.com>
Subject: Re: [PATCH V3 6/7] workqueue: Limit number of processed works in
 rescuer per turn

Hello,

On Fri, Nov 21, 2025 at 10:57:19PM +0800, Lai Jiangshan wrote:
> +static bool assign_rescuer_work(struct pool_workqueue *pwq, struct worker *rescuer, bool limited)

I find the organization a bit odd with the expiration detection in the
caller and the implmentation of it piped into this function. Please see
below.

>  	list_for_each_entry_safe_from(work, n, &pool->worklist, entry) {
> -		if (get_work_pwq(work) == pwq && assign_work(work, rescuer, &n)) {
> +		if (get_work_pwq(work) != pwq)
> +		       continue;
> +		/*
> +		 * put the cursor, resend mayday for itself and move on to other
> +		 * PWQs when the limit is reached.
> +		 */
> +		if (limited && !list_empty(&pwq->wq->maydays)) {
> +			list_add_tail(&cursor->entry, &work->entry);
> +			raw_spin_lock(&wq_mayday_lock);		/* for wq->maydays */
> +			send_mayday(work);
> +			raw_spin_unlock(&wq_mayday_lock);
> +			return false;

Does it make sense to maintain cursor position across pwqs? Shouldn't it be
reset? Imagine two pwqs' (A, B) work items interleaved:

        A1 B1 A2 B2 A3 B3

1. Two of A's work items are rescued and cursor is inserted before the next
   eligible one:

        B1 B2 A3 B3
              ^

2. Let's say limit is reached and we're moving on to B. Then, the rescuer
   would first run B3. Wouldn't it make more sense to go back to the head of
   the queue and start over so that it can pick up B1 first?

Thanks.

-- 
tejun