mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] workqueue: annotate the lockless nr_running read in need_more_worker()
@ 2026-10-12 14:36 Nguyen Ngoc Thang
  2026-10-12 18:25 ` Tejun Heo
  0 siblings, 1 reply; 2+ messages in thread
From: Nguyen Ngoc Thang @ 2026-10-12 14:36 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Lai Jiangshan, linux-kernel, Nguyen Ngoc Thang,
	syzbot+8922e6b8343f70be92e8

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: nr_running only goes 0 -> 1 here, and a stale 0
merely causes a spurious wakeup of an idle worker. Annotate the read
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>
---
 kernel/workqueue.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 1ae3732a2c51..18e74d9a0696 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -951,7 +951,11 @@ 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;
+	/*
+	 * wq_worker_running() bumps nr_running without pool->lock. A stale
+	 * read at worst causes a spurious wakeup, so the race is benign.
+	 */
+	return !list_empty(&pool->worklist) && !data_race(pool->nr_running);
 }
 
 /* Can I start working?  Called from busy but !running workers. */
-- 
2.43.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] workqueue: annotate the lockless nr_running read in need_more_worker()
  2026-10-12 14:36 [PATCH] workqueue: annotate the lockless nr_running read in need_more_worker() Nguyen Ngoc Thang
@ 2026-10-12 18:25 ` Tejun Heo
  0 siblings, 0 replies; 2+ messages in thread
From: Tejun Heo @ 2026-10-12 18:25 UTC (permalink / raw)
  To: Nguyen Ngoc Thang
  Cc: Lai Jiangshan, linux-kernel, syzbot+8922e6b8343f70be92e8, Breno Leitao

Hello,

On Mon, Oct 12, 2026 at 09:36:05PM +0700, Nguyen Ngoc Thang wrote:
> Subject: [PATCH] workqueue: annotate the lockless nr_running read in need_more_worker()

The read is under pool->lock. The lockless side is the increment in
wq_worker_running(). Can you fix the subject?

> The race is benign: nr_running only goes 0 -> 1 here, and a stale 0
> merely causes a spurious wakeup of an idle worker. Annotate the read
> with data_race().

The increment can also go 1 -> 2: a worker sleeping in a work item wakes
up after kick_pool() woke another worker which is now running. The reason
it's benign is that the only lockless write is an increment. Decrements
and the reset are under pool->lock, so a reader holding the lock sees
either the old or the new count. 0 costs at most a spurious wakeup.
Nonzero is correct because the incrementing worker is running and
re-checks the worklist under pool->lock before it stops counting. Can you
update the description accordingly?

> +	/*
> +	 * wq_worker_running() bumps nr_running without pool->lock. A stale
> +	 * read at worst causes a spurious wakeup, so the race is benign.
> +	 */

This duplicates the comment on nr_running in struct worker_pool. A pointer
to that would do.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-12 18:25 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12 14:36 [PATCH] workqueue: annotate the lockless nr_running read in need_more_worker() Nguyen Ngoc Thang
2026-10-12 18:25 ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®