[tip/core/rcu,04/15] rcu: Permit RCU_NONIDLE() to be used from interrupt context

From: "Paul E. McKenney" <paul.mckenney@linaro.org>

There is a need to use RCU from interrupt context, but either before
rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
interrupt occurs from idle, then lockdep-RCU will complain about such
uses, as they appear to be illegal uses of RCU from the idle loop.
In other environments, RCU_NONIDLE() could be used to properly protect
the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
from process context.

This commit therefore modifies RCU_NONIDLE() to permit its use more
globally.

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 include/linux/rcupdate.h |    6 ++----
 kernel/rcutiny.c         |    2 ++
 kernel/rcutree.c         |    2 ++
 3 files changed, 6 insertions(+), 4 deletions(-)

On Thu, Aug 30, 2012 at 11:56:17AM -0700, Paul E. McKenney wrote:
> From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> 
> There is a need to use RCU from interrupt context, but either before
> rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
> interrupt occurs from idle, then lockdep-RCU will complain about such
> uses, as they appear to be illegal uses of RCU from the idle loop.
> In other environments, RCU_NONIDLE() could be used to properly protect
> the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
> from process context.
> 
> This commit therefore modifies RCU_NONIDLE() to permit its use more
> globally.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Something seems wrong about this.  The addition of EXPORT_SYMBOL_GPL
suggests that such interrupt handlers might live in modules.  In what
situation might a module interrupt handler get called from the idle
loop, before rcu_irq_enter or after rcu_irq_exit, and need to know that
when using RCU?

- Josh Triplett

