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

Zhaolei
2009-05-18T03:07:58Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 2048 bytes --]

* From: "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. ;-)

Hello,

It seems Mathieu has no other comments on this patch now.
Ingo, what is your opinion on this patch?

Thanks
Zhaolei

>
>> -- Steve
>>
>>
>>
>ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éÝ¶\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dÊ‡Ú™ë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