diff mbox

trace: Don't declare trace_*_rcuidle functions in modules

Message ID 20120905062306.GA14756@leaf
State Accepted
Commit 7ece55a4a3a04abe37118b1d4fb0b702eeb1de4c
Headers show

Commit Message

Josh Triplett Sept. 5, 2012, 6:23 a.m. UTC
Tracepoints declare a static inline trace_*_rcuidle variant of the trace
function, to support safely generating trace events from the idle loop.
Module code never actually uses that variant of trace functions, because
modules don't run code that needs tracing with RCU idled.  However, the
declaration of those otherwise unused functions causes the module to
reference rcu_idle_exit and rcu_idle_enter, which RCU does not export to
modules.

To avoid this, don't generate trace_*_rcuidle functions for tracepoints
declared in module code.

Reported-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
On Tue, Sep 04, 2012 at 07:46:42PM -0400, Steven Rostedt wrote:
> We could add either. Probably keep the ugliness of tracepoints inside
> the tracepoint code than to start spreading it around to rcu. RCU has
> its own ugliness features ;-)
> 
> Hence, I would be OK if you send me a patch that does what you proposed
> and removes the EXPORT from RCU.

 include/linux/tracepoint.h |   28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

Comments

Mathieu Desnoyers Sept. 5, 2012, 2:26 p.m. UTC | #1
* Josh Triplett (josh@joshtriplett.org) wrote:
> Tracepoints declare a static inline trace_*_rcuidle variant of the trace
> function, to support safely generating trace events from the idle loop.
> Module code never actually uses that variant of trace functions, because
> modules don't run code that needs tracing with RCU idled.  However, the
> declaration of those otherwise unused functions causes the module to
> reference rcu_idle_exit and rcu_idle_enter, which RCU does not export to
> modules.
> 
> To avoid this, don't generate trace_*_rcuidle functions for tracepoints
> declared in module code.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

Thanks Josh!

Mathieu

