[PATCH v2] workqueue: annotate racy p->wake_cpu accesses in kick_pool_pick()

[PATCH v2] workqueue: annotate racy p->wake_cpu accesses in kick_pool_pick()

[Breno Leitao]

kick_pool_pick() reads and writes p->wake_cpu while the scheduler can
update it concurrently. KCSAN reports:

  BUG: KCSAN: data-race in kick_pool_pick+0xf8/0x2d8

  race at unknown origin, with read to 0xffff000663229da4 of 4 bytes by
  task 1817002 on cpu 40:
   kick_pool_pick+0xf8/0x2d8
   process_scheduled_works+0x2bc/0x888
   worker_thread+0x394/0x548
   kthread+0x1b8/0x1f0
   ret_from_fork+0x10/0x20

  value changed: 0x0000002b -> 0x0000002f

The race is harmless, this patch only acknowledge that this is racy and
it is fine, silenting KCSAN.

Mark both accesses with READ_ONCE() and WRITE_ONCE() to document that
they are intentionally racy and to stop the compiler from reloading or
tearing them.

Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Bradley Morgan <include@grrlz.net>
---
Changes in v2:
- Mark the p->wake_cpu store with WRITE_ONCE() as well (Tejun)
- Say in the changelog that the race is harmless, and why
- Carried Bradley's Reviewed-by across the WRITE_ONCE() addition
- Link to v1: https://patch.msgid.link/20260805-wq_race_kick-v1-1-d55adc12416b@debian.org
---
 kernel/workqueue.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 8fd6af72ffd8d..503cab539ec80 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1308,14 +1308,16 @@ static bool kick_pool_pick(struct worker_pool *pool, struct task_struct **wakep)
 	 * If @pool has non-strict affinity, @worker might have ended up outside
 	 * its affinity scope. Repatriate.
 	 */
-	if (!pool->attrs->affn_strict &&
-	    !cpumask_test_cpu(p->wake_cpu, pool->attrs->__pod_cpumask)) {
+	bool wake_cpu_in_pod = cpumask_test_cpu(READ_ONCE(p->wake_cpu),
+						pool->attrs->__pod_cpumask);
+
+	if (!pool->attrs->affn_strict && !wake_cpu_in_pod) {
 		struct work_struct *work = list_first_entry(&pool->worklist,
 						struct work_struct, entry);
 		int wake_cpu = cpumask_any_and_distribute(pool->attrs->__pod_cpumask,
 							  cpu_online_mask);
 		if (wake_cpu < nr_cpu_ids) {
-			p->wake_cpu = wake_cpu;
+			WRITE_ONCE(p->wake_cpu, wake_cpu);
 			get_work_pwq(work)->stats[PWQ_STAT_REPATRIATED]++;
 		}
 	}

---
base-commit: a5bde5d8fde8a8cb28e59a672d5ddc5b9c1e7656
change-id: 20260805-wq_race_kick-d7ae5c14258d

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH v2] workqueue: annotate racy p->wake_cpu accesses in kick_pool_pick()

[Tejun Heo]

Hello, Breno.

On Tue, Aug 11, 2026 at 02:55:56AM -0700, Breno Leitao wrote:
> The race is harmless, this patch only acknowledge that this is racy and
> it is fine, silenting KCSAN.

Can you say why it's harmless? wake_cpu is a best-effort placement hint.
Every writer stores a valid CPU id and the wakeup path validates it
through select_task_rq(), so a racy value only affects where the worker
wakes up. Also, s/acknowledge/acknowledges/ and s/silenting/silencing/.

> -	if (!pool->attrs->affn_strict &&
> -	    !cpumask_test_cpu(p->wake_cpu, pool->attrs->__pod_cpumask)) {
> +	bool wake_cpu_in_pod = cpumask_test_cpu(READ_ONCE(p->wake_cpu),
> +						pool->attrs->__pod_cpumask);
> +
> +	if (!pool->attrs->affn_strict && !wake_cpu_in_pod) {

The hoist drops the !affn_strict short-circuit and adds a declaration
after statements. Can you keep the test inline in the condition?

	if (!pool->attrs->affn_strict &&
	    !cpumask_test_cpu(READ_ONCE(p->wake_cpu),
			      pool->attrs->__pod_cpumask)) {

Thanks.

-- 
tejun