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

[Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>]

* 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