linux-kernel - Re: [PATCH v3] tracing: timer: Add deferrable flag to timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20150505231105.64208924@grimm.local.home>
Date:	Tue, 5 May 2015 23:11:05 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Badhri Jagan Sridharan <badhri@...gle.com>
Cc:	Ingo Molnar <mingo@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	John Stultz <john.stultz@...aro.org>,
	linux-kernel@...r.kernel.org
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@...gle.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)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/