[GIT PULL] workqueue: Fixes for v7.4-rc2

[GIT PULL] workqueue: Fixes for v7.4-rc2

[Tejun Heo]

Hello,

The following changes since commit d89e1d186ec9271bf78e62771779e9fc03bf0fc3:

  Merge tag 'fscrypt-for-linus' of git://git.kernel.org/pub/scm/fs/fscrypt/linux (2026-10-29 14:00:23 -1000)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/tj/wq.git tags/wq-for-7.4-rc2-fixes

for you to fetch changes up to c7d64f52bffaca093acfb34277877138f6ec243a:

  workqueue: Fix drain_workqueue() missing work items chained across pwqs (2026-11-02 08:06:21 -1000)

----------------------------------------------------------------
workqueue: Fixes for v7.4-rc2

- Draining a workqueue could return while a work item queued by another work
  item was still pending, making workqueue destruction warn and leak the
  workqueue.

----------------------------------------------------------------
Tejun Heo (1):
      workqueue: Fix drain_workqueue() missing work items chained across pwqs

 kernel/workqueue.c | 68 ++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 51 insertions(+), 17 deletions(-)

-- 
tejun

Re: [GIT PULL] workqueue: Fixes for v7.4-rc2

[Linus Torvalds]

On Sun, 15 Nov 2026 at 12:45, Tejun Heo <tj@kernel.org> wrote:
>
> - Draining a workqueue could return while a work item queued by another work
>   item was still pending, making workqueue destruction warn and leak the
>   workqueue.

Why is this re-creating - badly - the existing sequence counter code?

Not only does it do the memory barriers by hand - possibly getting
them wrong, but it seems actively broken.

There's a reason proper sequence counters have that "odd/even" rule,
and a writer increments the sequence counter *twice* - once when it
starts the modification and once when it is done.

As far as I can tell, your "fix" is still broken:

  drain_workqueue:

        mutex_lock(&wq->mutex);
        if (!wq->nr_drainers++)
                wq->flags |= __WQ_DRAINING;
        mutex_unlock(&wq->mutex);

        seq = atomic_read(&wq->drain_seq);


__queue_work:

        if (unlikely(wq->flags & (__WQ_DESTROYING | __WQ_DRAINING))) {
   ...
                smp_mb__before_atomic();
                atomic_inc(&wq->drain_seq);
                smp_mb__after_atomic();

but what if __queue_work() was called before __WQ_DRAINING was set and
is running concurrently and never incremented the sequence number?

Of even if it *did* increment the sequence number, what keeps it from
doing that *before* the atomic_read() in drain_workqueue() and then

Either way drain_workqueue() never sees that drain_seq change, and
thinks everythings is fine, even though __queue_work added an entry?

Is there some other serialization that I'm missing?

Anyway, sequence numbers almost never work with a single write,
exactly beause the single write is itself then not ordered with the
changes that the writer does (ie the sequence point update happens
before them or after them, but either way the reader might miss the
sequence number update, becuase it's not atomic wrt the actual change
it is supposed to be about.

The actual 8real* sequence numbers we have do that two-step change
exactly because of these issues. The writer increments once before it
starts writing, and increments it again after it is done. That way the
reader actually can check whether something changed (an odd value
means that change is happening and needs retrying regardless - which
is why read_seqcount_begin(() will loop on odd numbers)

It is possible that I'm missing something that makes your ad-hoc
version ok, but this really does look just plain broken.

I pulled it and then unpulled it, waiting for explanations.

             Linus