From: paulmck@kernel.org
To: rcu@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, kernel-team@fb.com,
mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com,
akpm@linux-foundation.org, mathieu.desnoyers@efficios.com,
josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org,
rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com,
fweisbec@gmail.com, oleg@redhat.com, joel@joelfernandes.org,
"Paul E. McKenney" <paulmck@kernel.org>
Subject: [PATCH tip/core/rcu 04/19] rcu: Add *_ONCE() and data_race() to rcu_node ->exp_tasks plus locking
Date: Wed, 15 Apr 2020 10:10:39 -0700 [thread overview]
Message-ID: <20200415171054.9013-4-paulmck@kernel.org> (raw)
In-Reply-To: <20200415171017.GA7821@paulmck-ThinkPad-P72>
From: "Paul E. McKenney" <paulmck@kernel.org>
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 <paulmck@kernel.org>
---
kernel/rcu/tree_exp.h | 19 +++++++++++--------
kernel/rcu/tree_plugin.h | 8 ++++----
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index 1a617b9..c2b04da 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -150,7 +150,7 @@ static void __maybe_unused sync_exp_reset_tree(void)
static bool sync_rcu_exp_done(struct rcu_node *rnp)
{
raw_lockdep_assert_held_rcu_node(rnp);
- return rnp->exp_tasks == NULL &&
+ return READ_ONCE(rnp->exp_tasks) == NULL &&
READ_ONCE(rnp->expmask) == 0;
}
@@ -373,7 +373,7 @@ static void sync_rcu_exp_select_node_cpus(struct work_struct *wp)
* until such time as the ->expmask bits are cleared.
*/
if (rcu_preempt_has_tasks(rnp))
- rnp->exp_tasks = rnp->blkd_tasks.next;
+ WRITE_ONCE(rnp->exp_tasks, rnp->blkd_tasks.next);
raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
/* IPI the remaining CPUs for expedited quiescent state. */
@@ -542,8 +542,8 @@ static void synchronize_rcu_expedited_wait(void)
}
pr_cont(" } %lu jiffies s: %lu root: %#lx/%c\n",
jiffies - jiffies_start, rcu_state.expedited_sequence,
- READ_ONCE(rnp_root->expmask),
- ".T"[!!rnp_root->exp_tasks]);
+ data_race(rnp_root->expmask),
+ ".T"[!!data_race(rnp_root->exp_tasks)]);
if (ndetected) {
pr_err("blocking rcu_node structures:");
rcu_for_each_node_breadth_first(rnp) {
@@ -553,8 +553,8 @@ static void synchronize_rcu_expedited_wait(void)
continue;
pr_cont(" l=%u:%d-%d:%#lx/%c",
rnp->level, rnp->grplo, rnp->grphi,
- READ_ONCE(rnp->expmask),
- ".T"[!!rnp->exp_tasks]);
+ data_race(rnp->expmask),
+ ".T"[!!data_race(rnp->exp_tasks)]);
}
pr_cont("\n");
}
@@ -721,17 +721,20 @@ static void sync_sched_exp_online_cleanup(int cpu)
*/
static int rcu_print_task_exp_stall(struct rcu_node *rnp)
{
- struct task_struct *t;
+ unsigned long flags;
int ndetected = 0;
+ struct task_struct *t;
- if (!rnp->exp_tasks)
+ if (!READ_ONCE(rnp->exp_tasks))
return 0;
+ raw_spin_lock_irqsave_rcu_node(rnp, flags);
t = list_entry(rnp->exp_tasks->prev,
struct task_struct, rcu_node_entry);
list_for_each_entry_continue(t, &rnp->blkd_tasks, rcu_node_entry) {
pr_cont(" P%d", t->pid);
ndetected++;
}
+ raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
return ndetected;
}
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 097635c..35d77db 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -226,7 +226,7 @@ static void rcu_preempt_ctxt_queue(struct rcu_node *rnp, struct rcu_data *rdp)
WARN_ON_ONCE(rnp->completedqs == rnp->gp_seq);
}
if (!rnp->exp_tasks && (blkd_state & RCU_EXP_BLKD))
- rnp->exp_tasks = &t->rcu_node_entry;
+ WRITE_ONCE(rnp->exp_tasks, &t->rcu_node_entry);
WARN_ON_ONCE(!(blkd_state & RCU_GP_BLKD) !=
!(rnp->qsmask & rdp->grpmask));
WARN_ON_ONCE(!(blkd_state & RCU_EXP_BLKD) !=
@@ -500,7 +500,7 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct *t, unsigned long flags)
if (&t->rcu_node_entry == rnp->gp_tasks)
WRITE_ONCE(rnp->gp_tasks, np);
if (&t->rcu_node_entry == rnp->exp_tasks)
- rnp->exp_tasks = np;
+ WRITE_ONCE(rnp->exp_tasks, np);
if (IS_ENABLED(CONFIG_RCU_BOOST)) {
/* Snapshot ->boost_mtx ownership w/rnp->lock held. */
drop_boost_mutex = rt_mutex_owner(&rnp->boost_mtx) == t;
@@ -761,7 +761,7 @@ dump_blkd_tasks(struct rcu_node *rnp, int ncheck)
__func__, rnp1->grplo, rnp1->grphi, rnp1->qsmask, rnp1->qsmaskinit, rnp1->qsmaskinitnext);
pr_info("%s: ->gp_tasks %p ->boost_tasks %p ->exp_tasks %p\n",
__func__, READ_ONCE(rnp->gp_tasks), rnp->boost_tasks,
- rnp->exp_tasks);
+ READ_ONCE(rnp->exp_tasks));
pr_info("%s: ->blkd_tasks", __func__);
i = 0;
list_for_each(lhp, &rnp->blkd_tasks) {
@@ -1036,7 +1036,7 @@ static int rcu_boost_kthread(void *arg)
for (;;) {
WRITE_ONCE(rnp->boost_kthread_status, RCU_KTHREAD_WAITING);
trace_rcu_utilization(TPS("End boost kthread@rcu_wait"));
- rcu_wait(rnp->boost_tasks || rnp->exp_tasks);
+ rcu_wait(rnp->boost_tasks || READ_ONCE(rnp->exp_tasks));
trace_rcu_utilization(TPS("Start boost kthread@rcu_wait"));
WRITE_ONCE(rnp->boost_kthread_status, RCU_KTHREAD_RUNNING);
more2boost = rcu_boost(rnp);
--
2.9.5
next prev parent reply other threads:[~2020-04-15 17:12 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-15 17:10 [PATCH tip/core/rcu 0/19] Miscellaneous fixes for v5.8 Paul E. McKenney
2020-04-15 17:10 ` [PATCH tip/core/rcu 01/19] rcu: Add KCSAN stubs paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 02/19] srcu: " paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 03/19] rcu: Mark rcu_state.ncpus to detect concurrent writes paulmck
2020-04-15 17:10 ` paulmck [this message]
2020-04-15 17:10 ` [PATCH tip/core/rcu 05/19] rcu: Add READ_ONCE and data_race() to rcu_node ->boost_tasks paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 06/19] srcu: Add data_race() to ->srcu_lock_count and ->srcu_unlock_count arrays paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 07/19] rcu: Add WRITE_ONCE() to rcu_node ->boost_tasks paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 08/19] rcu: Use data_race() for RCU CPU stall-warning prints paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 09/19] drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compilers paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 10/19] rcu: Expedite first two FQS scans under callback-overload conditions paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 11/19] rcu: Fix the (t=0 jiffies) false positive paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 12/19] Revert "rculist: Describe variadic macro argument in a Sphinx-compatible way" paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 13/19] rcu: Get rid of some doc warnings in update.c paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 14/19] rcu: Mark rcu_state.gp_seq to detect more concurrent writes paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 15/19] rcu: Replace assigned pointer ret value by corresponding boolean value paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 16/19] rcu: Replace 1 by true paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 17/19] rcu: Convert ULONG_CMP_GE() to time_after() for jiffy comparison paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 18/19] rcu: Convert rcu_initiate_boost() ULONG_CMP_GE() to time_after() paulmck
2020-04-15 17:10 ` [PATCH tip/core/rcu 19/19] rcu: Convert rcu_nohz_full_cpu() ULONG_CMP_LT() to time_before() paulmck
2020-04-15 17:21 ` [PATCH tip/core/rcu 0/19] Miscellaneous fixes for v5.8 Andrew Morton
2020-04-15 17:32 ` Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200415171054.9013-4-paulmck@kernel.org \
--to=paulmck@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=dipankar@in.ibm.com \
--cc=edumazet@google.com \
--cc=fweisbec@gmail.com \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox