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

[Lai Jiangshan]

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>")

Lai


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/