Re: [PATCH 1/3] ftrace: add tracepoint for timer

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

Thomas Gleixner wrote:
> On Wed, 27 May 2009, Xiao Guangrong wrote:
>>>> +	TP_printk("timer=%p func=%pf expires=%lu cpu=%d", __entry->timer,
>>>> +		  __entry->function, __entry->expires, __entry->cpu)
>>>> +);
>>>   How do we connect the trace to the jiffies value when the timer
>>>   was started ?
>>>
>> ftrace already have time information in trace event's output, we can use it instead
> 
>   Hmm, I'm not sure whether we can see the jiffies value there, but ok. 
>  
>>>> @@ -547,6 +550,7 @@ void init_timer_key(struct timer_list *timer,
>>>>  {
>>>>  	debug_timer_init(timer);
>>>>  	__init_timer(timer, name, key);
>>>> +	trace_timer_init(timer);
>>>   Can we please avoid to have two debug calls in one 2 line function ?
>>>
>> debug_timer_init() must call before object's init, but tracepoint have to call
>> after object's init beacuse if we move the tracepoint to before object init, the
>> object has no data yet.
> 
>   Err.
> 
>>>> + TRACE_EVENT(timer_init,
>>>> +
>>>> + TP_PROTO(struct timer_list *timer),
>>>> +
>>>> + TP_ARGS(timer),
>>>> +
>>>> + TP_STRUCT__entry(
>>>> + __field( void *, timer )
>>>> + ),
>>>> +
>>>> + TP_fast_assign(
>>>> + __entry->timer = timer;
>>>> + ),
>>>> +
>>>> + TP_printk("timer=%p", __entry->timer)
>>>> +);
> 
>   Is timer different before and after the __init_timer call ?
>  

Hi tglx:

I have different view about this.

In ftrace, we only use the timer's pointer, It's same value before or after
__init_timer(). 
But TRACE_EVENT not only be used in ftrace but also be used in other probe
module. Maybe detailed information of timer is requisite in other probe funtion.
In this case, we must put trace_timer_init() after  __init_timer().

Thanks,
Xiao Guangrong

>>>>  }
>>>>  EXPORT_SYMBOL(init_timer_key);
>>>>  
>>>> @@ -565,6 +569,7 @@ static inline void detach_timer(struct timer_list *timer,
>>>>  	struct list_head *entry = &timer->entry;
>>>>  
>>>>  	debug_timer_deactivate(timer);
>>>> +	trace_timer_cancel(timer);
>>>   Ditto. Please create one debug entity which covers both.
>>>
>> IMHO, we can't create one entity for init event, so we do better detach other event.
> 
>   See above.
>  
> Thanks,
> 
> 	tglx
> 
>