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: <CAJhGHyDhQ4Ob7x5N6p26B9rDvLnifH5aawLgVneB1TZOAFKZNQ@mail.gmail.com>
Date: Sat, 22 Nov 2025 14:22:12 +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 V3 6/7] workqueue: Limit number of processed works in
 rescuer per turn

Hello

On Sat, Nov 22, 2025 at 3:28 AM Tejun Heo <tj@...nel.org> wrote:
>
> 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

I might misunderstand the setting.

>
> 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?
>

The cursor is per PWQ. When the rescuer come back to this pool next time,
it can only handle the PWQ belonging to its wq, which is A, and it will search
from A3 and process A3 instead of searching from B1 if the cursor is reset.

The rescuer never moves to B (I assume "A1 B1 A2 B2 A3 B3" is the worklist
of the pool, and shows B is a pwq of the pool), because B is not a PWQ
of the workqueue since there never be two PWQs of the same wq in the same
pool.

The rescuer will leave this pool and will move to the next pwq in
pwq->wq->maydays and, continuing to help its own wq, shifting from
pwq to pwq with a limited number of processed work for each pwq in turn.

Thanks,
Lai