> ---
> On Tue, Sep 04, 2012 at 07:46:42PM -0400, Steven Rostedt wrote:
> > We could add either. Probably keep the ugliness of tracepoints inside
> > the tracepoint code than to start spreading it around to rcu. RCU has
> > its own ugliness features ;-)
> > 
> > Hence, I would be OK if you send me a patch that does what you proposed
> > and removes the EXPORT from RCU.
> 
>  include/linux/tracepoint.h |   28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 802de56..2f322c3 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -136,6 +136,22 @@ static inline void tracepoint_synchronize_unregister(void)
>  		postrcu;						\
>  	} while (0)
>  
> +#ifndef MODULE
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)	\
> +	static inline void trace_##name##_rcuidle(proto)		\
> +	{								\
> +		if (static_key_false(&__tracepoint_##name.key))		\
> +			__DO_TRACE(&__tracepoint_##name,		\
> +				TP_PROTO(data_proto),			\
> +				TP_ARGS(data_args),			\
> +				TP_CONDITION(cond),			\
> +				rcu_idle_exit(),			\
> +				rcu_idle_enter());			\
> +	}
> +#else
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
> +#endif
> +
>  /*
>   * Make sure the alignment of the structure in the __tracepoints section will
>   * not add unwanted padding between the beginning of the section and the
> @@ -151,16 +167,8 @@ static inline void tracepoint_synchronize_unregister(void)
>  				TP_ARGS(data_args),			\
>  				TP_CONDITION(cond),,);			\
>  	}								\
> -	static inline void trace_##name##_rcuidle(proto)		\
> -	{								\
> -		if (static_key_false(&__tracepoint_##name.key))		\
> -			__DO_TRACE(&__tracepoint_##name,		\
> -				TP_PROTO(data_proto),			\
> -				TP_ARGS(data_args),			\
> -				TP_CONDITION(cond),			\
> -				rcu_idle_exit(),			\
> -				rcu_idle_enter());			\
> -	}								\
> +	__DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args),		\
> +		PARAMS(cond), PARAMS(data_proto), PARAMS(data_args))	\
>  	static inline int						\
>  	register_trace_##name(void (*probe)(data_proto), void *data)	\
>  	{								\
> -- 
> 1.7.10.4
>
Paul E. McKenney Sept. 5, 2012, 4:36 p.m. UTC | #2
On Tue, Sep 04, 2012 at 11:23:06PM -0700, Josh Triplett wrote:
> Tracepoints declare a static inline trace_*_rcuidle variant of the trace
> function, to support safely generating trace events from the idle loop.
> Module code never actually uses that variant of trace functions, because
> modules don't run code that needs tracing with RCU idled.  However, the
> declaration of those otherwise unused functions causes the module to
> reference rcu_idle_exit and rcu_idle_enter, which RCU does not export to
> modules.
> 
> To avoid this, don't generate trace_*_rcuidle functions for tracepoints
> declared in module code.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Looks good to me, please let me know when I should remove the exports.
Sending the relevant patch is sufficient notification.  ;-)

Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

							Thanx, Paul

> ---
> On Tue, Sep 04, 2012 at 07:46:42PM -0400, Steven Rostedt wrote:
> > We could add either. Probably keep the ugliness of tracepoints inside
> > the tracepoint code than to start spreading it around to rcu. RCU has
> > its own ugliness features ;-)
> > 
> > Hence, I would be OK if you send me a patch that does what you proposed
> > and removes the EXPORT from RCU.
> 
>  include/linux/tracepoint.h |   28 ++++++++++++++++++----------
>  1 file changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index 802de56..2f322c3 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -136,6 +136,22 @@ static inline void tracepoint_synchronize_unregister(void)
>  		postrcu;						\
>  	} while (0)
> 
> +#ifndef MODULE
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)	\
> +	static inline void trace_##name##_rcuidle(proto)		\
> +	{								\
> +		if (static_key_false(&__tracepoint_##name.key))		\
> +			__DO_TRACE(&__tracepoint_##name,		\
> +				TP_PROTO(data_proto),			\
> +				TP_ARGS(data_args),			\
> +				TP_CONDITION(cond),			\
> +				rcu_idle_exit(),			\
> +				rcu_idle_enter());			\
> +	}
> +#else
> +#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
> +#endif
> +
>  /*
>   * Make sure the alignment of the structure in the __tracepoints section will
>   * not add unwanted padding between the beginning of the section and the
> @@ -151,16 +167,8 @@ static inline void tracepoint_synchronize_unregister(void)
>  				TP_ARGS(data_args),			\
>  				TP_CONDITION(cond),,);			\
>  	}								\
> -	static inline void trace_##name##_rcuidle(proto)		\
> -	{								\
> -		if (static_key_false(&__tracepoint_##name.key))		\
> -			__DO_TRACE(&__tracepoint_##name,		\
> -				TP_PROTO(data_proto),			\
> -				TP_ARGS(data_args),			\
> -				TP_CONDITION(cond),			\
> -				rcu_idle_exit(),			\
> -				rcu_idle_enter());			\
> -	}								\
> +	__DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args),		\
> +		PARAMS(cond), PARAMS(data_proto), PARAMS(data_args))	\
>  	static inline int						\
>  	register_trace_##name(void (*probe)(data_proto), void *data)	\
>  	{								\
> -- 
> 1.7.10.4
>
Steven Rostedt Sept. 6, 2012, 7:49 p.m. UTC | #3
On Tue, 2012-09-04 at 23:23 -0700, Josh Triplett wrote:
> Tracepoints declare a static inline trace_*_rcuidle variant of the trace
> function, to support safely generating trace events from the idle loop.
> Module code never actually uses that variant of trace functions, because
> modules don't run code that needs tracing with RCU idled.  However, the
> declaration of those otherwise unused functions causes the module to
> reference rcu_idle_exit and rcu_idle_enter, which RCU does not export to
> modules.
> 
> To avoid this, don't generate trace_*_rcuidle functions for tracepoints
> declared in module code.
> 
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Josh Triplett <josh@joshtriplett.org>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

-- Steve
diff mbox

Patch

diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index 802de56..2f322c3 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -136,6 +136,22 @@  static inline void tracepoint_synchronize_unregister(void)
 		postrcu;						\
 	} while (0)
 
+#ifndef MODULE
+#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)	\
+	static inline void trace_##name##_rcuidle(proto)		\
+	{								\
+		if (static_key_false(&__tracepoint_##name.key))		\
+			__DO_TRACE(&__tracepoint_##name,		\
+				TP_PROTO(data_proto),			\
+				TP_ARGS(data_args),			\
+				TP_CONDITION(cond),			\
+				rcu_idle_exit(),			\
+				rcu_idle_enter());			\
+	}
+#else
+#define __DECLARE_TRACE_RCU(name, proto, args, cond, data_proto, data_args)
+#endif
+
 /*
  * Make sure the alignment of the structure in the __tracepoints section will
  * not add unwanted padding between the beginning of the section and the
@@ -151,16 +167,8 @@  static inline void tracepoint_synchronize_unregister(void)
 				TP_ARGS(data_args),			\
 				TP_CONDITION(cond),,);			\
 	}								\
-	static inline void trace_##name##_rcuidle(proto)		\
-	{								\
-		if (static_key_false(&__tracepoint_##name.key))		\
-			__DO_TRACE(&__tracepoint_##name,		\
-				TP_PROTO(data_proto),			\
-				TP_ARGS(data_args),			\
-				TP_CONDITION(cond),			\
-				rcu_idle_exit(),			\
-				rcu_idle_enter());			\
-	}								\
+	__DECLARE_TRACE_RCU(name, PARAMS(proto), PARAMS(args),		\
+		PARAMS(cond), PARAMS(data_proto), PARAMS(data_args))	\
 	static inline int						\
 	register_trace_##name(void (*probe)(data_proto), void *data)	\
 	{								\