Messages in this thread | | | Date | Fri, 27 Jun 2008 09:30:10 -0400 | From | Mathieu Desnoyers <> | Subject | Re: [RFC PATCH] Kernel Tracepoints |
| |
* Masami Hiramatsu (mhiramat@redhat.com) wrote: > > > Implementation of kernel tracepoints. Inspired from the Linux Kernel Markers. > > What would you think redesigning markers on tracepoints? because most of the > logic (scaning sections, multiple probe and activation) seems very similar > to markers. >
We could, although markers, because they use var args, allow to put the iteration on the multi probe array out-of-line. Tracepoints cannot afford this and the iteration must be done at the initial call-site.
From what I see in your proposal, it's mostly to extract the if() call() code from the inner __trace_mark() macro and to put it in a separate macro, am I correct ? This would make the macro more readable.
> For example, (not complete, I just thought :-)) > > struct tracepoint { > const char *name; /* Tracepoint name */ > DEFINE_IMV(char, state); /* Immediate value state. */ > struct tracepoint_probe_closure *multi; /* Closures */ > void * callsite_data; /* private date from call site */ > } __attribute__((aligned(8))); > > #define __tracepoint_block(generic, name, data, func, args) > static const char __tpstrtab_##name[] \ > __attribute__((section("__tracepoints_strings"))) \ > = #name; \ > static struct tracepoint __tracepoint_##name \ > __attribute__((section("__tracepoints"), aligned(8))) = \ > { __tpstrtab_##name, 0, NULL, data}; \ > if (!generic) { \ > if (unlikely(imv_cond(__tracepoint_##name.state))) { \ > imv_cond_end(); \ > func(&__tracepoint_##name, args); \ > } else \ > imv_cond_end(); \ > } else { \ > if (unlikely(_imv_read(__tracepoint_##name.state))) \ > func(&__tracepoint_##name, args); \ > } > > struct marker { > const char *name; /* Marker name */ > const char *format; /* Marker format string, describing the > * variable argument list. > */ > } __attribute__((aligned(8))); > > #define trace_mark(name, fmt, args...) \ > do { \ > static const char __mstrtab_##name[] \ > __attribute__((section("__markers_strings"))) \ > = #name "\0" fmt; \ > static struct marker __marker_##name \ > __attribute__((section("__markers"), aligned(8))) = \ > { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)]}; \ > __tracepoint_block(1, name, __marker_##name, marker_probe_cb, args) \ > } while (0) > > > [...] > > + static inline int register_trace_##name( \ > > + void (*probe)(void *private_data, proto), \ > > + void *private_data) \ > > + { \ > > + return tracepoint_probe_register(#name, (void *)probe, \ > > + private_data); \ > > + } \ > > + static inline void unregister_trace_##name( \ > > + void (*probe)(void *private_data, proto), \ > > + void *private_data) \ > > + { \ > > + tracepoint_probe_unregister(#name, (void *)probe, \ > > + private_data); \ > > + } > > Out of curiousity, what the private_data is for? >
When a probe is registered, it gives more flexibility to be able to pass a pointer to private data associated with that probe. For instance, if a tracer needs to register the same probe to many different tracepoints, but having a different context associated with each, it will pass the same function pointer with different private_data to the registration function.
> > + > > +extern void tracepoint_update_probe_range(struct tracepoint *begin, > > + struct tracepoint *end); > > + > > +#else /* !CONFIG_TRACEPOINTS */ > > +#define DEFINE_TRACE(name, proto, args) \ > > + static inline void _do_trace_##name(struct tracepoint *tp, proto) \ > > + { } \ > > + static inline void __trace_##name(int generic, proto) \ > > + { } \ > > + static inline void trace_##name(proto) \ > > + { } \ > > + static inline void _trace_##name(proto) \ > > + { } > > By the way, I think you'd better add below two inlines. > > static inline int register_trace_##name( \ > void (*probe)(void *private_data, proto), \ > void *private_data) \ > { return -ENOSYS; } > static inline void unregister_trace_##name( \ > void (*probe)(void *private_data, proto), \ > void *private_data) \ > { } >
My original thought was that if tracepoints are disabled, the probe objects should not even be built. But I can foresee that they will not always be in separate objects, so it makes sense. Will add.
Thanks,
Mathieu
-- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |