Re: [PATCH tip/core/rcu 04/15] rcu: Permit RCU_NONIDLE() to be usedfrom interrupt context

[Steven Rostedt]

On Tue, 2012-09-04 at 16:33 -0700, Josh Triplett wrote:
>  
> > > +#ifdef 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
> > > +
> > 
> > Egad! More macros on top of macros! Yeah I know I'm the most guilty of
> > this, but it just seems to add one more indirection that I would like to
> > avoid.
> 
> This doesn't seem to add a significant amount of complexity; it forwards
> exactly the same parameters to the helper macro, and just moves the one
> function definition there and makes it conditional.  This still seems
> more preferable than exporting functions just so they won't get called.

The change itself is not complex. But what is already there is complex
enough. I'm talking more about adding to:

$ grep '#define' include/linux/tracepoint.h
#define _LINUX_TRACEPOINT_H
#define PARAMS(args...) args
#define TP_PROTO(args...)       args
#define TP_ARGS(args...)        args
#define TP_CONDITION(args...)   args
#define __DO_TRACE(tp, proto, args, cond, prercu, postrcu)              \
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
#define DEFINE_TRACE_FN(name, reg, unreg)                                \
#define DEFINE_TRACE(name)                                              \
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)                              \
#define EXPORT_TRACEPOINT_SYMBOL(name)                                  \
#define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
#define DEFINE_TRACE_FN(name, reg, unreg)
#define DEFINE_TRACE(name)
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)
#define DECLARE_TRACE_NOARGS(name)                                      \
#define DECLARE_TRACE(name, proto, args)                                \
#define DECLARE_TRACE_CONDITION(name, proto, args, cond)                \
#define TRACE_EVENT_FLAGS(event, flag)
#define DECLARE_EVENT_CLASS(name, proto, args, tstruct, assign, print)
#define DEFINE_EVENT(template, name, proto, args)               \
#define DEFINE_EVENT_PRINT(template, name, proto, args, print)  \
#define DEFINE_EVENT_CONDITION(template, name, proto,           \
#define TRACE_EVENT(name, proto, args, struct, assign, print)   \
#define TRACE_EVENT_FN(name, proto, args, struct,               \
#define TRACE_EVENT_CONDITION(name, proto, args, cond,          \
#define TRACE_EVENT_FLAGS(event, flag)


> 
> I could live with that; it seems preferable to unnecessary exports,
> though it still seems much uglier to me than the conditional definition
> of trace_*_rcuidle. :)

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.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/