public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] tracing: timer: Add deferrable flag to timer_start
@ 2015-05-06  2:44 Badhri Jagan Sridharan
  2015-05-06  3:11 ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Badhri Jagan Sridharan @ 2015-05-06  2:44 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar, Thomas Gleixner, John Stultz
  Cc: linux-kernel, Badhri Jagan Sridharan

The timer_start event now shows whether the timer is
deferrable in case of a low-res timer. The debug_activate
function now includes deferrable flag while calling
trace_timer_start event.

Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
---
Changelog since v2:
- fixed indentation

Changelog since v1:
- fixed indentation
- moved deferrable flag checking to tracing code

 include/trace/events/timer.h | 12 ++++++++----
 kernel/time/timer.c          |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

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	)
 	),
 
 	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)
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] tracing: timer: Add deferrable flag to timer_start
  2015-05-06  2:44 [PATCH v3] tracing: timer: Add deferrable flag to timer_start Badhri Jagan Sridharan
@ 2015-05-06  3:11 ` Steven Rostedt
  0 siblings, 0 replies; 5+ messages in thread
From: Steven Rostedt @ 2015-05-06  3:11 UTC (permalink / raw)
  To: Badhri Jagan Sridharan
  Cc: Ingo Molnar, Thomas Gleixner, John Stultz, linux-kernel

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)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3] tracing: timer: Add deferrable flag to timer_start
@ 2015-05-07 23:20 Badhri Jagan Sridharan
  2015-05-13 14:11 ` Steven Rostedt
  0 siblings, 1 reply; 5+ messages in thread
From: Badhri Jagan Sridharan @ 2015-05-07 23:20 UTC (permalink / raw)
  To: Steven Rostedt, Ingo Molnar, Thomas Gleixner, John Stultz
  Cc: linux-kernel, Badhri Jagan Sridharan

The timer_start event now shows whether the timer is
deferrable in case of a low-res timer. The debug_activate
function now includes deferrable flag while calling
trace_timer_start event.

Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
---
Changelog since v2:
- changed deferrable flag to unsigned int

Changelog since v1:
- fixed indentation
- moved deferrable flag checking to tracing code

 include/trace/events/timer.h | 13 +++++++++----
 kernel/time/timer.c          |  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 68c2c20..d7abef1 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -43,15 +43,18 @@ 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,
+		unsigned int 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( unsigned int,	deferrable	)
 	),
 
 	TP_fast_assign(
@@ -59,11 +62,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)
-- 
2.2.0.rc0.207.ga3a616c


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] tracing: timer: Add deferrable flag to timer_start
  2015-05-07 23:20 Badhri Jagan Sridharan
@ 2015-05-13 14:11 ` Steven Rostedt
  2015-05-13 23:39   ` John Stultz
  0 siblings, 1 reply; 5+ messages in thread
From: Steven Rostedt @ 2015-05-13 14:11 UTC (permalink / raw)
  To: Badhri Jagan Sridharan
  Cc: Ingo Molnar, Thomas Gleixner, John Stultz, linux-kernel

On Thu,  7 May 2015 16:20:34 -0700
Badhri Jagan Sridharan <badhri@google.com> wrote:

> The timer_start event now shows whether the timer is
> deferrable in case of a low-res timer. The debug_activate
> function now includes deferrable flag while calling
> trace_timer_start event.
> 
> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>

Acked-by: Steven Rostedt <rostedt@goodmis.org>

John, Thomas, you want to pick this up?

-- Steve

> ---
> Changelog since v2:
> - changed deferrable flag to unsigned int
> 
> Changelog since v1:
> - fixed indentation
> - moved deferrable flag checking to tracing code
> 
>  include/trace/events/timer.h | 13 +++++++++----
>  kernel/time/timer.c          |  3 ++-
>  2 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
> index 68c2c20..d7abef1 100644
> --- a/include/trace/events/timer.h
> +++ b/include/trace/events/timer.h
> @@ -43,15 +43,18 @@ 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,
> +		unsigned int 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( unsigned int,	deferrable	)
>  	),
>  
>  	TP_fast_assign(
> @@ -59,11 +62,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)


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] tracing: timer: Add deferrable flag to timer_start
  2015-05-13 14:11 ` Steven Rostedt
@ 2015-05-13 23:39   ` John Stultz
  0 siblings, 0 replies; 5+ messages in thread
From: John Stultz @ 2015-05-13 23:39 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: Badhri Jagan Sridharan, Ingo Molnar, Thomas Gleixner, lkml

On Wed, May 13, 2015 at 7:11 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Thu,  7 May 2015 16:20:34 -0700
> Badhri Jagan Sridharan <badhri@google.com> wrote:
>
>> The timer_start event now shows whether the timer is
>> deferrable in case of a low-res timer. The debug_activate
>> function now includes deferrable flag while calling
>> trace_timer_start event.
>>
>> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
>
> Acked-by: Steven Rostedt <rostedt@goodmis.org>
>
> John, Thomas, you want to pick this up?

Got some time to queue a few items and pulled this into my queue. Will
push to Thomas for 4.2 unless he objects.

thanks
-john

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-05-13 23:39 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-05-06  2:44 [PATCH v3] tracing: timer: Add deferrable flag to timer_start Badhri Jagan Sridharan
2015-05-06  3:11 ` Steven Rostedt
  -- strict thread matches above, loose matches on Subject: below --
2015-05-07 23:20 Badhri Jagan Sridharan
2015-05-13 14:11 ` Steven Rostedt
2015-05-13 23:39   ` John Stultz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox