Re: [PATCH] workqueue: Fix false positive stall reports
From: Tejun Heo
Date: Sat Mar 21 2026 - 14:29:28 EST
Hello,
On Fri, Mar 20, 2026 at 12:23:32PM -0700, Song Liu wrote:
> index b77119d71641..5b501ff1223a 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -7701,6 +7701,23 @@ static void wq_watchdog_timer_fn(struct timer_list *unused)
>
> /* did we stall? */
> if (time_after(now, ts + thresh)) {
> + unsigned long irq_flags;
> +
> + raw_spin_lock_irqsave(&pool->lock, irq_flags);
Can you use guard() instead? I don't think we can just keep the lock in the
block.
> + /*
> + * Recheck last_progress_ts with pool->lock, this
> + * eliminates false positive where we report wq
> + * stall for newly queued work.
> + */
And I think this deserves a bit more explanation. Can you move this comment
outside the block where /* did we stall? */ is and give the scenario where
it can go wrong w/o the locked check?
> + pool_ts = READ_ONCE(pool->last_progress_ts);
pool->last_progress_ts is write protected by pool lock. No need for
READ_ONCE().
> + if (time_after(pool_ts, touched))
> + ts = pool_ts;
> + else
> + ts = touched;
> + raw_spin_unlock_irqrestore(&pool->lock, irq_flags);
> + if (!time_after(now, ts + thresh))
> + continue;
Thanks.
--
tejun