Skip to content

Commit 8ff3729

Browse files
committed
rcu: Add *_ONCE() for grace-period progress indicators
The various RCU structures' ->gp_seq, ->gp_seq_needed, ->gp_req_activity, and ->gp_activity fields are read locklessly, so they must be updated with WRITE_ONCE() and, when read locklessly, with READ_ONCE(). This commit makes these changes. This data race was reported by KCSAN. Not appropriate for backporting due to failure being unlikely. Signed-off-by: Paul E. McKenney <[email protected]>
1 parent bfeebe2 commit 8ff3729

File tree

3 files changed

+23
-19
lines changed

3 files changed

+23
-19
lines changed

kernel/rcu/tree.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,7 +1175,7 @@ static bool rcu_start_this_gp(struct rcu_node *rnp_start, struct rcu_data *rdp,
11751175
TPS("Prestarted"));
11761176
goto unlock_out;
11771177
}
1178-
rnp->gp_seq_needed = gp_seq_req;
1178+
WRITE_ONCE(rnp->gp_seq_needed, gp_seq_req);
11791179
if (rcu_seq_state(rcu_seq_current(&rnp->gp_seq))) {
11801180
/*
11811181
* We just marked the leaf or internal node, and a
@@ -1210,8 +1210,8 @@ static bool rcu_start_this_gp(struct rcu_node *rnp_start, struct rcu_data *rdp,
12101210
unlock_out:
12111211
/* Push furthest requested GP to leaf node and rcu_data structure. */
12121212
if (ULONG_CMP_LT(gp_seq_req, rnp->gp_seq_needed)) {
1213-
rnp_start->gp_seq_needed = rnp->gp_seq_needed;
1214-
rdp->gp_seq_needed = rnp->gp_seq_needed;
1213+
WRITE_ONCE(rnp_start->gp_seq_needed, rnp->gp_seq_needed);
1214+
WRITE_ONCE(rdp->gp_seq_needed, rnp->gp_seq_needed);
12151215
}
12161216
if (rnp != rnp_start)
12171217
raw_spin_unlock_rcu_node(rnp);
@@ -1423,7 +1423,7 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp)
14231423
}
14241424
rdp->gp_seq = rnp->gp_seq; /* Remember new grace-period state. */
14251425
if (ULONG_CMP_LT(rdp->gp_seq_needed, rnp->gp_seq_needed) || rdp->gpwrap)
1426-
rdp->gp_seq_needed = rnp->gp_seq_needed;
1426+
WRITE_ONCE(rdp->gp_seq_needed, rnp->gp_seq_needed);
14271427
WRITE_ONCE(rdp->gpwrap, false);
14281428
rcu_gpnum_ovf(rnp, rdp);
14291429
return ret;
@@ -3276,12 +3276,12 @@ int rcutree_prepare_cpu(unsigned int cpu)
32763276
rnp = rdp->mynode;
32773277
raw_spin_lock_rcu_node(rnp); /* irqs already disabled. */
32783278
rdp->beenonline = true; /* We have now been online. */
3279-
rdp->gp_seq = rnp->gp_seq;
3280-
rdp->gp_seq_needed = rnp->gp_seq;
3279+
rdp->gp_seq = READ_ONCE(rnp->gp_seq);
3280+
rdp->gp_seq_needed = rdp->gp_seq;
32813281
rdp->cpu_no_qs.b.norm = true;
32823282
rdp->core_needs_qs = false;
32833283
rdp->rcu_iw_pending = false;
3284-
rdp->rcu_iw_gp_seq = rnp->gp_seq - 1;
3284+
rdp->rcu_iw_gp_seq = rdp->gp_seq - 1;
32853285
trace_rcu_grace_period(rcu_state.name, rdp->gp_seq, TPS("cpuonl"));
32863286
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
32873287
rcu_prepare_kthreads(cpu);

kernel/rcu/tree_plugin.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -753,7 +753,7 @@ dump_blkd_tasks(struct rcu_node *rnp, int ncheck)
753753
raw_lockdep_assert_held_rcu_node(rnp);
754754
pr_info("%s: grp: %d-%d level: %d ->gp_seq %ld ->completedqs %ld\n",
755755
__func__, rnp->grplo, rnp->grphi, rnp->level,
756-
(long)rnp->gp_seq, (long)rnp->completedqs);
756+
(long)READ_ONCE(rnp->gp_seq), (long)rnp->completedqs);
757757
for (rnp1 = rnp; rnp1; rnp1 = rnp1->parent)
758758
pr_info("%s: %d:%d ->qsmask %#lx ->qsmaskinit %#lx ->qsmaskinitnext %#lx\n",
759759
__func__, rnp1->grplo, rnp1->grphi, rnp1->qsmask, rnp1->qsmaskinit, rnp1->qsmaskinitnext);

