[PATCH] srcu: Fix a rare race issue in __srcu_read_(un)lock()

[Pingfan Liu <kernelfans@xxxxxxxxx>]

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@xxxxxxxxx>
Cc: Lai Jiangshan <jiangshanlai@xxxxxxxxx>
Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxx>
Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
To: rcu@xxxxxxxxxxxxxxx
---
 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