linux-kernel - Re: [PATCH V4 3/4] 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: <CAJhGHyCretunE7VaG5ZSBtkwmC56K2YMgRt_9NbwNpf2Yt_yHA@mail.gmail.com>
Date: Wed, 3 Dec 2025 11:22:32 +0800
From: Lai Jiangshan <jiangshanlai@...il.com>
To: Tejun Heo <tj@...nel.org>
Cc: linux-kernel@...r.kernel.org, ying chen <yc1082463@...il.com>, 
	Lai Jiangshan <jiangshan.ljs@...group.com>
Subject: Re: [PATCH V4 3/4] workqueue: Limit number of processed works in
 rescuer per turn

On Wed, Dec 3, 2025 at 2:16 AM Tejun Heo <tj@...nel.org> 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