2023-03-20 17:43:06

by Peter Zijlstra

[permalink] [raw]
Subject: [PATCH] rcu: Remove RCU_NONIDLE()


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) <[email protected]>
---
.../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.


2023-03-20 17:44:58

by Mark Rutland

[permalink] [raw]
Subject: Re: [PATCH] rcu: Remove RCU_NONIDLE()

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) <[email protected]>

Acked-by: Mark Rutland <[email protected]>

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.

2023-03-20 19:04:30

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: Remove RCU_NONIDLE()

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) <[email protected]>
>
> Acked-by: Mark Rutland <[email protected]>

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.

2023-03-24 22:27:53

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH] rcu: Remove RCU_NONIDLE()

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) <[email protected]>

Thanks for all the hard work leading to that!

Acked-by: Frederic Weisbecker <[email protected]>

2023-03-24 22:59:32

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH] rcu: Remove RCU_NONIDLE()

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) <[email protected]>
>
> Thanks for all the hard work leading to that!
>
> Acked-by: Frederic Weisbecker <[email protected]>

What Frederic said! I will apply this ack on my next rebase.

Thanx, Paul