rcu: Don't use negative nesting depth in __rcu_read_unlock()

Lai Jiangshan · paulmckrcu · commit 5f5fa7ea89dc · 2020-04-27T11:03:50.000-07:00
Now that RCU flavors have been consolidated, an RCU-preempt
rcu_read_unlock() in an interrupt or softirq handler cannot possibly
end the RCU read-side critical section.  Consider the old vulnerability
involving rcu_read_unlock() being invoked within such a handler that
interrupted an __rcu_read_unlock_special(), in which a wakeup might be
invoked with a scheduler lock held.  Because rcu_read_unlock_special()
no longer does wakeups in such situations, it is no longer necessary
for __rcu_read_unlock() to set the nesting level negative.

This commit therefore removes this recursion-protection code from
__rcu_read_unlock().

[ paulmck: Let rcu_exp_handler() continue to call rcu_report_exp_rdp(). ]
[ paulmck: Adjust other checks given no more negative nesting. ]
Signed-off-by: Lai Jiangshan &lt;laijs@linux.alibaba.com&gt;
Signed-off-by: Paul E. McKenney &lt;paulmck@kernel.org&gt;
diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
@@ -639,6 +639,7 @@ static void wait_rcu_exp_gp(struct work_struct *wp)
  */
 static void rcu_exp_handler(void *unused)
 {
+	int depth = rcu_preempt_depth();
 	unsigned long flags;
 	struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
 	struct rcu_node *rnp = rdp->mynode;
@@ -649,7 +650,7 @@ static void rcu_exp_handler(void *unused)
 	 * critical section.  If also enabled or idle, immediately
 	 * report the quiescent state, otherwise defer.
 	 */
-	if (!rcu_preempt_depth()) {
+	if (!depth) {
 		if (!(preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK)) ||
 		    rcu_dynticks_curr_cpu_in_eqs()) {
 			rcu_report_exp_rdp(rdp);
@@ -673,7 +674,7 @@ static void rcu_exp_handler(void *unused)
 	 * can have caused this quiescent state to already have been
 	 * reported, so we really do need to check ->expmask.
 	 */
-	if (rcu_preempt_depth() > 0) {
+	if (depth > 0) {
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 		if (rnp->expmask & rdp->grpmask) {
 			rdp->exp_deferred_qs = true;
@@ -683,30 +684,8 @@ static void rcu_exp_handler(void *unused)
 		return;
 	}
 
-	/*
-	 * The final and least likely case is where the interrupted
-	 * code was just about to or just finished exiting the RCU-preempt
-	 * read-side critical section, and no, we can't tell which.
-	 * So either way, set ->deferred_qs to flag later code that
-	 * a quiescent state is required.
-	 *
-	 * If the CPU is fully enabled (or if some buggy RCU-preempt
-	 * read-side critical section is being used from idle), just
-	 * invoke rcu_preempt_deferred_qs() to immediately report the
-	 * quiescent state.  We cannot use rcu_read_unlock_special()
-	 * because we are in an interrupt handler, which will cause that
-	 * function to take an early exit without doing anything.
-	 *
-	 * Otherwise, force a context switch after the CPU enables everything.
-	 */
-	rdp->exp_deferred_qs = true;
-	if (!(preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK)) ||
-	    WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs())) {
-		rcu_preempt_deferred_qs(t);
-	} else {
-		set_tsk_need_resched(t);
-		set_preempt_need_resched();
-	}
+	// Finally, negative nesting depth should not happen.
+	WARN_ON_ONCE(1);
 }
 
 /* PREEMPTION=y, so no PREEMPTION=n expedited grace period to clean up after. */
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
@@ -345,19 +345,17 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node *rnp)
 	return READ_ONCE(rnp->gp_tasks) != NULL;
 }
 
-/* Bias and limit values for ->rcu_read_lock_nesting. */
-#define RCU_NEST_BIAS INT_MAX
-#define RCU_NEST_NMAX (-INT_MAX / 2)
+/* limit value for ->rcu_read_lock_nesting. */
 #define RCU_NEST_PMAX (INT_MAX / 2)
 
 static void rcu_preempt_read_enter(void)
 {
 	current->rcu_read_lock_nesting++;
 }
 
-static void rcu_preempt_read_exit(void)
+static int rcu_preempt_read_exit(void)
 {
-	current->rcu_read_lock_nesting--;
+	return --current->rcu_read_lock_nesting;
 }
 
 static void rcu_preempt_depth_set(int val)
@@ -390,21 +388,15 @@ void __rcu_read_unlock(void)
 {
 	struct task_struct *t = current;
 
-	if (rcu_preempt_depth() != 1) {
-		rcu_preempt_read_exit();
-	} else {
+	if (rcu_preempt_read_exit() == 0) {
 		barrier();  /* critical section before exit code. */
-		rcu_preempt_depth_set(-RCU_NEST_BIAS);
-		barrier();  /* assign before ->rcu_read_unlock_special load */
 		if (unlikely(READ_ONCE(t->rcu_read_unlock_special.s)))
 			rcu_read_unlock_special(t);
-		barrier();  /* ->rcu_read_unlock_special load before assign */
-		rcu_preempt_depth_set(0);
 	}
 	if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
 		int rrln = rcu_preempt_depth();
 
-		WARN_ON_ONCE(rrln < 0 && rrln > RCU_NEST_NMAX);
+		WARN_ON_ONCE(rrln < 0 || rrln > RCU_NEST_PMAX);
 	}
 }
 EXPORT_SYMBOL_GPL(__rcu_read_unlock);
