Skip to content

Commit 314eeb4

Browse files
committed
rcu: Add *_ONCE() and data_race() to rcu_node ->exp_tasks plus locking
There are lockless loads from the rcu_node structure's ->exp_tasks field, so this commit causes all stores to use WRITE_ONCE() and all lockless loads to use READ_ONCE() or data_race(), with the latter for debug prints. This code also did a unprotected traversal of the linked list pointed into by ->exp_tasks, so this commit also acquires the rcu_node structure's ->lock to properly protect this traversal. This list was traversed unprotected only when printing an RCU CPU stall warning for an expedited grace period, so the odds of seeing this in production are not all that high. This data race was reported by KCSAN. Signed-off-by: Paul E. McKenney <[email protected]>
1 parent 2f08469 commit 314eeb4

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

kernel/rcu/tree_exp.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ static void __maybe_unused sync_exp_reset_tree(void)
150150
static bool sync_rcu_exp_done(struct rcu_node *rnp)
151151
{
152152
raw_lockdep_assert_held_rcu_node(rnp);
153-
return rnp->exp_tasks == NULL &&
153+
return READ_ONCE(rnp->exp_tasks) == NULL &&
154154
READ_ONCE(rnp->expmask) == 0;
155155
}
156156

@@ -373,7 +373,7 @@ static void sync_rcu_exp_select_node_cpus(struct work_struct *wp)
373373
* until such time as the ->expmask bits are cleared.
374374
*/
375375
if (rcu_preempt_has_tasks(rnp))
376-
rnp->exp_tasks = rnp->blkd_tasks.next;
376+
WRITE_ONCE(rnp->exp_tasks, rnp->blkd_tasks.next);
377377
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
378378

379379
/* IPI the remaining CPUs for expedited quiescent state. */
@@ -542,8 +542,8 @@ static void synchronize_rcu_expedited_wait(void)
542542
}
543543
pr_cont(" } %lu jiffies s: %lu root: %#lx/%c\n",
544544
jiffies - jiffies_start, rcu_state.expedited_sequence,
545-
READ_ONCE(rnp_root->expmask),
546-
".T"[!!rnp_root->exp_tasks]);
545+
data_race(rnp_root->expmask),
546+
".T"[!!data_race(rnp_root->exp_tasks)]);
547547
if (ndetected) {
548548
pr_err("blocking rcu_node structures:");
549549
rcu_for_each_node_breadth_first(rnp) {
@@ -553,8 +553,8 @@ static void synchronize_rcu_expedited_wait(void)
553553
continue;
554554
pr_cont(" l=%u:%d-%d:%#lx/%c",
555555
rnp->level, rnp->grplo, rnp->grphi,
556-
READ_ONCE(rnp->expmask),
557-
".T"[!!rnp->exp_tasks]);
556+
data_race(rnp->expmask),
557+
".T"[!!data_race(rnp->exp_tasks)]);
558558
}
559559
pr_cont("\n");
560560
}
@@ -721,17 +721,20 @@ static void sync_sched_exp_online_cleanup(int cpu)
721721
*/
722722
static int rcu_print_task_exp_stall(struct rcu_node *rnp)
723723
{
724-
struct task_struct *t;
724+
unsigned long flags;
725725
int ndetected = 0;
726+
struct task_struct *t;
726727

727-
if (!rnp->exp_tasks)
728+
if (!READ_ONCE(rnp->exp_tasks))
728729
return 0;
730+
raw_spin_lock_irqsave_rcu_node(rnp, flags);
729731
t = list_entry(rnp->exp_tasks->prev,
730732
struct task_struct, rcu_node_entry);
731733
list_for_each_entry_continue(t, &rnp->blkd_tasks, rcu_node_entry) {
732734
pr_cont(" P%d", t->pid);
733735
ndetected++;
734736
}
737+
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
735738
return ndetected;
736739
}
737740

kernel/rcu/tree_plugin.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
226226
WARN_ON_ONCE(rnp->completedqs == rnp->gp_seq);
227227
}
228228
if (!rnp->exp_tasks && (blkd_state & RCU_EXP_BLKD))
229-
rnp->exp_tasks = &t->rcu_node_entry;
229+
WRITE_ONCE(rnp->exp_tasks, &t->rcu_node_entry);
230230
WARN_ON_ONCE(!(blkd_state & RCU_GP_BLKD) !=
231231
!(rnp->qsmask & rdp->grpmask));
232232
WARN_ON_ONCE(!(blkd_state & RCU_EXP_BLKD) !=
@@ -500,7 +500,7 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct *t, unsigned long flags)
500500
if (&t->rcu_node_entry == rnp->gp_tasks)
501501
WRITE_ONCE(rnp->gp_tasks, np);
502502
if (&t->rcu_node_entry == rnp->exp_tasks)
503-
rnp->exp_tasks = np;
503+
WRITE_ONCE(rnp->exp_tasks, np);
504504
if (IS_ENABLED(CONFIG_RCU_BOOST)) {
505505
/* Snapshot ->boost_mtx ownership w/rnp->lock held. */
506506
drop_boost_mutex = rt_mutex_owner(&rnp->boost_mtx) == t;
@@ -761,7 +761,7 @@ dump_blkd_tasks(struct rcu_node *rnp, int ncheck)
761761
__func__, rnp1->grplo, rnp1->grphi, rnp1->qsmask, rnp1->qsmaskinit, rnp1->qsmaskinitnext);
762762
pr_info("%s: ->gp_tasks %p ->boost_tasks %p ->exp_tasks %p\n",
763763
__func__, READ_ONCE(rnp->gp_tasks), rnp->boost_tasks,
764-
rnp->exp_tasks);
764+
READ_ONCE(rnp->exp_tasks));
765765
pr_info("%s: ->blkd_tasks", __func__);
766766
i = 0;
767767
list_for_each(lhp, &rnp->blkd_tasks) {
@@ -1036,7 +1036,7 @@ static int rcu_boost_kthread(void *arg)
10361036
for (;;) {
10371037
WRITE_ONCE(rnp->boost_kthread_status, RCU_KTHREAD_WAITING);
10381038
trace_rcu_utilization(TPS("End boost kthread@rcu_wait"));
1039-
rcu_wait(rnp->boost_tasks || rnp->exp_tasks);
1039+
rcu_wait(rnp->boost_tasks || READ_ONCE(rnp->exp_tasks));
10401040
trace_rcu_utilization(TPS("Start boost kthread@rcu_wait"));
10411041
WRITE_ONCE(rnp->boost_kthread_status, RCU_KTHREAD_RUNNING);
10421042
more2boost = rcu_boost(rnp);

0 commit comments

Comments
 (0)