kernel/rcu/tree_stall.h

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -592,21 +592,22 @@ void show_rcu_gp_kthreads(void)
592592
(long)READ_ONCE(rcu_get_root()->gp_seq_needed),
593593
READ_ONCE(rcu_state.gp_flags));
594594
rcu_for_each_node_breadth_first(rnp) {
595-
if (ULONG_CMP_GE(rcu_state.gp_seq, rnp->gp_seq_needed))
595+
if (ULONG_CMP_GE(READ_ONCE(rcu_state.gp_seq),
596+
READ_ONCE(rnp->gp_seq_needed)))
596597
continue;
597598
pr_info("\trcu_node %d:%d ->gp_seq %ld ->gp_seq_needed %ld\n",
598-
rnp->grplo, rnp->grphi, (long)rnp->gp_seq,
599-
(long)rnp->gp_seq_needed);
599+
rnp->grplo, rnp->grphi, (long)READ_ONCE(rnp->gp_seq),
600+
(long)READ_ONCE(rnp->gp_seq_needed));
600601
if (!rcu_is_leaf_node(rnp))
601602
continue;
602603
for_each_leaf_node_possible_cpu(rnp, cpu) {
603604
rdp = per_cpu_ptr(&rcu_data, cpu);
604605
if (rdp->gpwrap ||
605-
ULONG_CMP_GE(rcu_state.gp_seq,
606-
rdp->gp_seq_needed))
606+
ULONG_CMP_GE(READ_ONCE(rcu_state.gp_seq),
607+
READ_ONCE(rdp->gp_seq_needed)))
607608
continue;
608609
pr_info("\tcpu %d ->gp_seq_needed %ld\n",
609-
cpu, (long)rdp->gp_seq_needed);
610+
cpu, (long)READ_ONCE(rdp->gp_seq_needed));
610611
}
611612
}
612613
for_each_possible_cpu(cpu) {
@@ -631,7 +632,8 @@ static void rcu_check_gp_start_stall(struct rcu_node *rnp, struct rcu_data *rdp,
631632
static atomic_t warned = ATOMIC_INIT(0);
632633

633634
if (!IS_ENABLED(CONFIG_PROVE_RCU) || rcu_gp_in_progress() ||
634-
ULONG_CMP_GE(rnp_root->gp_seq, rnp_root->gp_seq_needed))
635+
ULONG_CMP_GE(READ_ONCE(rnp_root->gp_seq),
636+
READ_ONCE(rnp_root->gp_seq_needed)))
635637
return;
636638
j = jiffies; /* Expensive access, and in common case don't get here. */
637639
if (time_before(j, READ_ONCE(rcu_state.gp_req_activity) + gpssdelay) ||
@@ -642,7 +644,8 @@ static void rcu_check_gp_start_stall(struct rcu_node *rnp, struct rcu_data *rdp,
642644
raw_spin_lock_irqsave_rcu_node(rnp, flags);
643645
j = jiffies;
644646
if (rcu_gp_in_progress() ||
645-
ULONG_CMP_GE(rnp_root->gp_seq, rnp_root->gp_seq_needed) ||
647+
ULONG_CMP_GE(READ_ONCE(rnp_root->gp_seq),
648+
READ_ONCE(rnp_root->gp_seq_needed)) ||
646649
time_before(j, READ_ONCE(rcu_state.gp_req_activity) + gpssdelay) ||
647650
time_before(j, READ_ONCE(rcu_state.gp_activity) + gpssdelay) ||
648651
atomic_read(&warned)) {
@@ -655,9 +658,10 @@ static void rcu_check_gp_start_stall(struct rcu_node *rnp, struct rcu_data *rdp,
655658
raw_spin_lock_rcu_node(rnp_root); /* irqs already disabled. */
656659
j = jiffies;
657660
if (rcu_gp_in_progress() ||
658-
ULONG_CMP_GE(rnp_root->gp_seq, rnp_root->gp_seq_needed) ||
659-
time_before(j, rcu_state.gp_req_activity + gpssdelay) ||
660-
time_before(j, rcu_state.gp_activity + gpssdelay) ||
661+
ULONG_CMP_GE(READ_ONCE(rnp_root->gp_seq),
662+
READ_ONCE(rnp_root->gp_seq_needed)) ||
663+
time_before(j, READ_ONCE(rcu_state.gp_req_activity) + gpssdelay) ||
664+
time_before(j, READ_ONCE(rcu_state.gp_activity) + gpssdelay) ||
661665
atomic_xchg(&warned, 1)) {
662666
if (rnp_root != rnp)
663667
/* irqs remain disabled. */

0 commit comments

Comments
 (0)