> ---
>  include/linux/rcupdate.h |    6 ++----
>  kernel/rcutiny.c         |    2 ++
>  kernel/rcutree.c         |    2 ++
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 115ead2..0fbbd52 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -210,14 +210,12 @@ extern void exit_rcu(void);
>   * to nest RCU_NONIDLE() wrappers, but the nesting level is currently
>   * quite limited.  If deeper nesting is required, it will be necessary
>   * to adjust DYNTICK_TASK_NESTING_VALUE accordingly.
> - *
> - * This macro may be used from process-level code only.
>   */
>  #define RCU_NONIDLE(a) \
>  	do { \
> -		rcu_idle_exit(); \
> +		rcu_irq_enter(); \
>  		do { a; } while (0); \
> -		rcu_idle_enter(); \
> +		rcu_irq_exit(); \
>  	} while (0)
>  
>  /*
> diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> index e4163c5..2e073a2 100644
> --- a/kernel/rcutiny.c
> +++ b/kernel/rcutiny.c
> @@ -115,6 +115,7 @@ void rcu_irq_exit(void)
>  	rcu_idle_enter_common(newval);
>  	local_irq_restore(flags);
>  }
> +EXPORT_SYMBOL_GPL(rcu_irq_exit);
>  
>  /* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcutree.c. */
>  static void rcu_idle_exit_common(long long oldval)
> @@ -172,6 +173,7 @@ void rcu_irq_enter(void)
>  	rcu_idle_exit_common(oldval);
>  	local_irq_restore(flags);
>  }
> +EXPORT_SYMBOL_GPL(rcu_irq_enter);
>  
>  #ifdef CONFIG_DEBUG_LOCK_ALLOC
>  
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index f280e54..96b8aff 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -447,6 +447,7 @@ void rcu_irq_exit(void)
>  		rcu_idle_enter_common(rdtp, oldval);
>  	local_irq_restore(flags);
>  }
> +EXPORT_SYMBOL_GPL(rcu_irq_exit);
>  
>  /*
>   * rcu_idle_exit_common - inform RCU that current CPU is moving away from idle
> @@ -542,6 +543,7 @@ void rcu_irq_enter(void)
>  		rcu_idle_exit_common(rdtp, oldval);
>  	local_irq_restore(flags);
>  }
> +EXPORT_SYMBOL_GPL(rcu_irq_enter);
>  
>  /**
>   * rcu_nmi_enter - inform RCU of entry to NMI context
> -- 
> 1.7.8
>

On Fri, Aug 31, 2012 at 11:00:52AM -0700, Josh Triplett wrote:
> On Thu, Aug 30, 2012 at 11:56:17AM -0700, Paul E. McKenney wrote:
> > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > 
> > There is a need to use RCU from interrupt context, but either before
> > rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
> > interrupt occurs from idle, then lockdep-RCU will complain about such
> > uses, as they appear to be illegal uses of RCU from the idle loop.
> > In other environments, RCU_NONIDLE() could be used to properly protect
> > the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
> > from process context.
> > 
> > This commit therefore modifies RCU_NONIDLE() to permit its use more
> > globally.
> > 
> > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
> > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Something seems wrong about this.  The addition of EXPORT_SYMBOL_GPL
> suggests that such interrupt handlers might live in modules.  In what
> situation might a module interrupt handler get called from the idle
> loop, before rcu_irq_enter or after rcu_irq_exit, and need to know that
> when using RCU?

Drivers can be in modules, in which case their interrupt handlers will
also be in the corresponding module.  I do agree that in most cases,
the irq_enter() and irq_exit() hooks would be invoked by non-module code,
but I do believe that I had to add those exports due to build failures.

Steven will let me know if I am confused on this point.

							Thanx, Paul

> - Josh Triplett
> 
> > ---
> >  include/linux/rcupdate.h |    6 ++----
> >  kernel/rcutiny.c         |    2 ++
> >  kernel/rcutree.c         |    2 ++
> >  3 files changed, 6 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 115ead2..0fbbd52 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -210,14 +210,12 @@ extern void exit_rcu(void);
> >   * to nest RCU_NONIDLE() wrappers, but the nesting level is currently
> >   * quite limited.  If deeper nesting is required, it will be necessary
> >   * to adjust DYNTICK_TASK_NESTING_VALUE accordingly.
> > - *
> > - * This macro may be used from process-level code only.
> >   */
> >  #define RCU_NONIDLE(a) \
> >  	do { \
> > -		rcu_idle_exit(); \
> > +		rcu_irq_enter(); \
> >  		do { a; } while (0); \
> > -		rcu_idle_enter(); \
> > +		rcu_irq_exit(); \
> >  	} while (0)
> >  
> >  /*
> > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> > index e4163c5..2e073a2 100644
> > --- a/kernel/rcutiny.c
> > +++ b/kernel/rcutiny.c
> > @@ -115,6 +115,7 @@ void rcu_irq_exit(void)
> >  	rcu_idle_enter_common(newval);
> >  	local_irq_restore(flags);
> >  }
> > +EXPORT_SYMBOL_GPL(rcu_irq_exit);
> >  
> >  /* Common code for rcu_idle_exit() and rcu_irq_enter(), see kernel/rcutree.c. */
> >  static void rcu_idle_exit_common(long long oldval)
> > @@ -172,6 +173,7 @@ void rcu_irq_enter(void)
> >  	rcu_idle_exit_common(oldval);
> >  	local_irq_restore(flags);
> >  }
> > +EXPORT_SYMBOL_GPL(rcu_irq_enter);
> >  
> >  #ifdef CONFIG_DEBUG_LOCK_ALLOC
> >  
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index f280e54..96b8aff 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -447,6 +447,7 @@ void rcu_irq_exit(void)
> >  		rcu_idle_enter_common(rdtp, oldval);
> >  	local_irq_restore(flags);
> >  }
> > +EXPORT_SYMBOL_GPL(rcu_irq_exit);
> >  
> >  /*
> >   * rcu_idle_exit_common - inform RCU that current CPU is moving away from idle
> > @@ -542,6 +543,7 @@ void rcu_irq_enter(void)
> >  		rcu_idle_exit_common(rdtp, oldval);
> >  	local_irq_restore(flags);
> >  }
> > +EXPORT_SYMBOL_GPL(rcu_irq_enter);
> >  
> >  /**
> >   * rcu_nmi_enter - inform RCU of entry to NMI context
> > -- 
> > 1.7.8
> > 
>

On Tue, Sep 04, 2012 at 03:33:50PM -0700, Paul E. McKenney wrote:
> On Fri, Aug 31, 2012 at 11:00:52AM -0700, Josh Triplett wrote:
> > On Thu, Aug 30, 2012 at 11:56:17AM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > > 
> > > There is a need to use RCU from interrupt context, but either before
> > > rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
> > > interrupt occurs from idle, then lockdep-RCU will complain about such
> > > uses, as they appear to be illegal uses of RCU from the idle loop.
> > > In other environments, RCU_NONIDLE() could be used to properly protect
> > > the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
> > > from process context.
> > > 
> > > This commit therefore modifies RCU_NONIDLE() to permit its use more
> > > globally.
> > > 
> > > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > > Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Something seems wrong about this.  The addition of EXPORT_SYMBOL_GPL
> > suggests that such interrupt handlers might live in modules.  In what
> > situation might a module interrupt handler get called from the idle
> > loop, before rcu_irq_enter or after rcu_irq_exit, and need to know that
> > when using RCU?
> 
> Drivers can be in modules, in which case their interrupt handlers will
> also be in the corresponding module.  I do agree that in most cases,
> the irq_enter() and irq_exit() hooks would be invoked by non-module code,
> but I do believe that I had to add those exports due to build failures.

I definitely understand that drivers can have interrupt handlers in
modules; however, I have a hard time seeing a case where a module has a
legitimate reason to invoke rcu_irq_enter or rcu_irq_exit, or to run
before the former or after the latter.

- Josh Triplett

On Tue, 2012-09-04 at 15:33 -0700, Paul E. McKenney wrote:
> On Fri, Aug 31, 2012 at 11:00:52AM -0700, Josh Triplett wrote:
> > On Thu, Aug 30, 2012 at 11:56:17AM -0700, Paul E. McKenney wrote:
> > > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > > 
> > > There is a need to use RCU from interrupt context, but either before
> > > rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
> > > interrupt occurs from idle, then lockdep-RCU will complain about such
> > > uses, as they appear to be illegal uses of RCU from the idle loop.
> > > In other environments, RCU_NONIDLE() could be used to properly protect
> > > the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
> > > from process context.
> > > 
> > > This commit therefore modifies RCU_NONIDLE() to permit its use more
> > > globally.
> > > 
> > > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > > Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
> > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > 
> > Something seems wrong about this.  The addition of EXPORT_SYMBOL_GPL
> > suggests that such interrupt handlers might live in modules.  In what
> > situation might a module interrupt handler get called from the idle
> > loop, before rcu_irq_enter or after rcu_irq_exit, and need to know that
> > when using RCU?
> 
> Drivers can be in modules, in which case their interrupt handlers will
> also be in the corresponding module.  I do agree that in most cases,
> the irq_enter() and irq_exit() hooks would be invoked by non-module code,
> but I do believe that I had to add those exports due to build failures.
> 
> Steven will let me know if I am confused on this point.
> 

You're not confused, the situation is confusing :-/

Because some trace events happen inside the idle loop after rcu has
"shutdown", we needed to create "trace_foo_rcuidle()" handlers that can
handle this condition. That is, for every trace_foo() static inline
(used at the tracepoint location), there exists a static inline
trace_foo_rcuidle(), that looks something like this:

static inline void trace_##name##_rcuidle(proto) {
	if (static_key_false(&__tracepoint_##name.key)) { 
		rcu_idle_exit();
		__DO_TRACE();
		rcu_idle_enter();
	}
}

Although these calls are never used by module code, because they are
static inlines, they are still defined for all tracepoints, kernel
tracepoints as well as module tracepoints. And thus, need the export :-(

-- Steve

On Tue, Sep 04, 2012 at 06:51:22PM -0400, Steven Rostedt wrote:
> On Tue, 2012-09-04 at 15:33 -0700, Paul E. McKenney wrote:
> > On Fri, Aug 31, 2012 at 11:00:52AM -0700, Josh Triplett wrote:
> > > On Thu, Aug 30, 2012 at 11:56:17AM -0700, Paul E. McKenney wrote:
> > > > From: "Paul E. McKenney" <paul.mckenney@linaro.org>
> > > > 
> > > > There is a need to use RCU from interrupt context, but either before
> > > > rcu_irq_enter() is called or after rcu_irq_exit() is called.  If the
> > > > interrupt occurs from idle, then lockdep-RCU will complain about such
> > > > uses, as they appear to be illegal uses of RCU from the idle loop.
> > > > In other environments, RCU_NONIDLE() could be used to properly protect
> > > > the use of RCU, but RCU_NONIDLE() currently cannot be invoked except
> > > > from process context.
> > > > 
> > > > This commit therefore modifies RCU_NONIDLE() to permit its use more
> > > > globally.
> > > > 
> > > > Reported-by: Steven Rostedt <rostedt@goodmis.org>
> > > > Signed-off-by: Paul E. McKenney <paul.mckenney@linaro.org>
> > > > Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> > > 
> > > Something seems wrong about this.  The addition of EXPORT_SYMBOL_GPL
> > > suggests that such interrupt handlers might live in modules.  In what
> > > situation might a module interrupt handler get called from the idle
> > > loop, before rcu_irq_enter or after rcu_irq_exit, and need to know that
> > > when using RCU?
> > 
> > Drivers can be in modules, in which case their interrupt handlers will
> > also be in the corresponding module.  I do agree that in most cases,
> > the irq_enter() and irq_exit() hooks would be invoked by non-module code,
> > but I do believe that I had to add those exports due to build failures.
> > 
> > Steven will let me know if I am confused on this point.
> > 
> 
> You're not confused, the situation is confusing :-/
> 
> Because some trace events happen inside the idle loop after rcu has
> "shutdown", we needed to create "trace_foo_rcuidle()" handlers that can
> handle this condition. That is, for every trace_foo() static inline
> (used at the tracepoint location), there exists a static inline
> trace_foo_rcuidle(), that looks something like this:
> 
> static inline void trace_##name##_rcuidle(proto) {
> 	if (static_key_false(&__tracepoint_##name.key)) { 
> 		rcu_idle_exit();
> 		__DO_TRACE();
> 		rcu_idle_enter();
> 	}
> }
> 
> Although these calls are never used by module code, because they are
> static inlines, they are still defined for all tracepoints, kernel
> tracepoints as well as module tracepoints. And thus, need the export :-(

Very good -- I will add this to the commit log, with attribution.

							Thanx, Paul