@@ -556,7 +548,7 @@ static bool rcu_preempt_need_deferred_qs(struct task_struct *t)
 {
 	return (__this_cpu_read(rcu_data.exp_deferred_qs) ||
 		READ_ONCE(t->rcu_read_unlock_special.s)) &&
-	       rcu_preempt_depth() <= 0;
+	       rcu_preempt_depth() == 0;
 }
 
 /*
@@ -692,7 +684,7 @@ static void rcu_flavor_sched_clock_irq(int user)
 	} else if (rcu_preempt_need_deferred_qs(t)) {
 		rcu_preempt_deferred_qs(t); /* Report deferred QS. */
 		return;
-	} else if (!rcu_preempt_depth()) {
+	} else if (!WARN_ON_ONCE(rcu_preempt_depth())) {
 		rcu_qs(); /* Report immediate QS. */
 		return;
 	}

Original file line number	Diff line number	Diff line change
`@@ -345,19 +345,17 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node *rnp)`
`345`	`345`	`return READ_ONCE(rnp->gp_tasks) != NULL;`
`346`	`346`	`}`
`347`	`347`
`348`		`-/* Bias and limit values for ->rcu_read_lock_nesting. */`
`349`		`-#define RCU_NEST_BIAS INT_MAX`
`350`		`-#define RCU_NEST_NMAX (-INT_MAX / 2)`
	`348`	`+/* limit value for ->rcu_read_lock_nesting. */`
`351`	`349`	`#define RCU_NEST_PMAX (INT_MAX / 2)`
`352`	`350`
`353`	`351`	`static void rcu_preempt_read_enter(void)`
`354`	`352`	`{`
`355`	`353`	`current->rcu_read_lock_nesting++;`
`356`	`354`	`}`
`357`	`355`
`358`		`-static void rcu_preempt_read_exit(void)`
	`356`	`+static int rcu_preempt_read_exit(void)`
`359`	`357`	`{`
`360`		`- current->rcu_read_lock_nesting--;`
	`358`	`+ return --current->rcu_read_lock_nesting;`
`361`	`359`	`}`
`362`	`360`
`363`	`361`	`static void rcu_preempt_depth_set(int val)`
`@@ -390,21 +388,15 @@ void __rcu_read_unlock(void)`
`390`	`388`	`{`
`391`	`389`	`struct task_struct *t = current;`
`392`	`390`
`393`		`- if (rcu_preempt_depth() != 1) {`
`394`		`- rcu_preempt_read_exit();`
`395`		`- } else {`
	`391`	`+ if (rcu_preempt_read_exit() == 0) {`
`396`	`392`	`barrier(); /* critical section before exit code. */`
`397`		`- rcu_preempt_depth_set(-RCU_NEST_BIAS);`
`398`		`- barrier(); /* assign before ->rcu_read_unlock_special load */`
`399`	`393`	`if (unlikely(READ_ONCE(t->rcu_read_unlock_special.s)))`
`400`	`394`	`rcu_read_unlock_special(t);`
`401`		`- barrier(); /* ->rcu_read_unlock_special load before assign */`
`402`		`- rcu_preempt_depth_set(0);`
`403`	`395`	`}`
`404`	`396`	`if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {`
`405`	`397`	`int rrln = rcu_preempt_depth();`
`406`	`398`
`407`		`- WARN_ON_ONCE(rrln < 0 && rrln > RCU_NEST_NMAX);`
	`399`	`+ WARN_ON_ONCE(rrln < 0 \|\| rrln > RCU_NEST_PMAX);`
`408`	`400`	`}`
`409`	`401`	`}`
`410`	`402`	`EXPORT_SYMBOL_GPL(__rcu_read_unlock);`
`@@ -556,7 +548,7 @@ static bool rcu_preempt_need_deferred_qs(struct task_struct *t)`
`556`	`548`	`{`
`557`	`549`	`return (__this_cpu_read(rcu_data.exp_deferred_qs) \|\|`
`558`	`550`	`READ_ONCE(t->rcu_read_unlock_special.s)) &&`
`559`		`- rcu_preempt_depth() <= 0;`
	`551`	`+ rcu_preempt_depth() == 0;`
`560`	`552`	`}`
`561`	`553`
`562`	`554`	`/*`
`@@ -692,7 +684,7 @@ static void rcu_flavor_sched_clock_irq(int user)`
`692`	`684`	`} else if (rcu_preempt_need_deferred_qs(t)) {`
`693`	`685`	`rcu_preempt_deferred_qs(t); /* Report deferred QS. */`
`694`	`686`	`return;`
`695`		`- } else if (!rcu_preempt_depth()) {`
	`687`	`+ } else if (!WARN_ON_ONCE(rcu_preempt_depth())) {`
`696`	`688`	`rcu_qs(); /* Report immediate QS. */`
`697`	`689`	`return;`
`698`	`690`	`}`