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

Steven Rostedt
2009-05-14T13:25:32Z

On Thu, 14 May 2009, Mathieu Desnoyers wrote:
> > >
> >
> > 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>
> >
>
> Isn't it all included under kernel/trace/events.c ? e.g. :

Mathieu,

I've told you this two or three times before. That file no longer exists.
Maybe it is still there for .30, but these updates are for .31 (too late
for .30). The code to make TRACE_EVENT work for modules removed the
events.c file and got rid of those nasty stages.

-- Steve

>
> #include <trace/trace_events.h>
>
> #include "trace_output.h"
>
> #include "trace_events_stage_1.h"
> #include "trace_events_stage_2.h"
> #include "trace_events_stage_3.h"
>
> Therefore, adding a #include <linux/interrupt.h> would be just weird
> here. We would have to do that for every tracepoint headers, and we
> would just lose the ability to keep track of which tracepoint header has
> which include dependency.
>
> I thought the goal of TRACE_EVENT() was exactly the opposite : to
> declare everything in one location....
>
> Therefore, if you agree that it's good to have TRACE_EVENT() declaring
> everything in one location, then I don't see why you would argue to move
> the include dependencies in the very remote kernel/trace/events.c file ?
>
> Mathieu
>
> > I don't think it's a problem, CREATE_TRACE_POINTS is defined only
> > once for a <trace/events/xxxx.h>.
> >
> > Lai.
> >
>
> --
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
>