Messages in this thread | | | Date | Tue, 5 May 2015 23:11:05 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH v3] tracing: timer: Add deferrable flag to timer_start |
| |
On Tue, 5 May 2015 19:44:10 -0700 Badhri Jagan Sridharan <badhri@google.com> wrote:
> diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h > index 68c2c20..342d741 100644 > --- a/include/trace/events/timer.h > +++ b/include/trace/events/timer.h > @@ -43,15 +43,17 @@ DEFINE_EVENT(timer_class, timer_init, > */ > TRACE_EVENT(timer_start, > > - TP_PROTO(struct timer_list *timer, unsigned long expires), > + TP_PROTO(struct timer_list *timer, > + unsigned long expires, char deferrable), > > - TP_ARGS(timer, expires), > + TP_ARGS(timer, expires, deferrable), > > TP_STRUCT__entry( > __field( void *, timer ) > __field( void *, function ) > __field( unsigned long, expires ) > __field( unsigned long, now ) > + __field( char, deferrable )
I wonder if we should use int instead of char. Although TIMER_DEFERRED is defined as 0x1LU, and that tbase_get_deferrable() returns:
((unsigned int)(unsigned long)base & TIMER_DEFERRABLE);
It is still an int that is returned. If TIMER_DEFERRABLE becomes something greater than 255, then that char wont be enough to hold it.
I'm being paranoid here, but you never know what changes may happen in the future, and I rather be robust and safe. And trace events save in 4byte chunks anyway, so you wont be saving anything in the buffer by making it a char.
-- Steve
> ), > > TP_fast_assign( > @@ -59,11 +61,13 @@ TRACE_EVENT(timer_start, > __entry->function = timer->function; > __entry->expires = expires; > __entry->now = jiffies; > + __entry->deferrable = deferrable; > ), > > - TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld]", > + TP_printk("timer=%p function=%pf expires=%lu [timeout=%ld] defer=%c", > __entry->timer, __entry->function, __entry->expires, > - (long)__entry->expires - __entry->now) > + (long)__entry->expires - __entry->now, > + __entry->deferrable > 0 ? 'y':'n') > ); > > /** > diff --git a/kernel/time/timer.c b/kernel/time/timer.c > index 2ece3aa..e5588da 100644 > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -648,7 +648,8 @@ static inline void > debug_activate(struct timer_list *timer, unsigned long expires) > { > debug_timer_activate(timer); > - trace_timer_start(timer, expires); > + trace_timer_start(timer, expires, > + tbase_get_deferrable(timer->base)); > } > > static inline void debug_deactivate(struct timer_list *timer)
| |