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    [W:0.027 / U:34.536 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site