Re: [PATCH] workqueue: Add pool_workqueue to pending_pwqs list when unplugging multiple inactive works

Matthew Brost
2026-04-01T00:48:29Z

On Tue, Mar 31, 2026 at 05:22:08PM -0700, Matthew Brost wrote:
> On Tue, Mar 31, 2026 at 02:05:41PM -1000, Tejun Heo wrote:
> > Hello,
> >
> > On Tue, Mar 31, 2026 at 03:18:39PM -0700, Matthew Brost wrote:
> > > @@ -1849,8 +1849,20 @@ static void unplug_oldest_pwq(struct workqueue_struct *wq)
> > > raw_spin_lock_irq(&pwq->pool->lock);
> > > if (pwq->plugged) {
> > > pwq->plugged = false;
> > > - if (pwq_activate_first_inactive(pwq, true))
> > > + if (pwq_activate_first_inactive(pwq, true)) {
> > > + if (!list_empty(&pwq->inactive_works)) {
> > > + struct worker_pool *pool = pwq->pool;
> > > + struct wq_node_nr_active *nna =
> > > + wq_node_nr_active(wq, pool->node);
> > > +
> > > + raw_spin_lock(&nna->lock);
> > > + if (list_empty(&pwq->pending_node))
> > > + list_add_tail(&pwq->pending_node,
> > > + &nna->pending_pwqs);
> > > + raw_spin_unlock(&nna->lock);
> > > + }
> >
> > It's a bit gnarly to open code locking and list operation. Would just
> > calling pwq_activate_first_inactive(pwq, false) one more time work here?
> > That'd trigger tryinc_node_nr_active() failure in pwq_tryinc_nr_active() and
> > the addition to the pending list. As this is quite subtle, it'd be nice to
> > have some comment - it's compensating for the missed pwq_tryinc_nr_active()
> > call due to plugging, right?

Sorry - missed a question.

It is compensating for early bail in pwq_tryinc_nr_active() due to plugging here:

1726 static bool pwq_tryinc_nr_active(struct pool_workqueue *pwq, bool fill)
...
1741 if (unlikely(pwq->plugged))
1742 return false;
...
/* Add to &nna->pending_pwqs */

This far from my domain, so if there is different idea to fix this let
me know - takes 5-10 minutes on my end to test out.

The pwq_activate_first_inactive(pwq, false) suggestion works, so will
post that shortly unless you have another idea.

Matt

>
> Yeah, I think that will work. Let me verify with my reproducer and
> adjust the patch accordingly.
>
> +1 on the comment as well—very subtle. Took a few days of
> reverse-engineering work queues to track down.
>
> Matt
>
> >
> > Thanks.
> >
> > --
> > tejun