Messages in this thread | | | Date | Fri, 29 May 2009 10:00:37 +0800 | From | Zhaolei <> | Subject | Re: Re: [PATCH 1/3] ftrace: add tracepoint for timer |
| |
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 ?
Hello, Thomas
Yes, timer's pointer is same before and after __init_timer(). We don't need to put debug_timer_init() and trace_timer_init() into different place.
But, for trace_timer_start() in __mod_timer(), we need to put it after timer->* changed. Nevertheless, it don't means we need separate trace_timer_start() and debug_timer_activate(), because we can put move debug_timer_activate() below, as: - debug_timer_activate(timer); ... timer->expires = expires; internal_add_timer(base, timer); + debug_timer_activate(timer); + trace_timer_start(timer, smp_processor_id());
Then we can also combine debug_timer_activate() and trace_timer_start() into one function.
So we'll do what you suggested.
Thanks Zhaolei
| |