| Message ID | 20230531101736.12981-2-frederic@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rcu: Support for lazy callbacks on !CONFIG_RCU_NOCB_CPU | expand |
On Wed, May 31, 2023 at 12:17:28PM +0200, Frederic Weisbecker wrote: > rcu_report_dead() is the last RCU word from the CPU down through the > hotplug path. It is called in the idle loop right before the CPU shuts > down for good. Because it removes the CPU from the grace period state > machine and reports an ultimate quiescent state if necessary, no further > use of RCU is allowed. Therefore it is expected that IRQs are disabled > upon calling this function and are not to be re-enabled again until the > CPU shuts down. > > Remove the IRQs disablement from that function and verify instead that > it is actually called with IRQs disabled as it is expected at that > special point in the idle path. > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org> > --- > kernel/rcu/tree.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index fae9b4e29c93..bc4e7c9b51cb 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -4476,11 +4476,16 @@ void rcu_cpu_starting(unsigned int cpu) > */ > void rcu_report_dead(unsigned int cpu) > { > - unsigned long flags, seq_flags; > + unsigned long flags; > unsigned long mask; > struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu); > struct rcu_node *rnp = rdp->mynode; /* Outgoing CPU's rdp & rnp. */ > > + /* > + * IRQS must be disabled from now on and until the CPU dies, or an interrupt > + * may introduce a new READ-side while it is actually off the QS masks. > + */ > + lockdep_assert_irqs_disabled(); > // Do any dangling deferred wakeups. > do_nocb_deferred_wakeup(rdp); > > @@ -4488,7 +4493,6 @@ void rcu_report_dead(unsigned int cpu) > > /* Remove outgoing CPU from mask in the leaf rcu_node structure. */ > mask = rdp->grpmask; > - local_irq_save(seq_flags); True, IRQs should be disabled here. The idle loop disables irqs before calling cpuhp_report_idle_dead() which calls rcu_report_dead(). I was curious about this path called from cpu_die_early() in ARM, in which case it is an existing bug if it did not already disable interrupts. So your lockdep check is a good thing in that regard. For this patch: Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org> thanks, - Joel > arch_spin_lock(&rcu_state.ofl_lock); > raw_spin_lock_irqsave_rcu_node(rnp, flags); /* Enforce GP memory-order guarantee. */ > rdp->rcu_ofl_gp_seq = READ_ONCE(rcu_state.gp_seq); > @@ -4502,8 +4506,6 @@ void rcu_report_dead(unsigned int cpu) > WRITE_ONCE(rnp->qsmaskinitnext, rnp->qsmaskinitnext & ~mask); > raw_spin_unlock_irqrestore_rcu_node(rnp, flags); > arch_spin_unlock(&rcu_state.ofl_lock); > - local_irq_restore(seq_flags); > - > rdp->cpu_started = false; > } > > -- > 2.40.1 >
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index fae9b4e29c93..bc4e7c9b51cb 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -4476,11 +4476,16 @@ void rcu_cpu_starting(unsigned int cpu) */ void rcu_report_dead(unsigned int cpu) { - unsigned long flags, seq_flags; + unsigned long flags; unsigned long mask; struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu); struct rcu_node *rnp = rdp->mynode; /* Outgoing CPU's rdp & rnp. */ + /* + * IRQS must be disabled from now on and until the CPU dies, or an interrupt + * may introduce a new READ-side while it is actually off the QS masks. + */ + lockdep_assert_irqs_disabled(); // Do any dangling deferred wakeups. do_nocb_deferred_wakeup(rdp); @@ -4488,7 +4493,6 @@ void rcu_report_dead(unsigned int cpu) /* Remove outgoing CPU from mask in the leaf rcu_node structure. */ mask = rdp->grpmask; - local_irq_save(seq_flags); arch_spin_lock(&rcu_state.ofl_lock); raw_spin_lock_irqsave_rcu_node(rnp, flags); /* Enforce GP memory-order guarantee. */ rdp->rcu_ofl_gp_seq = READ_ONCE(rcu_state.gp_seq); @@ -4502,8 +4506,6 @@ void rcu_report_dead(unsigned int cpu) WRITE_ONCE(rnp->qsmaskinitnext, rnp->qsmaskinitnext & ~mask); raw_spin_unlock_irqrestore_rcu_node(rnp, flags); arch_spin_unlock(&rcu_state.ofl_lock); - local_irq_restore(seq_flags); - rdp->cpu_started = false; }
rcu_report_dead() is the last RCU word from the CPU down through the hotplug path. It is called in the idle loop right before the CPU shuts down for good. Because it removes the CPU from the grace period state machine and reports an ultimate quiescent state if necessary, no further use of RCU is allowed. Therefore it is expected that IRQs are disabled upon calling this function and are not to be re-enabled again until the CPU shuts down. Remove the IRQs disablement from that function and verify instead that it is actually called with IRQs disabled as it is expected at that special point in the idle path. Signed-off-by: Frederic Weisbecker <frederic@kernel.org> --- kernel/rcu/tree.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)