Re: [PATCH v3] ftrace: add a tracepoint for __raise_softirq_irqoff()

Mathieu Desnoyers
2009-05-14T03:50:49Z

* Lai Jiangshan (laijs@cn.fujitsu.com) wrote:
> Mathieu Desnoyers wrote:
> >
> > I partially agree with you :
> >
> > Yes, we should try to fix TRACE_EVENT, but we should fix it _before_ we
> > start using it widely. Circular header dependencies is a real problem
> > with TRACE_EVENT right now.
> >
> > Until we fix this, I will be tempted to stay with a known-good solution,
> > which is DECLARE/DEFINE_TRACE.
> >
> >
>
> I partially agree with you:
>
> Yes, Circular header dependencies is a real problem with TRACE_EVENT
> right now. It is also a problem with DECLARE_TRACE. It's a stubborn
> disease with C-Language (for complex headers). Can we fix C-Language?
>
> o Macros in header (!CREATE_TRACE_POINTS)
>
> When CREATE_TRACE_POINTS is not defined, TRACE_EVENT is definitely
> the same as DECLARE_TRACE. Actually, TRACE_EVENT is:
>
> #define TRACE_EVENT(name, proto, args, struct, assign, print) \
> DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
>
> So TRACE_EVENT and DECLARE_TRACE are the same in header files.
> And so TRACE_EVENT and DECLARE_TRACE have the same advantages and
> disadvantages. More TRACE_EVENT equals to a known-good solution.
>
> o Macros in c-file
>
> tracepoint uses DEFINE_TRACE only.
>
> ftrace uses CREATE_TRACE_POINTS + TRACE_EVENT:
> #define CREATE_TRACE_POINTS
> #include <trace/events/sched.h> (which uses TRACE_EVENT)
>
> ftrace generates more code which uses the tracepoints.
>
> >
> > Then add a forward declaration of
> >
> > struct softirqaction;
> >
> > At the top of trace/irq.h. I did it in quite a few places in the LTTng
> > tree. TP_PROTO just needs a forward declaration, not the full structure
> > declaration.
> >
>
> Thank you for your valuable suggestions.
>
> You are the father of tracepoint and LTTng, your experience in
> LTTng is very useful for ftrace.
>
> I'm glad for your suggestions.
>
>
> Xiao Guangrong, could you add forward declarations of
>
> struct irqaction;
> struct softirq_action;
>
> at the top of trace/irq.h as Mathieu's suggestions.
> (and remove "#include <linux/interrupt.h>")
>

You will probably still need something like :

#ifdef CREATE_TRACE_POINTS
#include <linux/interrupt.h>
#else
struct irqaction;
struct softirq_action;
#endif

So that FTRACE has the header dependencies it needs to build.

Mathieu

> Lai
>
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68