From: Pingfan Liu <kernelfans@gmail.com>
To: rcu@vger.kernel.org
Cc: Pingfan Liu <kernelfans@gmail.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
"Paul E. McKenney" <paulmck@kernel.org>,
Josh Triplett <josh@joshtriplett.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Subject: [PATCH] srcu: Fix a rare race issue in __srcu_read_(un)lock()
Date: Thu, 3 Nov 2022 21:13:13 +0800 [thread overview]
Message-ID: <20221103131313.41536-1-kernelfans@gmail.com> (raw)
Clarify at first:
This issue is totally detected by code suspicion, not a real experience.
Scene:
__srcu_read_(un)lock() uses percpu variable srcu_(un)lock_count[2].
Normally, the percpu can help avoid the non-atomic RMW issue, but in
some rare cases, it can not.
Supposing that __srcu_read_lock() runs on cpuX, the statement
this_cpu_inc(ssp->sda->srcu_lock_count[idx]);
can be decomposed into two sub group:
-1. get the address of this_cpu_ptr(ssp->sda)->srcu_lock_count[idx],
denoted as addressX and let unsigned long *pX = addressX;
-2. *pX = *pX + 1;
Now, assuming there are two tasks, denoted as taskA and taskB, three
cpus, denoted as cpuX, cpuY and cpuZ. Let both taskA and taskB finish
the first step as above on cpuX, but migrate to cpuY and cpuZ
individually just before the second step. Then both of them continue to
execute concurrently from the second step. This will raise a typical
non-atomic RMW issue.
Solution:
This issue can be tackled by disable preemption around the two sub
groups.
Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
Cc: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: rcu@vger.kernel.org
---
kernel/rcu/srcutree.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 1c304fec89c0..a38a3779dc01 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -636,7 +636,11 @@ int __srcu_read_lock(struct srcu_struct *ssp)
int idx;
idx = READ_ONCE(ssp->srcu_idx) & 0x1;
+ __preempt_count_inc();
+ barrier();
this_cpu_inc(ssp->sda->srcu_lock_count[idx]);
+ barrier();
+ __preempt_count_dec();
smp_mb(); /* B */ /* Avoid leaking the critical section. */
return idx;
}
@@ -650,7 +654,11 @@ EXPORT_SYMBOL_GPL(__srcu_read_lock);
void __srcu_read_unlock(struct srcu_struct *ssp, int idx)
{
smp_mb(); /* C */ /* Avoid leaking the critical section. */
+ __preempt_count_inc();
+ barrier();
this_cpu_inc(ssp->sda->srcu_unlock_count[idx]);
+ barrier();
+ __preempt_count_dec();
}
EXPORT_SYMBOL_GPL(__srcu_read_unlock);
--
2.31.1
next reply other threads:[~2022-11-03 13:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-03 13:13 Pingfan Liu [this message]
2022-11-03 13:36 ` [PATCH] srcu: Fix a rare race issue in __srcu_read_(un)lock() Frederic Weisbecker
2022-11-03 15:05 ` Mathieu Desnoyers
2022-11-04 9:38 ` Pingfan Liu
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=20221103131313.41536-1-kernelfans@gmail.com \
--to=kernelfans@gmail.com \
--cc=jiangshanlai@gmail.com \
--cc=josh@joshtriplett.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=paulmck@kernel.org \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.