| Message ID | 20230320173751.GU2194297@hirez.programming.kicks-ass.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 27ddaac97a12cee6d9ef2e3e864675228a8d7118 |
| Headers | show |
| Series | rcu: Remove RCU_NONIDLE() | expand |
On Mon, Mar 20, 2023 at 06:37:51PM +0100, Peter Zijlstra wrote: > > Since there are now exactly _zero_ users of RCU_NONIDLE(), make it go > away before someone else decides to (ab)use it. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Mark Rutland <mark.rutland@arm.com> Mark. > --- > .../RCU/Design/Requirements/Requirements.rst | 36 +--------------------- > Documentation/RCU/whatisRCU.rst | 1 - > include/linux/rcupdate.h | 25 --------------- > 3 files changed, 1 insertion(+), 61 deletions(-) > > diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst > index 49387d823619..77155b51d4c2 100644 > --- a/Documentation/RCU/Design/Requirements/Requirements.rst > +++ b/Documentation/RCU/Design/Requirements/Requirements.rst > @@ -2071,41 +2071,7 @@ call. > > Because RCU avoids interrupting idle CPUs, it is illegal to execute an > RCU read-side critical section on an idle CPU. (Kernels built with > -``CONFIG_PROVE_RCU=y`` will splat if you try it.) The RCU_NONIDLE() > -macro and ``_rcuidle`` event tracing is provided to work around this > -restriction. In addition, rcu_is_watching() may be used to test > -whether or not it is currently legal to run RCU read-side critical > -sections on this CPU. I learned of the need for diagnostics on the one > -hand and RCU_NONIDLE() on the other while inspecting idle-loop code. > -Steven Rostedt supplied ``_rcuidle`` event tracing, which is used quite > -heavily in the idle loop. However, there are some restrictions on the > -code placed within RCU_NONIDLE(): > - > -#. Blocking is prohibited. In practice, this is not a serious > - restriction given that idle tasks are prohibited from blocking to > - begin with. > -#. Although nesting RCU_NONIDLE() is permitted, they cannot nest > - indefinitely deeply. However, given that they can be nested on the > - order of a million deep, even on 32-bit systems, this should not be a > - serious restriction. This nesting limit would probably be reached > - long after the compiler OOMed or the stack overflowed. > -#. Any code path that enters RCU_NONIDLE() must sequence out of that > - same RCU_NONIDLE(). For example, the following is grossly > - illegal: > - > - :: > - > - 1 RCU_NONIDLE({ > - 2 do_something(); > - 3 goto bad_idea; /* BUG!!! */ > - 4 do_something_else();}); > - 5 bad_idea: > - > - > - It is just as illegal to transfer control into the middle of > - RCU_NONIDLE()'s argument. Yes, in theory, you could transfer in > - as long as you also transferred out, but in practice you could also > - expect to get sharply worded review comments. > +``CONFIG_PROVE_RCU=y`` will splat if you try it.) > > It is similarly socially unacceptable to interrupt an ``nohz_full`` CPU > running in userspace. RCU must therefore track ``nohz_full`` userspace > diff --git a/Documentation/RCU/whatisRCU.rst b/Documentation/RCU/whatisRCU.rst > index 2c5563a91998..c3b1cbfa1530 100644 > --- a/Documentation/RCU/whatisRCU.rst > +++ b/Documentation/RCU/whatisRCU.rst > @@ -1117,7 +1117,6 @@ in docbook. Here is the list, by category. > > RCU_LOCKDEP_WARN > rcu_sleep_check > - RCU_NONIDLE > > All: Unchecked RCU-protected pointer access:: > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > index 094321c17e48..ddd42efc6224 100644 > --- a/include/linux/rcupdate.h > +++ b/include/linux/rcupdate.h > @@ -156,31 +156,6 @@ static inline int rcu_nocb_cpu_deoffload(int cpu) { return 0; } > static inline void rcu_nocb_flush_deferred_wakeup(void) { } > #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */ > > -/** > - * RCU_NONIDLE - Indicate idle-loop code that needs RCU readers > - * @a: Code that RCU needs to pay attention to. > - * > - * RCU read-side critical sections are forbidden in the inner idle loop, > - * that is, between the ct_idle_enter() and the ct_idle_exit() -- RCU > - * will happily ignore any such read-side critical sections. However, > - * things like powertop need tracepoints in the inner idle loop. > - * > - * This macro provides the way out: RCU_NONIDLE(do_something_with_RCU()) > - * will tell RCU that it needs to pay attention, invoke its argument > - * (in this example, calling the do_something_with_RCU() function), > - * and then tell RCU to go back to ignoring this CPU. It is permissible > - * to nest RCU_NONIDLE() wrappers, but not indefinitely (but the limit is > - * on the order of a million or so, even on 32-bit systems). It is > - * not legal to block within RCU_NONIDLE(), nor is it permissible to > - * transfer control either into or out of RCU_NONIDLE()'s statement. > - */ > -#define RCU_NONIDLE(a) \ > - do { \ > - ct_irq_enter_irqson(); \ > - do { a; } while (0); \ > - ct_irq_exit_irqson(); \ > - } while (0) > - > /* > * Note a quasi-voluntary context switch for RCU-tasks's benefit. > * This is a macro rather than an inline function to avoid #include hell.
On Mon, Mar 20, 2023 at 05:40:01PM +0000, Mark Rutland wrote: > On Mon, Mar 20, 2023 at 06:37:51PM +0100, Peter Zijlstra wrote: > > > > Since there are now exactly _zero_ users of RCU_NONIDLE(), make it go > > away before someone else decides to (ab)use it. > > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Acked-by: Mark Rutland <mark.rutland@arm.com> Thank you both! Queued for v6.5, but it will hit -next much sooner than that. ;-) Thanx, Paul > Mark. > > > --- > > .../RCU/Design/Requirements/Requirements.rst | 36 +--------------------- > > Documentation/RCU/whatisRCU.rst | 1 - > > include/linux/rcupdate.h | 25 --------------- > > 3 files changed, 1 insertion(+), 61 deletions(-) > > > > diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst > > index 49387d823619..77155b51d4c2 100644 > > --- a/Documentation/RCU/Design/Requirements/Requirements.rst > > +++ b/Documentation/RCU/Design/Requirements/Requirements.rst > > @@ -2071,41 +2071,7 @@ call. > > > > Because RCU avoids interrupting idle CPUs, it is illegal to execute an > > RCU read-side critical section on an idle CPU. (Kernels built with > > -``CONFIG_PROVE_RCU=y`` will splat if you try it.) The RCU_NONIDLE() > > -macro and ``_rcuidle`` event tracing is provided to work around this > > -restriction. In addition, rcu_is_watching() may be used to test > > -whether or not it is currently legal to run RCU read-side critical > > -sections on this CPU. I learned of the need for diagnostics on the one > > -hand and RCU_NONIDLE() on the other while inspecting idle-loop code. > > -Steven Rostedt supplied ``_rcuidle`` event tracing, which is used quite > > -heavily in the idle loop. However, there are some restrictions on the > > -code placed within RCU_NONIDLE(): > > - > > -#. Blocking is prohibited. In practice, this is not a serious > > - restriction given that idle tasks are prohibited from blocking to > > - begin with. > > -#. Although nesting RCU_NONIDLE() is permitted, they cannot nest > > - indefinitely deeply. However, given that they can be nested on the > > - order of a million deep, even on 32-bit systems, this should not be a > > - serious restriction. This nesting limit would probably be reached > > - long after the compiler OOMed or the stack overflowed. > > -#. Any code path that enters RCU_NONIDLE() must sequence out of that > > - same RCU_NONIDLE(). For example, the following is grossly > > - illegal: > > - > > - :: > > - > > - 1 RCU_NONIDLE({ > > - 2 do_something(); > > - 3 goto bad_idea; /* BUG!!! */ > > - 4 do_something_else();}); > > - 5 bad_idea: > > - > > - > > - It is just as illegal to transfer control into the middle of > > - RCU_NONIDLE()'s argument. Yes, in theory, you could transfer in > > - as long as you also transferred out, but in practice you could also > > - expect to get sharply worded review comments. > > +``CONFIG_PROVE_RCU=y`` will splat if you try it.) > > > > It is similarly socially unacceptable to interrupt an ``nohz_full`` CPU > > running in userspace. RCU must therefore track ``nohz_full`` userspace > > diff --git a/Documentation/RCU/whatisRCU.rst b/Documentation/RCU/whatisRCU.rst > > index 2c5563a91998..c3b1cbfa1530 100644 > > --- a/Documentation/RCU/whatisRCU.rst > > +++ b/Documentation/RCU/whatisRCU.rst > > @@ -1117,7 +1117,6 @@ in docbook. Here is the list, by category. > > > > RCU_LOCKDEP_WARN > > rcu_sleep_check > > - RCU_NONIDLE > > > > All: Unchecked RCU-protected pointer access:: > > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h > > index 094321c17e48..ddd42efc6224 100644 > > --- a/include/linux/rcupdate.h > > +++ b/include/linux/rcupdate.h > > @@ -156,31 +156,6 @@ static inline int rcu_nocb_cpu_deoffload(int cpu) { return 0; } > > static inline void rcu_nocb_flush_deferred_wakeup(void) { } > > #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */ > > > > -/** > > - * RCU_NONIDLE - Indicate idle-loop code that needs RCU readers > > - * @a: Code that RCU needs to pay attention to. > > - * > > - * RCU read-side critical sections are forbidden in the inner idle loop, > > - * that is, between the ct_idle_enter() and the ct_idle_exit() -- RCU > > - * will happily ignore any such read-side critical sections. However, > > - * things like powertop need tracepoints in the inner idle loop. > > - * > > - * This macro provides the way out: RCU_NONIDLE(do_something_with_RCU()) > > - * will tell RCU that it needs to pay attention, invoke its argument > > - * (in this example, calling the do_something_with_RCU() function), > > - * and then tell RCU to go back to ignoring this CPU. It is permissible > > - * to nest RCU_NONIDLE() wrappers, but not indefinitely (but the limit is > > - * on the order of a million or so, even on 32-bit systems). It is > > - * not legal to block within RCU_NONIDLE(), nor is it permissible to > > - * transfer control either into or out of RCU_NONIDLE()'s statement. > > - */ > > -#define RCU_NONIDLE(a) \ > > - do { \ > > - ct_irq_enter_irqson(); \ > > - do { a; } while (0); \ > > - ct_irq_exit_irqson(); \ > > - } while (0) > > - > > /* > > * Note a quasi-voluntary context switch for RCU-tasks's benefit. > > * This is a macro rather than an inline function to avoid #include hell.
Le Mon, Mar 20, 2023 at 06:37:51PM +0100, Peter Zijlstra a écrit : > > Since there are now exactly _zero_ users of RCU_NONIDLE(), make it go > away before someone else decides to (ab)use it. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Thanks for all the hard work leading to that! Acked-by: Frederic Weisbecker <frederic@kernel.org>
On Fri, Mar 24, 2023 at 11:24:21PM +0100, Frederic Weisbecker wrote: > Le Mon, Mar 20, 2023 at 06:37:51PM +0100, Peter Zijlstra a écrit : > > > > Since there are now exactly _zero_ users of RCU_NONIDLE(), make it go > > away before someone else decides to (ab)use it. > > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> > > Thanks for all the hard work leading to that! > > Acked-by: Frederic Weisbecker <frederic@kernel.org> What Frederic said! I will apply this ack on my next rebase. Thanx, Paul
diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst index 49387d823619..77155b51d4c2 100644 --- a/Documentation/RCU/Design/Requirements/Requirements.rst +++ b/Documentation/RCU/Design/Requirements/Requirements.rst @@ -2071,41 +2071,7 @@ call. Because RCU avoids interrupting idle CPUs, it is illegal to execute an RCU read-side critical section on an idle CPU. (Kernels built with -``CONFIG_PROVE_RCU=y`` will splat if you try it.) The RCU_NONIDLE() -macro and ``_rcuidle`` event tracing is provided to work around this -restriction. In addition, rcu_is_watching() may be used to test -whether or not it is currently legal to run RCU read-side critical -sections on this CPU. I learned of the need for diagnostics on the one -hand and RCU_NONIDLE() on the other while inspecting idle-loop code. -Steven Rostedt supplied ``_rcuidle`` event tracing, which is used quite -heavily in the idle loop. However, there are some restrictions on the -code placed within RCU_NONIDLE(): - -#. Blocking is prohibited. In practice, this is not a serious - restriction given that idle tasks are prohibited from blocking to - begin with. -#. Although nesting RCU_NONIDLE() is permitted, they cannot nest - indefinitely deeply. However, given that they can be nested on the - order of a million deep, even on 32-bit systems, this should not be a - serious restriction. This nesting limit would probably be reached - long after the compiler OOMed or the stack overflowed. -#. Any code path that enters RCU_NONIDLE() must sequence out of that - same RCU_NONIDLE(). For example, the following is grossly - illegal: - - :: - - 1 RCU_NONIDLE({ - 2 do_something(); - 3 goto bad_idea; /* BUG!!! */ - 4 do_something_else();}); - 5 bad_idea: - - - It is just as illegal to transfer control into the middle of - RCU_NONIDLE()'s argument. Yes, in theory, you could transfer in - as long as you also transferred out, but in practice you could also - expect to get sharply worded review comments. +``CONFIG_PROVE_RCU=y`` will splat if you try it.) It is similarly socially unacceptable to interrupt an ``nohz_full`` CPU running in userspace. RCU must therefore track ``nohz_full`` userspace diff --git a/Documentation/RCU/whatisRCU.rst b/Documentation/RCU/whatisRCU.rst index 2c5563a91998..c3b1cbfa1530 100644 --- a/Documentation/RCU/whatisRCU.rst +++ b/Documentation/RCU/whatisRCU.rst @@ -1117,7 +1117,6 @@ in docbook. Here is the list, by category. RCU_LOCKDEP_WARN rcu_sleep_check - RCU_NONIDLE All: Unchecked RCU-protected pointer access:: diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h index 094321c17e48..ddd42efc6224 100644 --- a/include/linux/rcupdate.h +++ b/include/linux/rcupdate.h @@ -156,31 +156,6 @@ static inline int rcu_nocb_cpu_deoffload(int cpu) { return 0; } static inline void rcu_nocb_flush_deferred_wakeup(void) { } #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */ -/** - * RCU_NONIDLE - Indicate idle-loop code that needs RCU readers - * @a: Code that RCU needs to pay attention to. - * - * RCU read-side critical sections are forbidden in the inner idle loop, - * that is, between the ct_idle_enter() and the ct_idle_exit() -- RCU - * will happily ignore any such read-side critical sections. However, - * things like powertop need tracepoints in the inner idle loop. - * - * This macro provides the way out: RCU_NONIDLE(do_something_with_RCU()) - * will tell RCU that it needs to pay attention, invoke its argument - * (in this example, calling the do_something_with_RCU() function), - * and then tell RCU to go back to ignoring this CPU. It is permissible - * to nest RCU_NONIDLE() wrappers, but not indefinitely (but the limit is - * on the order of a million or so, even on 32-bit systems). It is - * not legal to block within RCU_NONIDLE(), nor is it permissible to - * transfer control either into or out of RCU_NONIDLE()'s statement. - */ -#define RCU_NONIDLE(a) \ - do { \ - ct_irq_enter_irqson(); \ - do { a; } while (0); \ - ct_irq_exit_irqson(); \ - } while (0) - /* * Note a quasi-voluntary context switch for RCU-tasks's benefit. * This is a macro rather than an inline function to avoid #include hell.
Since there are now exactly _zero_ users of RCU_NONIDLE(), make it go away before someone else decides to (ab)use it. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- .../RCU/Design/Requirements/Requirements.rst | 36 +--------------------- Documentation/RCU/whatisRCU.rst | 1 - include/linux/rcupdate.h | 25 --------------- 3 files changed, 1 insertion(+), 61 deletions(-)