The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick()
@ 2026-08-05 11:17 Breno Leitao
  2026-08-05 18:42 ` Bradley Morgan
  2026-08-10 19:50 ` Tejun Heo
  0 siblings, 2 replies; 4+ messages in thread
From: Breno Leitao @ 2026-08-05 11:17 UTC (permalink / raw)
  To: Tejun Heo, Lai Jiangshan; +Cc: linux-kernel, kernel-team, Breno Leitao

kick_pool_pick() reads p->wake_cpu in a racy way with scheduler. This
gets the following message in KCSAN

  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

Mark p->wake_cpu's read as READ_ONCE(p->wake_cpu), in order to a) avoid
torn down reads, b) acknowledge this racy read, and c) silent KCSAN.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 kernel/workqueue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 26d5680c751c6..333752ac38298 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1308,8 +1308,10 @@ 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,

---
base-commit: 0f6da28aab51b16762ed82e8fdeaa5042da45b08
change-id: 20260805-wq_race_kick-d7ae5c14258d

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


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

* Re: [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick()
  2026-08-05 11:17 [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick() Breno Leitao
@ 2026-08-05 18:42 ` Bradley Morgan
  2026-08-10 19:50 ` Tejun Heo
  1 sibling, 0 replies; 4+ messages in thread
From: Bradley Morgan @ 2026-08-05 18:42 UTC (permalink / raw)
  To: leitao; +Cc: jiangshanlai, kernel-team, linux-kernel, tj

Hi Bruno,

Could you

- Add why this is bad
- and Add maybe stable? 

In the commit description 


:)

Other than that, tbh this patch is good, please add

Reviewed-by: Bradley Morgan <include@grrlz.net>

Thanks!

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

* Re: [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick()
  2026-08-05 11:17 [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick() Breno Leitao
  2026-08-05 18:42 ` Bradley Morgan
@ 2026-08-10 19:50 ` Tejun Heo
  2026-08-11  9:28   ` Breno Leitao
  1 sibling, 1 reply; 4+ messages in thread
From: Tejun Heo @ 2026-08-10 19:50 UTC (permalink / raw)
  To: Breno Leitao; +Cc: Lai Jiangshan, linux-kernel, kernel-team

On Wed, Aug 05, 2026 at 04:17:48AM -0700, Breno Leitao wrote:
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 26d5680c751c6..333752ac38298 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -1308,8 +1308,10 @@ 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) {

Can you update the p->wake_cpu assignement several lines below to
WRITE_ONCE() too?

Thanks.

-- 
tejun

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

* Re: [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick()
  2026-08-10 19:50 ` Tejun Heo
@ 2026-08-11  9:28   ` Breno Leitao
  0 siblings, 0 replies; 4+ messages in thread
From: Breno Leitao @ 2026-08-11  9:28 UTC (permalink / raw)
  To: Tejun Heo; +Cc: Lai Jiangshan, linux-kernel, kernel-team

On Mon, Aug 10, 2026 at 09:50:57AM -1000, Tejun Heo wrote:
> On Wed, Aug 05, 2026 at 04:17:48AM -0700, Breno Leitao wrote:
> > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > index 26d5680c751c6..333752ac38298 100644
> > --- a/kernel/workqueue.c
> > +++ b/kernel/workqueue.c
> > @@ -1308,8 +1308,10 @@ 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) {
> 
> Can you update the p->wake_cpu assignement several lines below to
> WRITE_ONCE() too?

Ack, let me update and resend.

Thanks,
--breno

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

end of thread, other threads:[~2026-08-11  9:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 11:17 [PATCH] workqueue: read p->wake_cpu once in kick_pool_pick() Breno Leitao
2026-08-05 18:42 ` Bradley Morgan
2026-08-10 19:50 ` Tejun Heo
2026-08-11  9:28   ` Breno Leitao

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