| Message ID | 20220608144516.935970247@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpuidle,rcu: Cleanup the mess | expand |
On Wed, Jun 08, 2022 at 04:27:39PM +0200, Peter Zijlstra wrote: > Current rcu_idle_exit() is terminally broken because it uses > local_irq_{save,restore}(), which are traced which uses RCU. > > However, now that all the callers are sure to have IRQs disabled, we > can remove these calls. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Acked-by: Paul E. McKenney <paulmck@kernel.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > kernel/rcu/tree.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -659,7 +659,7 @@ static noinstr void rcu_eqs_enter(bool u > * If you add or remove a call to rcu_idle_enter(), be sure to test with > * CONFIG_RCU_EQS_DEBUG=y. > */ > -void rcu_idle_enter(void) > +void noinstr rcu_idle_enter(void) > { > lockdep_assert_irqs_disabled(); > rcu_eqs_enter(false); > @@ -896,13 +896,10 @@ static void noinstr rcu_eqs_exit(bool us > * If you add or remove a call to rcu_idle_exit(), be sure to test with > * CONFIG_RCU_EQS_DEBUG=y. > */ > -void rcu_idle_exit(void) > +void noinstr rcu_idle_exit(void) > { > - unsigned long flags; > - > - local_irq_save(flags); > + lockdep_assert_irqs_disabled(); > rcu_eqs_exit(false); > - local_irq_restore(flags); > } > EXPORT_SYMBOL_GPL(rcu_idle_exit); > > >
On Wed, Jun 08, 2022 at 04:27:39PM +0200, Peter Zijlstra wrote: > Current rcu_idle_exit() is terminally broken because it uses > local_irq_{save,restore}(), which are traced which uses RCU. > > However, now that all the callers are sure to have IRQs disabled, we > can remove these calls. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > Acked-by: Paul E. McKenney <paulmck@kernel.org> We have some fun conflicts between this series and Frederic's context-tracking series. But it looks like these can be resolved by: 1. A patch on top of Frederic's series that provides the old rcu_*() names for the functions now prefixed with ct_*() such as ct_idle_exit(). 2. Another patch on top of Frederic's series that takes the changes remaining from this patch, shown below. Frederic's series uses raw_local_irq_save() and raw_local_irq_restore(), which can then be removed. Or is there a better way to do this? Thanx, Paul ------------------------------------------------------------------------ commit f64cee8c159e9863a74594efe3d33fb513a6a7b5 Author: Peter Zijlstra <peterz@infradead.org> Date: Tue Jun 14 17:24:43 2022 -0700 context_tracking: Interrupts always disabled for ct_idle_exit() Now that the idle-loop cleanups have ensured that rcu_idle_exit() is always invoked with interrupts disabled, remove the interrupt disabling in favor of a debug check. Signed-off-by: Peter Zijlstra <peterz@infradead.org> Cc: Frederic Weisbecker <frederic@kernel.org> Signed-off-by: Paul E. McKenney <paulmck@kernel.org> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c index 1da44803fd319..99310cf5b0254 100644 --- a/kernel/context_tracking.c +++ b/kernel/context_tracking.c @@ -332,11 +332,8 @@ EXPORT_SYMBOL_GPL(ct_idle_enter); */ void noinstr ct_idle_exit(void) { - unsigned long flags; - - raw_local_irq_save(flags); + WARN_ON_ONCE(IS_ENABLED(CONFIG_RCU_EQS_DEBUG) && !raw_irqs_disabled()); ct_kernel_enter(false, RCU_DYNTICKS_IDX - CONTEXT_IDLE); - raw_local_irq_restore(flags); } EXPORT_SYMBOL_GPL(ct_idle_exit);
--- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -659,7 +659,7 @@ static noinstr void rcu_eqs_enter(bool u * If you add or remove a call to rcu_idle_enter(), be sure to test with * CONFIG_RCU_EQS_DEBUG=y. */ -void rcu_idle_enter(void) +void noinstr rcu_idle_enter(void) { lockdep_assert_irqs_disabled(); rcu_eqs_enter(false); @@ -896,13 +896,10 @@ static void noinstr rcu_eqs_exit(bool us * If you add or remove a call to rcu_idle_exit(), be sure to test with * CONFIG_RCU_EQS_DEBUG=y. */ -void rcu_idle_exit(void) +void noinstr rcu_idle_exit(void) { - unsigned long flags; - - local_irq_save(flags); + lockdep_assert_irqs_disabled(); rcu_eqs_exit(false); - local_irq_restore(flags); } EXPORT_SYMBOL_GPL(rcu_idle_exit);
Current rcu_idle_exit() is terminally broken because it uses local_irq_{save,restore}(), which are traced which uses RCU. However, now that all the callers are sure to have IRQs disabled, we can remove these calls. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Paul E. McKenney <paulmck@kernel.org> --- kernel/rcu/tree.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)