Messages in this thread | | | Date | Wed, 27 May 2009 15:36:06 +0800 | From | Xiao Guangrong <> | Subject | Re: [PATCH 1/3] ftrace: add tracepoint for timer |
| |
Thomas Gleixner wrote: > On Fri, 22 May 2009, Xiao Guangrong wrote: > >> This patch is modify from Mathieu's patch base on ingo's suggestion, the original patch >> can be found here: >> http://marc.info/?l=linux-kernel&m=123791201816247&w=2 > > I have a hard time to connect this patch to the original one. >
There are some timer hook in Mathieu's patch, I just modify the tracepoint name and add the tracepoint in other suitable place.
>> +TRACE_EVENT(timer_start, >> + >> + TP_PROTO(struct timer_list *timer, int cpu), >> + >> + TP_ARGS(timer, cpu), >> + >> + TP_STRUCT__entry( >> + __field( void *, timer ) >> + __field( void *, function ) >> + __field( unsigned long, expires ) >> + __field( int, cpu ) >> + ), >> + >> + TP_fast_assign( >> + __entry->timer = timer; >> + __entry->function = timer->function; >> + __entry->expires = timer->expires; >> + __entry->cpu = cpu; >> + ), >> + >> + 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
>> + >> +TRACE_EVENT(timer_expire, >> + >> + TP_PROTO(struct timer_list *timer), >> + >> + TP_ARGS(timer), >> + >> + TP_STRUCT__entry( >> + __field( void *, timer ) >> + __field( void *, function ) >> + ), >> + >> + TP_fast_assign( >> + __entry->timer = timer; >> + __entry->function = timer->function; >> + ), >> + >> + TP_printk("timer=%p func=%pf", __entry->timer, __entry->function) >> +); > > Ditto. >
Yes, you are right, I'll remove __entry->function in v2 patch
>> +TRACE_EVENT(timer_cancel, >> + >> + TP_PROTO(struct timer_list *timer), >> + >> + TP_ARGS(timer), >> + >> + TP_STRUCT__entry( >> + __field( void *, timer ) >> + __field( void *, function ) >> + ), >> + >> + TP_fast_assign( >> + __entry->timer = timer; >> + __entry->function = timer->function; >> + ), >> + >> + TP_printk("timer=%p func=%pf", __entry->timer, __entry->function) >> +); > > Same here. > >> @@ -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.
>> } >> 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.
for example:
in init event: void init_timer_key(...) { debug_timer_init(timer); __init_timer(timer, name, key); trace_timer_init(timer); } but in detach event (): void detach_timer(...) { ...... trace_timer_deactivate() }
void trace_timer_deactivate() { debug_timer_deactivate(timer); trace_timer_cancel(timer); } it's disunity for us.
Thanks, Xiao Guangrong > .... > > Thanks, > > tglx > >
| |