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

[Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>]

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