[PATCH v2] workqueue: annotate the lockless nr_running increment racing with need_more_worker()

[PATCH v2] workqueue: annotate the lockless nr_running increment racing with need_more_worker()

[Nguyen Ngoc Thang]

wq_worker_running() increments pool->nr_running with only preemption
disabled, while kick_pool_pick() reads it via need_more_worker() under
pool->lock, so the two can run concurrently on different CPUs. KCSAN
reports:

  BUG: KCSAN: data-race in kick_pool_pick / wq_worker_running

  write to 0xffff888059c2af24 of 4 bytes by task 3007 on cpu 0:
   wq_worker_running+0x114/0x130
   schedule_timeout+0x16e/0x180
   io_schedule_timeout+0x4c/0x70
   submit_bio_wait+0xd7/0x110
   blkdev_issue_flush+0x4c/0x70
   ext4_sync_file+0x60d/0x6c0
   aio_fsync_work+0x7f/0x270
   process_one_work+0x4ae/0xa00

  read to 0xffff888059c2af24 of 4 bytes by task 3001 on cpu 1:
   kick_pool_pick+0x7a/0x3a0
   __queue_work+0x72d/0xcf0
   queue_work_on+0x9d/0xb0
   schedule_on_each_cpu+0x11a/0x2d0
   vmstat_refresh+0x23/0x140
   proc_sys_call_handler+0x2b9/0x4f0

The race is benign. The increment in wq_worker_running() is the only
lockless write to nr_running; decrements and the reset are done under
pool->lock, so a reader holding the lock sees either the old or the new
count. A stale 0 costs at most a spurious wakeup of an idle worker. A
nonzero count is correct because the incrementing worker is running and
re-checks the worklist under pool->lock before it stops counting. The
increment can also go 1 -> 2, e.g. when a worker sleeping in a work item
wakes up after kick_pool() woke another worker.

Annotate the read in need_more_worker() with data_race().

Reported-by: syzbot+8922e6b8343f70be92e8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8922e6b8343f70be92e8
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
v2:
 - Fix subject: the read is under pool->lock, the lockless side is the
   increment (Tejun).
 - Rewrite the "why benign" paragraph accordingly (Tejun).
 - Replace the duplicated comment with a pointer to @nr_running (Tejun).
 - Code unchanged.
v1: https://lore.kernel.org/all/20261012143605.281358-1-ngocthang2710.1999@gmail.com/

 kernel/workqueue.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1ae3732a2c51..102e1ab2bb7b 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -951,7 +951,8 @@ static unsigned long work_offqd_pack_flags(struct work_offq_data *offqd)
  */
 static bool need_more_worker(struct worker_pool *pool)
 {
-	return !list_empty(&pool->worklist) && !pool->nr_running;
+	/* Racy against the lockless increment, see @nr_running. */
+	return !list_empty(&pool->worklist) && !data_race(pool->nr_running);
 }
 
 /* Can I start working?  Called from busy but !running workers. */
-- 
2.43.0

Re: [PATCH v2] workqueue: annotate the lockless nr_running increment racing with need_more_worker()

[Breno Leitao]

On Tue, Oct 13, 2026 at 06:29:05PM +0700, Nguyen Ngoc Thang wrote:
>  static bool need_more_worker(struct worker_pool *pool)
>  {
> -	return !list_empty(&pool->worklist) && !pool->nr_running;
> +	/* Racy against the lockless increment, see @nr_running. */
> +	return !list_empty(&pool->worklist) && !data_race(pool->nr_running);

Isn't it better to annotate the nr_running field in the structure, rather than
at reading time?

Re: [PATCH v2] workqueue: annotate the lockless nr_running increment racing with need_more_worker()

[Nguyen Ngoc Thang]

On Wed, Oct 14, 2026, Breno Leitao wrote:
> Isn't it better to annotate the nr_running field in the structure, rather than
> at reading time?

Agreed. The lockless increment is how nr_running is meant to work, so
the field is the right place for the annotation. v3 marks it
__data_racy and drops the data_race() in need_more_worker():

  https://lore.kernel.org/all/20261014154342.20661-1-ngocthang2710.1999@gmail.com/

The trade-off is that KCSAN now ignores every access to nr_running,
including the locked ones. If someone later adds a decrement without
pool->lock, KCSAN won't catch it. I think that's acceptable, because
the rules are documented right above the field.

Thanks,
Nguyen Ngoc Thang