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

Xiao Guangrong
2009-05-15T01:53:25Z

Steven Rostedt wrote:
> On Thu, 14 May 2009, Mathieu Desnoyers wrote:
>>> From: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
>>>
>>> This patch is modified from Mathieu Desnoyers' patch. The original patch
>>> can be found here:
>>> http://marc.info/?l=linux-kernel&m=123791201816245&w=2
>>> This tracepoint can trace the time stamp when softirq action is raised.
>>>
>>> Changelog for v1 -> v2:
>>> 1: Use TRACE_EVENT instead of DEFINE_TRACE
>>> 2: Move the tracepoint from raise_softirq_irqoff() to
>>> __raise_softirq_irqoff()
>>>
>>> Changelog for v2 -> v3:
>>> Move the definition of __raise_softifq_irqoff() to .c file when
>>> CONFIG_TRACEPOINTS is enabled, to avoid recursive includes
>>>
>>> Changelog for v3 -> v4:
>>> 1: Come back to v2, and use forward declarations to avoid
>>> recursive includes as Mathieu's suggestion
>>> 2: Modifiy the tracepoint name
>>> 3: Add comments for this tracepoint
>>>
>> This is a step in the right direction, but please see my email to Lai
>> about the fact that this assumes correct and undocumented include
>> dependencies in kernel/trace/events.c. Not explicitely stating the
>> include dependencies is a build error waiting to happen.
>>
>> Including interrupt.h under a ifdef would allow keeping track of
>> TRACE_EVENT specific build dependencies neatly on a per header basis.
>
> This is all moot, the events.c file no longer exists and as not an issue.
>

As Steve's says, use ftrace in ftrace.h not in events.c now.
So, this mistake does not exist.
Dose this patch has other error? I expect for your views.

Thanks for your review, is great help to me. ;-)

> -- Steve
>
>
>