Messages in this thread | | | Date | Thu, 14 May 2009 14:06:34 +0800 | From | Lai Jiangshan <> | Subject | Re: [PATCH v3] ftrace: add a tracepoint for __raise_softirq_irqoff() |
| |
Mathieu Desnoyers wrote: > * 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 >
It's not needed for trace/events/irq.h
Yes, it's a solution.
But I don't think we have to do this, CREATE_TRACE_POINTS is needed only _once_ for every <trace/events/xxxx.h>
The .c file which defines CREATE_TRACE_POINTS can provide (had provided likely) things like "#include <linux/interrupt.h>"
See kernel/softirq.c: #include <linux/interrupt.h> ...... ...... #define CREATE_TRACE_POINTS #include <trace/events/irq.h>
I don't think it's a problem, CREATE_TRACE_POINTS is defined only once for a <trace/events/xxxx.h>.
Lai.
| |