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

[Tejun Heo <tj@kernel.org>]

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