[PATCH] rcu: Mark accesses to ->rcu_urgent_qs and ->rcu_need_heavy_qs

[PATCH] rcu: Mark accesses to ->rcu_urgent_qs and ->rcu_need_heavy_qs

[Itai Handler]

rcu_all_qs() and rcu_note_context_switch() read/clear the per-CPU
->rcu_urgent_qs and ->rcu_need_heavy_qs flags with plain raw_cpu_read()
and this_cpu_write(), while the RCU core clears them with WRITE_ONCE() in
rcu_disable_urgency_upon_qs().  KCSAN flags the resulting same-CPU race:

  BUG: KCSAN: data-race in rcu_all_qs / rcu_disable_urgency_upon_qs

It is benign -- the flags are advisory and rcu_all_qs() re-reads
->rcu_urgent_qs with smp_load_acquire() before acting on it -- but these
are the last unmarked accesses to the two flags; every other access
already uses READ_ONCE()/WRITE_ONCE()/smp_*.  Mark them to match.  No
functional change.

Reproduced on a PREEMPT_NONE, CONFIG_KCSAN_INTERRUPT_WATCHER=y kernel with
a pthreads program whose threads (two per CPU) loop reading a large file:

	for (;;) {
		int fd = open("/proc/kallsyms", O_RDONLY);
		while (read(fd, buf, sizeof(buf)) > 0)
			;
		close(fd);
	}

The read()s drive cond_resched() -> rcu_all_qs() while the busy CPUs keep
the grace period urgent, so the RCU core clears the flags concurrently.

Fixes: 2dba13f0b6c2 ("rcu: Switch urgent quiescent-state requests to rcu_data structure")
Signed-off-by: Itai Handler <itai.handler@gmail.com>
---
 kernel/rcu/tree_plugin.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 95ad967adcf3..608f73286647 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -970,7 +970,7 @@ void rcu_all_qs(void)
 {
 	unsigned long flags;
 
-	if (!raw_cpu_read(rcu_data.rcu_urgent_qs))
+	if (!READ_ONCE(*raw_cpu_ptr(&rcu_data.rcu_urgent_qs)))
 		return;
 	preempt_disable();  // For CONFIG_PREEMPT_COUNT=y kernels
 	/* Load rcu_urgent_qs before other flags. */
@@ -978,8 +978,8 @@ void rcu_all_qs(void)
 		preempt_enable();
 		return;
 	}
-	this_cpu_write(rcu_data.rcu_urgent_qs, false);
-	if (unlikely(raw_cpu_read(rcu_data.rcu_need_heavy_qs))) {
+	WRITE_ONCE(*this_cpu_ptr(&rcu_data.rcu_urgent_qs), false);
+	if (unlikely(READ_ONCE(*this_cpu_ptr(&rcu_data.rcu_need_heavy_qs)))) {
 		local_irq_save(flags);
 		rcu_momentary_eqs();
 		local_irq_restore(flags);
@@ -999,8 +999,8 @@ void rcu_note_context_switch(bool preempt)
 	/* Load rcu_urgent_qs before other flags. */
 	if (!smp_load_acquire(this_cpu_ptr(&rcu_data.rcu_urgent_qs)))
 		goto out;
-	this_cpu_write(rcu_data.rcu_urgent_qs, false);
-	if (unlikely(raw_cpu_read(rcu_data.rcu_need_heavy_qs)))
+	WRITE_ONCE(*this_cpu_ptr(&rcu_data.rcu_urgent_qs), false);
+	if (unlikely(READ_ONCE(*this_cpu_ptr(&rcu_data.rcu_need_heavy_qs))))
 		rcu_momentary_eqs();
 out:
 	rcu_tasks_qs(current, preempt);
-- 
2.34.1

Re: [PATCH] rcu: Mark accesses to ->rcu_urgent_qs and ->rcu_need_heavy_qs

[Paul E. McKenney]

On Thu, Jul 23, 2026 at 10:41:38AM +0300, Itai Handler wrote:
> rcu_all_qs() and rcu_note_context_switch() read/clear the per-CPU
> ->rcu_urgent_qs and ->rcu_need_heavy_qs flags with plain raw_cpu_read()
> and this_cpu_write(), while the RCU core clears them with WRITE_ONCE() in
> rcu_disable_urgency_upon_qs().  KCSAN flags the resulting same-CPU race:
> 
>   BUG: KCSAN: data-race in rcu_all_qs / rcu_disable_urgency_upon_qs
> 
> It is benign -- the flags are advisory and rcu_all_qs() re-reads
> ->rcu_urgent_qs with smp_load_acquire() before acting on it -- but these
> are the last unmarked accesses to the two flags; every other access
> already uses READ_ONCE()/WRITE_ONCE()/smp_*.  Mark them to match.  No
> functional change.
> 
> Reproduced on a PREEMPT_NONE, CONFIG_KCSAN_INTERRUPT_WATCHER=y kernel with
> a pthreads program whose threads (two per CPU) loop reading a large file:
> 
> 	for (;;) {
> 		int fd = open("/proc/kallsyms", O_RDONLY);
> 		while (read(fd, buf, sizeof(buf)) > 0)
> 			;
> 		close(fd);
> 	}
> 
> The read()s drive cond_resched() -> rcu_all_qs() while the busy CPUs keep
> the grace period urgent, so the RCU core clears the flags concurrently.
> 
> Fixes: 2dba13f0b6c2 ("rcu: Switch urgent quiescent-state requests to rcu_data structure")
> Signed-off-by: Itai Handler <itai.handler@gmail.com>

Nice!!!  Queued for further review and testing, thank you!

							Thanx, Paul

> ---
>  kernel/rcu/tree_plugin.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index 95ad967adcf3..608f73286647 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -970,7 +970,7 @@ void rcu_all_qs(void)
>  {
>  	unsigned long flags;
>  
> -	if (!raw_cpu_read(rcu_data.rcu_urgent_qs))
> +	if (!READ_ONCE(*raw_cpu_ptr(&rcu_data.rcu_urgent_qs)))
>  		return;
>  	preempt_disable();  // For CONFIG_PREEMPT_COUNT=y kernels
>  	/* Load rcu_urgent_qs before other flags. */
> @@ -978,8 +978,8 @@ void rcu_all_qs(void)
>  		preempt_enable();
>  		return;
>  	}
> -	this_cpu_write(rcu_data.rcu_urgent_qs, false);
> -	if (unlikely(raw_cpu_read(rcu_data.rcu_need_heavy_qs))) {
> +	WRITE_ONCE(*this_cpu_ptr(&rcu_data.rcu_urgent_qs), false);
> +	if (unlikely(READ_ONCE(*this_cpu_ptr(&rcu_data.rcu_need_heavy_qs)))) {
>  		local_irq_save(flags);
>  		rcu_momentary_eqs();
>  		local_irq_restore(flags);
> @@ -999,8 +999,8 @@ void rcu_note_context_switch(bool preempt)
>  	/* Load rcu_urgent_qs before other flags. */
>  	if (!smp_load_acquire(this_cpu_ptr(&rcu_data.rcu_urgent_qs)))
>  		goto out;
> -	this_cpu_write(rcu_data.rcu_urgent_qs, false);
> -	if (unlikely(raw_cpu_read(rcu_data.rcu_need_heavy_qs)))
> +	WRITE_ONCE(*this_cpu_ptr(&rcu_data.rcu_urgent_qs), false);
> +	if (unlikely(READ_ONCE(*this_cpu_ptr(&rcu_data.rcu_need_heavy_qs))))
>  		rcu_momentary_eqs();
>  out:
>  	rcu_tasks_qs(current, preempt);
> -- 
> 2.34.1
>