Re: [PATCH v3] tracing: timer: Add deferrable flag to timer_start

[Steven Rostedt <rostedt@goodmis.org>]

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)