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 <[email protected]>
---
 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


Reply via email to