Simplifying the declaration may help [LWN.net]

Simplifying the declaration may help

Posted Apr 28, 2009 19:26 UTC (Tue) by dw (subscriber, #12017)
Parent article: On the value of static tracepoints

While I appreciate the value of trace points, if someone said they'd add 30 lines of macros to my lovely code because it might benefit a user someday, I'd probably not take kindly to it either.

The printk support doesn't seem useful, it's like a design argument was abandoned in favour of implementing both sides of the debate. Even with printk, it should be possible to combine instantiation + declaration like so:

#define TRACE_EVENT(name, assign, fields...)

TRACE_EVENT(mm_page_allocation,
            ({ t->pfn = pfn; t->free = free; }),
            unsigned long pfn, unsigned long free)

The fields argument could be stringified with cpp's paste operator and converted to a format string on first use at runtime, or perhaps by reading debug information during the build.

(Log in to post comments)

Simplifying the declaration may help

Posted Apr 28, 2009 19:56 UTC (Tue) by compudj (subscriber, #43335) [Link]

This is a no-go, as Linux Kernel Markers history has shown. It's good for debug-style ad-hoc tracing, but having a declaration in a global header, like the tracepoints are doing, _really_ helps for tracepoint maintainability.

Having to dig into the C source to find buried trace_mark() instances is not a pretty picture.

I think keeping the trace_mark() infrastructure is good as far as quick local tracing addition is concerned, but should not be added to mainline kernel code, given it is hard to maintain.

And regarding the declaration complexity, it's added by the TRACE_EVENT() macros done by Steven. The original tracepoint flavor I did use DECLARE_TRACE, which is far simpler, e.g.

DECLARE_TRACE(irq_entry,
TP_PROTO(unsigned int id, struct pt_regs *regs,
struct irqaction *action),
TP_ARGS(id, regs, action));

But it involves creating a probe module which contains the callback elsewhere. Adding tracer-specific callbacks in a different location is seen as too much of a burden by Ingo and Steven, hence their TRACE_EVENT() macro, which combines declaration and callback "definition".

I am still unconvinced it's the best way to go though, as keeping the callbacks separated from the header declaration isolates the "tracer ABI" from the kernel instrumentation. But given we stipulate that the tracer ABI *will* evolve over time (given we give the userspace tools enough flexibility to cope with this), it can be argued that having an "all in one place" TRACE_EVENT() declaration/definition is more valuable than isolation of instrumentation for userspace-visible ABI. I guess usage will tell.

Mathieu