All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] locking/percpu-rwsem: Annotate intentional data race in readers_active_check()
@ 2026-06-23  9:34 Sun Shaojie
  2026-06-23  9:44 ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Sun Shaojie @ 2026-06-23  9:34 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long
  Cc: linux-kernel, Sun Shaojie

KCSAN reports a data race in readers_active_check():

  BUG: KCSAN: data-race in readers_active_check / percpu_down_write

  race at unknown origin, with read to 0xffff9f3eb5bf5f30 of 4 bytes
  by task 1271 on cpu 14:
   readers_active_check+0x...
   percpu_down_write+0x152/0x1f0

  value changed: 0xfffffff9 -> 0xfffffff8

This is a benign race. Annotate it with data_race() to suppress the
KCSAN warning.

Signed-off-by: Sun Shaojie <sunshaojie@kylinos.cn>
---
 kernel/locking/percpu-rwsem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index f7e152c40d6d..6c78961fe753 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -211,7 +211,7 @@ EXPORT_SYMBOL_GPL(percpu_is_read_locked);
  */
 static bool readers_active_check(struct percpu_rw_semaphore *sem)
 {
-	if (per_cpu_sum(*sem->read_count) != 0)
+	if (data_race(per_cpu_sum(*sem->read_count)) != 0)
 		return false;
 
 	/*
-- 
2.25.1


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

* Re: [PATCH] locking/percpu-rwsem: Annotate intentional data race in readers_active_check()
  2026-06-23  9:34 [PATCH] locking/percpu-rwsem: Annotate intentional data race in readers_active_check() Sun Shaojie
@ 2026-06-23  9:44 ` Peter Zijlstra
  2026-06-23 10:41   ` [PATCH v2] " Sun Shaojie
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2026-06-23  9:44 UTC (permalink / raw)
  To: Sun Shaojie
  Cc: Ingo Molnar, Will Deacon, Boqun Feng, Waiman Long, linux-kernel

On Tue, Jun 23, 2026 at 05:34:45PM +0800, Sun Shaojie wrote:
> KCSAN reports a data race in readers_active_check():
> 
>   BUG: KCSAN: data-race in readers_active_check / percpu_down_write
> 
>   race at unknown origin, with read to 0xffff9f3eb5bf5f30 of 4 bytes
>   by task 1271 on cpu 14:
>    readers_active_check+0x...
>    percpu_down_write+0x152/0x1f0
> 
>   value changed: 0xfffffff9 -> 0xfffffff8
> 
> This is a benign race. Annotate it with data_race() to suppress the
> KCSAN warning.

I'm not even sure I can tell what actual access is seemed problematic
from this report, let alone tell your reasoning for why it is benign.

And while I think the patch is ok (with my vague memories of the code at
hand), this changelog is entirely insufficient.

> Signed-off-by: Sun Shaojie <sunshaojie@kylinos.cn>
> ---
>  kernel/locking/percpu-rwsem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
> index f7e152c40d6d..6c78961fe753 100644
> --- a/kernel/locking/percpu-rwsem.c
> +++ b/kernel/locking/percpu-rwsem.c
> @@ -211,7 +211,7 @@ EXPORT_SYMBOL_GPL(percpu_is_read_locked);
>   */
>  static bool readers_active_check(struct percpu_rw_semaphore *sem)
>  {
> -	if (per_cpu_sum(*sem->read_count) != 0)
> +	if (data_race(per_cpu_sum(*sem->read_count)) != 0)
>  		return false;
>  
>  	/*
> -- 
> 2.25.1
> 

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

* [PATCH v2] locking/percpu-rwsem: Annotate intentional data race in readers_active_check()
  2026-06-23  9:44 ` Peter Zijlstra
@ 2026-06-23 10:41   ` Sun Shaojie
  2026-06-23 13:25     ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Sun Shaojie @ 2026-06-23 10:41 UTC (permalink / raw)
  To: peterz; +Cc: boqun, linux-kernel, longman, mingo, sunshaojie, will

KCSAN reports a data race between readers_active_check() and a
concurrently executing reader:

  BUG: KCSAN: data-race in readers_active_check / percpu_down_write

  race at unknown origin, with read to 0xffff9f3eb5bf5f30 of 4 bytes
  by task 1271 on cpu 14:
   readers_active_check+0x...
   percpu_down_write+0x152/0x1f0

  value changed: 0xfffffff9 -> 0xfffffff8

readers_active_check() calls per_cpu_sum(*sem->read_count), which
iterates over all CPUs and reads each CPU's per-CPU read_count
variable.  Concurrently, a reader on a remote CPU is modifying its own
CPU's read_count via this_cpu_inc() / this_cpu_dec() as it enters and
exits the critical section.  These are plain reads and writes to the
same per-CPU storage, hence KCSAN flags a data race.

This race is benign.  readers_active_check() is called from the
percpu_down_write() wait loop (rcuwait_wait_event) after sem->block is
already set.  At this point:

  - New readers must immediately back out (they see block set, decrement
    their counter, and wake the writer), so counters can only decrease.

  - If the sum catches a reader's increment before its decrement,
    readers_active_check() sees a non-zero sum and returns false.  The
    writer merely iterates the wait loop again -- a harmless retry.

  - A false zero (observing sum == 0 while a reader is still active)
    cannot happen: per_cpu_sum() reads each CPU's counter, and each
    per-CPU int read is atomic on all architectures, so an active
    reader's counter is always seen as non-zero.

Annotate the read with data_race() to suppress the KCSAN warning and
document the intentional nature of this unlocked access.

Signed-off-by: Sun Shaojie <sunshaojie@kylinos.cn>
---
 kernel/locking/percpu-rwsem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/percpu-rwsem.c b/kernel/locking/percpu-rwsem.c
index f7e152c40d6d..6c78961fe753 100644
--- a/kernel/locking/percpu-rwsem.c
+++ b/kernel/locking/percpu-rwsem.c
@@ -211,7 +211,7 @@ EXPORT_SYMBOL_GPL(percpu_is_read_locked);
  */
 static bool readers_active_check(struct percpu_rw_semaphore *sem)
 {
-	if (per_cpu_sum(*sem->read_count) != 0)
+	if (data_race(per_cpu_sum(*sem->read_count)) != 0)
 		return false;
 
 	/*
-- 
2.25.1


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

* Re: [PATCH v2] locking/percpu-rwsem: Annotate intentional data race in readers_active_check()
  2026-06-23 10:41   ` [PATCH v2] " Sun Shaojie
@ 2026-06-23 13:25     ` Peter Zijlstra
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2026-06-23 13:25 UTC (permalink / raw)
  To: Sun Shaojie; +Cc: boqun, linux-kernel, longman, mingo, will

On Tue, Jun 23, 2026 at 06:41:32PM +0800, Sun Shaojie wrote:
> KCSAN reports a data race between readers_active_check() and a
> concurrently executing reader:
> 
>   BUG: KCSAN: data-race in readers_active_check / percpu_down_write
> 
>   race at unknown origin, with read to 0xffff9f3eb5bf5f30 of 4 bytes
>   by task 1271 on cpu 14:
>    readers_active_check+0x...
>    percpu_down_write+0x152/0x1f0
> 
>   value changed: 0xfffffff9 -> 0xfffffff8
> 
> readers_active_check() calls per_cpu_sum(*sem->read_count), which
> iterates over all CPUs and reads each CPU's per-CPU read_count
> variable.  Concurrently, a reader on a remote CPU is modifying its own
> CPU's read_count via this_cpu_inc() / this_cpu_dec() as it enters and
> exits the critical section.  These are plain reads and writes to the
> same per-CPU storage, hence KCSAN flags a data race.
> 
> This race is benign.  readers_active_check() is called from the
> percpu_down_write() wait loop (rcuwait_wait_event) after sem->block is
> already set.  At this point:
> 
>   - New readers must immediately back out (they see block set, decrement
>     their counter, and wake the writer), so counters can only decrease.
> 
>   - If the sum catches a reader's increment before its decrement,
>     readers_active_check() sees a non-zero sum and returns false.  The
>     writer merely iterates the wait loop again -- a harmless retry.
> 
>   - A false zero (observing sum == 0 while a reader is still active)
>     cannot happen: per_cpu_sum() reads each CPU's counter, and each
>     per-CPU int read is atomic on all architectures, so an active
>     reader's counter is always seen as non-zero.
> 
> Annotate the read with data_race() to suppress the KCSAN warning and
> document the intentional nature of this unlocked access.

Thank you, much better indeed.

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

end of thread, other threads:[~2026-06-23 13:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-23  9:34 [PATCH] locking/percpu-rwsem: Annotate intentional data race in readers_active_check() Sun Shaojie
2026-06-23  9:44 ` Peter Zijlstra
2026-06-23 10:41   ` [PATCH v2] " Sun Shaojie
2026-06-23 13:25     ` Peter Zijlstra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.