lkml.org 
[lkml]   [2011]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH] Tracepoint: introduce tracepoint() API
On Thu, Nov 17, 2011 at 04:06:54PM -0500, Steven Rostedt wrote:
> On Thu, 2011-11-17 at 15:50 -0500, Mathieu Desnoyers wrote:
> > Introduce:
> >
> > tracepoint(event_name, arg1, arg2, ...)
> >
> > while keeping the old tracepoint API in place, e.g.:
> >
> > trace_event_name(arg1, arg2, ...)
> >
> > This allows skipping parameter side-effects (pointer dereference,
> > function calls, ...) when the tracepoint is not dynamically activated.
> >
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > ---
> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index d530a44..c9c73f7 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -107,6 +107,12 @@ void tracepoint_update_probe_range(struct tracepoint * const *begin,
> >
> > #ifdef CONFIG_TRACEPOINTS
> >
> > +#define tracepoint(name, args...) \
> > + do { \
> > + if (static_branch(&__tracepoint_##name.key)) \
> > + __trace_##name(args); \
> > + } while (0)
> > +
> > /*
> > * it_func[0] is never NULL because there is at least one element in the array
> > * when the array itself is non NULL.
> > @@ -144,13 +150,17 @@ void tracepoint_update_probe_range(struct tracepoint * const *begin,
> > */
> > #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
> > extern struct tracepoint __tracepoint_##name; \
> > + static inline void __trace_##name(proto) \
> > + { \
> > + __DO_TRACE(&__tracepoint_##name, \
> > + TP_PROTO(data_proto), \
> > + TP_ARGS(data_args), \
> > + TP_CONDITION(cond)); \
> > + } \
>
> I wrote a patch earlier today that does almost the exact same thing, but
> I had more in macro part, which I would have cleaned up after the RFC. I
> didn't add another static inline, but I think this approach is a little
> cleaner (with the second static inline).
>
> I didn't post mine because I was still analyzing the assembly to make
> sure it did what I expected. But I got side tracked on other things (RT
> related) and didn't quite finish the analysis.
>
> Did you do a compare of kmem_cache_alloc() to see if this fixes the
> reported problem?
>
> -- Steve
>
> > static inline void trace_##name(proto) \
> > { \
> > if (static_branch(&__tracepoint_##name.key)) \
> > - __DO_TRACE(&__tracepoint_##name, \
> > - TP_PROTO(data_proto), \
> > - TP_ARGS(data_args), \
> > - TP_CONDITION(cond)); \
> > + __trace_##name(args); \
> > } \
> > static inline int \
> > register_trace_##name(void (*probe)(data_proto), void *data) \
> > @@ -193,7 +203,12 @@ void tracepoint_update_probe_range(struct tracepoint * const *begin,
> > EXPORT_SYMBOL(__tracepoint_##name)
> >
> > #else /* !CONFIG_TRACEPOINTS */
> > +
> > +#define tracepoint(name, args...) __trace_##name(args)
> > +
> > #define __DECLARE_TRACE(name, proto, args, cond, data_proto, data_args) \
> > + static inline void __trace_##name(proto) \
> > + { } \
> > static inline void trace_##name(proto) \
> > { } \
> > static inline int \
> >
>

I brought this issue up a while back, with a very similar patch to what
Mathieu wrote. Please see:

http://thread.gmane.org/gmane.linux.kernel/838911

One conclusion from that thread was that it would be nice if gcc could
optimize the load so that it only happens in the unlikely path. I filed
a gcc bug for that: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40207,
but it doesn't seem to be implemented yet...

Thanks,

-Jason


\
 
 \ /
  Last update: 2011-11-17 23:01    [from the cache]
©2003-2011 Jasper Spaans