Re: [PATCH V4 3/4] workqueue: Limit number of processed works in rescuer per turn

[Lai Jiangshan]

On Wed, Dec 3, 2025 at 2:16 AM Tejun Heo <tj@xxxxxxxxxx> wrote:
>
> On Tue, Nov 25, 2025 at 02:36:16PM +0800, Lai Jiangshan wrote:
> ...
> > +             while (assign_rescuer_work(pwq, rescuer, ++count > RESCUER_BATCH))
> >                       process_scheduled_works(rescuer);
>
> Can we something like the following instead?
>
>         while (assign_rescuer_work(pwq, rescuer, &count))
>
> It just feels odd to for the caller to decide "you should stop" and then
> taking actions on the return value of the callee.

I think the nanming of "limited" or "throttled" gives the wrong
impression of "you should stop". How about renaming it again to
"rotate" or "prefer_rotate", meaning "prefer to rotate instead of
assigning work from this pwq"?

> Alternatively, just
> separate out the pwq rotation into a separate function, so that the caller
> can do
>
>         while (assign_rescuer_work(..)) {
>                 process_scheduled_works(rescuer);
>                 if (++count > RESCUER_BATCH) {
>                         rotate mayday list;

The current implementation of the cursor cannot be processed by workers as a
regular work item, which means it has no pwq reference and cannot be left
behind if the pwq is not put back on the mayday list. As a result, the code
here has to handle the cursor explicitly, which adds extra burden to
understanding the code.

I prefer using send_mayday() as in this patch (which is naturally available
in assign_rescuer_work() but requires extra code here) rather than the
rotate code used before this patchset.

In other words, I prefer the rotate logic - which decides whether to assign work
or not - to live in assign_rescuer_work(). I’m also okay with moving the
counting logic into assign_rescuer_work(),  as
assign_rescuer_work(pwq, rescuer, &count);

Thanks
Lai

>                         break;
>                 }
>         }
>
> Thanks.
>
> --
> tejun