Skip to content

Commit 5f5fa7e

Browse files
Lai Jiangshanpaulmckrcu
authored andcommitted
rcu: Don't use negative nesting depth in __rcu_read_unlock()
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 <[email protected]> Signed-off-by: Paul E. McKenney <[email protected]>
1 parent f0bdf6d commit 5f5fa7e

File tree

2 files changed

+12
-41
lines changed

2 files changed

+12
-41
lines changed

kernel/rcu/tree_exp.h

Lines changed: 5 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -639,6 +639,7 @@ static void wait_rcu_exp_gp(struct work_struct *wp)
639639
*/
640640
static void rcu_exp_handler(void *unused)
641641
{
642+
int depth = rcu_preempt_depth();
642643
unsigned long flags;
643644
struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
644645
struct rcu_node *rnp = rdp->mynode;
@@ -649,7 +650,7 @@ static void rcu_exp_handler(void *unused)
649650
* critical section. If also enabled or idle, immediately
650651
* report the quiescent state, otherwise defer.
651652
*/
652-
if (!rcu_preempt_depth()) {
653+
if (!depth) {
653654
if (!(preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK)) ||
654655
rcu_dynticks_curr_cpu_in_eqs()) {
655656
rcu_report_exp_rdp(rdp);
@@ -673,7 +674,7 @@ static void rcu_exp_handler(void *unused)
673674
* can have caused this quiescent state to already have been
674675
* reported, so we really do need to check ->expmask.
675676
*/
676-
if (rcu_preempt_depth() > 0) {
677+
if (depth > 0) {
677678
raw_spin_lock_irqsave_rcu_node(rnp, flags);
678679
if (rnp->expmask & rdp->grpmask) {
679680
rdp->exp_deferred_qs = true;
@@ -683,30 +684,8 @@ static void rcu_exp_handler(void *unused)
683684
return;
684685
}
685686

686-
/*
687-
* The final and least likely case is where the interrupted
688-
* code was just about to or just finished exiting the RCU-preempt
689-
* read-side critical section, and no, we can't tell which.
690-
* So either way, set ->deferred_qs to flag later code that
691-
* a quiescent state is required.
692-
*
693-
* If the CPU is fully enabled (or if some buggy RCU-preempt
694-
* read-side critical section is being used from idle), just
695-
* invoke rcu_preempt_deferred_qs() to immediately report the
696-
* quiescent state. We cannot use rcu_read_unlock_special()
697-
* because we are in an interrupt handler, which will cause that
698-
* function to take an early exit without doing anything.
699-
*
700-
* Otherwise, force a context switch after the CPU enables everything.
701-
*/
702-
rdp->exp_deferred_qs = true;
703-
if (!(preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK)) ||
704-
WARN_ON_ONCE(rcu_dynticks_curr_cpu_in_eqs())) {
705-
rcu_preempt_deferred_qs(t);
706-
} else {
707-
set_tsk_need_resched(t);
708-
set_preempt_need_resched();
709-
}
687+
// Finally, negative nesting depth should not happen.
688+
WARN_ON_ONCE(1);
710689
}
711690

712691
/* PREEMPTION=y, so no PREEMPTION=n expedited grace period to clean up after. */

kernel/rcu/tree_plugin.h

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -345,19 +345,17 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node *rnp)
345345
return READ_ONCE(rnp->gp_tasks) != NULL;
346346
}
347347

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. */
351349
#define RCU_NEST_PMAX (INT_MAX / 2)
352350

353351
static void rcu_preempt_read_enter(void)
354352
{
355353
current->rcu_read_lock_nesting++;
356354
}
357355

358-
static void rcu_preempt_read_exit(void)
356+
static int rcu_preempt_read_exit(void)
359357
{
360-
current->rcu_read_lock_nesting--;
358+
return --current->rcu_read_lock_nesting;
361359
}
362360

363361
static void rcu_preempt_depth_set(int val)
@@ -390,21 +388,15 @@ void __rcu_read_unlock(void)
390388
{
391389
struct task_struct *t = current;
392390

393-
if (rcu_preempt_depth() != 1) {
394-
rcu_preempt_read_exit();
395-
} else {
391+
if (rcu_preempt_read_exit() == 0) {
396392
barrier(); /* critical section before exit code. */
397-
rcu_preempt_depth_set(-RCU_NEST_BIAS);
398-
barrier(); /* assign before ->rcu_read_unlock_special load */
399393
if (unlikely(READ_ONCE(t->rcu_read_unlock_special.s)))
400394
rcu_read_unlock_special(t);
401-
barrier(); /* ->rcu_read_unlock_special load before assign */
402-
rcu_preempt_depth_set(0);
403395
}
404396
if (IS_ENABLED(CONFIG_PROVE_LOCKING)) {
405397
int rrln = rcu_preempt_depth();
406398

407-
WARN_ON_ONCE(rrln < 0 && rrln > RCU_NEST_NMAX);
399+
WARN_ON_ONCE(rrln < 0 || rrln > RCU_NEST_PMAX);
408400
}
409401
}
410402
EXPORT_SYMBOL_GPL(__rcu_read_unlock);
@@ -556,7 +548,7 @@ static bool rcu_preempt_need_deferred_qs(struct task_struct *t)
556548
{
557549
return (__this_cpu_read(rcu_data.exp_deferred_qs) ||
558550
READ_ONCE(t->rcu_read_unlock_special.s)) &&
559-
rcu_preempt_depth() <= 0;
551+
rcu_preempt_depth() == 0;
560552
}
561553

562554
/*
@@ -692,7 +684,7 @@ static void rcu_flavor_sched_clock_irq(int user)
692684
} else if (rcu_preempt_need_deferred_qs(t)) {
693685
rcu_preempt_deferred_qs(t); /* Report deferred QS. */
694686
return;
695-
} else if (!rcu_preempt_depth()) {
687+
} else if (!WARN_ON_ONCE(rcu_preempt_depth())) {
696688
rcu_qs(); /* Report immediate QS. */
697689
return;
698690
}

0 commit comments

Comments
 (0)