[PATCH v2] tracing: Adds handler name to irq_handler_entry

[PATCH v2] tracing: Adds handler name to irq_handler_entry

[Badhri Jagan Sridharan]

This patch includes the ISR function name to
irq_handler_entry trace point.

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

 include/trace/events/irq.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index ff8f6c0..dd8918c 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -58,14 +58,17 @@ TRACE_EVENT(irq_handler_entry,
 	TP_STRUCT__entry(
 		__field(	int,	irq		)
 		__string(	name,	action->name	)
+		__field(	void*,	handler		)
 	),
 
 	TP_fast_assign(
 		__entry->irq = irq;
 		__assign_str(name, action->name);
+		__entry->handler = action->handler;
 	),
 
-	TP_printk("irq=%d name=%s", __entry->irq, __get_str(name))
+	TP_printk("irq=%d name=%s handler=%pf",
+		  __entry->irq, __get_str(name), __entry->handler)
 );
 
 /**
-- 
2.2.0.rc0.207.ga3a616c

Re: [PATCH v2] tracing: Adds handler name to irq_handler_entry

[Steven Rostedt]

[ Added Thomas to the Cc, as he maintains interrupts ]

On Tue,  5 May 2015 20:06:27 -0700
Badhri Jagan Sridharan <badhri@google.com> wrote:

> This patch includes the ISR function name to
> irq_handler_entry trace point.

It states what this patch does, but not why it does it.

How much more difficult to map the handler to the interrupt?

Just asking, I'm not sure I needed this, and I use this tracepoint all
the time. But then again, I'm a bit of an tracing expert, and can add
function graph tracing to see what is happening too. I shouldn't always
be the judge on usefulness of added info here ;-)

> 
> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
> ---
> Changelog since v1:
> - fixed indentation
> 
>  include/trace/events/irq.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
> index ff8f6c0..dd8918c 100644
> --- a/include/trace/events/irq.h
> +++ b/include/trace/events/irq.h
> @@ -58,14 +58,17 @@ TRACE_EVENT(irq_handler_entry,
>  	TP_STRUCT__entry(
>  		__field(	int,	irq		)
>  		__string(	name,	action->name	)
> +		__field(	void*,	handler		)

Nit, you need a space between 'void' and '*'.

-- Steve

>  	),
>  
>  	TP_fast_assign(
>  		__entry->irq = irq;
>  		__assign_str(name, action->name);
> +		__entry->handler = action->handler;
>  	),
>  
> -	TP_printk("irq=%d name=%s", __entry->irq, __get_str(name))
> +	TP_printk("irq=%d name=%s handler=%pf",
> +		  __entry->irq, __get_str(name), __entry->handler)
>  );
>  
>  /**

Re: [PATCH v2] tracing: Adds handler name to irq_handler_entry

[Badhri Jagan Sridharan]

Yes Steven.. It shouldn't be too difficult to find out the
handler for a specific device. But, I am looking
at some user space scripts that looks across different devices,
to data-mine interrupt stats. Although, the interrupt
numbers from the device tree change from one device to
another, if its the same device driver, the handlers
remain the same. So it gives me a good sense of
similar interrupts that fire.

If you think that its not to much of an overhead,I will send
a follow up patch to correct the spacing error that you
mentioned.

And did you really add Thomas to CC ?

Thanks,
Badhri

On 05/05/2015 08:19 PM, Steven Rostedt wrote:
> [ Added Thomas to the Cc, as he maintains interrupts ]
>
> On Tue,  5 May 2015 20:06:27 -0700
> Badhri Jagan Sridharan <badhri@google.com> wrote:
>
>> This patch includes the ISR function name to
>> irq_handler_entry trace point.
>
> It states what this patch does, but not why it does it.
>
> How much more difficult to map the handler to the interrupt?
>
> Just asking, I'm not sure I needed this, and I use this tracepoint all
> the time. But then again, I'm a bit of an tracing expert, and can add
> function graph tracing to see what is happening too. I shouldn't always
> be the judge on usefulness of added info here ;-)
>
>>
>> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
>> ---
>> Changelog since v1:
>> - fixed indentation
>>
>>   include/trace/events/irq.h | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>> index ff8f6c0..dd8918c 100644
>> --- a/include/trace/events/irq.h
>> +++ b/include/trace/events/irq.h
>> @@ -58,14 +58,17 @@ TRACE_EVENT(irq_handler_entry,
>>   	TP_STRUCT__entry(
>>   		__field(	int,	irq		)
>>   		__string(	name,	action->name	)
>> +		__field(	void*,	handler		)
>
> Nit, you need a space between 'void' and '*'.
>
> -- Steve
>
>>   	),
>>
>>   	TP_fast_assign(
>>   		__entry->irq = irq;
>>   		__assign_str(name, action->name);
>> +		__entry->handler = action->handler;
>>   	),
>>
>> -	TP_printk("irq=%d name=%s", __entry->irq, __get_str(name))
>> +	TP_printk("irq=%d name=%s handler=%pf",
>> +		  __entry->irq, __get_str(name), __entry->handler)
>>   );
>>
>>   /**
>

Re: [PATCH v2] tracing: Adds handler name to irq_handler_entry

[Steven Rostedt]

On Thu, 07 May 2015 14:44:49 -0700
Badhri Jagan Sridharan <badhri@google.com> wrote:

> Yes Steven.. It shouldn't be too difficult to find out the
> handler for a specific device. But, I am looking
> at some user space scripts that looks across different devices,
> to data-mine interrupt stats. Although, the interrupt
> numbers from the device tree change from one device to
> another, if its the same device driver, the handlers
> remain the same. So it gives me a good sense of
> similar interrupts that fire.
> 
> If you think that its not to much of an overhead,I will send
> a follow up patch to correct the spacing error that you
> mentioned.

OK.

> 
> And did you really add Thomas to CC ?

Ug, utter fail :-p  I wrote that from my laptop while upstairs (not in
my office) and did not have Thomas in my address book. Then my wife
came in and started talking to me, and when she left I hit send
(forgetting to add Thomas in the Cc).

Added now!

-- Steve


> 
> Thanks,
> Badhri
> 
> On 05/05/2015 08:19 PM, Steven Rostedt wrote:
> > [ Added Thomas to the Cc, as he maintains interrupts ]
> >
> > On Tue,  5 May 2015 20:06:27 -0700
> > Badhri Jagan Sridharan <badhri@google.com> wrote:
> >
> >> This patch includes the ISR function name to
> >> irq_handler_entry trace point.
> >
> > It states what this patch does, but not why it does it.
> >
> > How much more difficult to map the handler to the interrupt?
> >
> > Just asking, I'm not sure I needed this, and I use this tracepoint all
> > the time. But then again, I'm a bit of an tracing expert, and can add
> > function graph tracing to see what is happening too. I shouldn't always
> > be the judge on usefulness of added info here ;-)
> >
> >>
> >> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
> >> ---
> >> Changelog since v1:
> >> - fixed indentation
> >>
> >>   include/trace/events/irq.h | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
> >> index ff8f6c0..dd8918c 100644
> >> --- a/include/trace/events/irq.h
> >> +++ b/include/trace/events/irq.h
> >> @@ -58,14 +58,17 @@ TRACE_EVENT(irq_handler_entry,
> >>   	TP_STRUCT__entry(
> >>   		__field(	int,	irq		)
> >>   		__string(	name,	action->name	)
> >> +		__field(	void*,	handler		)
> >
> > Nit, you need a space between 'void' and '*'.
> >
> > -- Steve
> >
> >>   	),
> >>
> >>   	TP_fast_assign(
> >>   		__entry->irq = irq;
> >>   		__assign_str(name, action->name);
> >> +		__entry->handler = action->handler;
> >>   	),
> >>
> >> -	TP_printk("irq=%d name=%s", __entry->irq, __get_str(name))
> >> +	TP_printk("irq=%d name=%s handler=%pf",
> >> +		  __entry->irq, __get_str(name), __entry->handler)
> >>   );
> >>
> >>   /**
> >

Re: [PATCH v2] tracing: Adds handler name to irq_handler_entry

[Badhri Jagan Sridharan]

Thanks Steven :) I have made the changes
that you mentioned and sent out an
updated version of the patch.

On 05/07/2015 02:54 PM, Steven Rostedt wrote:
> On Thu, 07 May 2015 14:44:49 -0700
> Badhri Jagan Sridharan <badhri@google.com> wrote:
>
>> Yes Steven.. It shouldn't be too difficult to find out the
>> handler for a specific device. But, I am looking
>> at some user space scripts that looks across different devices,
>> to data-mine interrupt stats. Although, the interrupt
>> numbers from the device tree change from one device to
>> another, if its the same device driver, the handlers
>> remain the same. So it gives me a good sense of
>> similar interrupts that fire.
>>
>> If you think that its not to much of an overhead,I will send
>> a follow up patch to correct the spacing error that you
>> mentioned.
>
> OK.
>
>>
>> And did you really add Thomas to CC ?
>
> Ug, utter fail :-p  I wrote that from my laptop while upstairs (not in
> my office) and did not have Thomas in my address book. Then my wife
> came in and started talking to me, and when she left I hit send
> (forgetting to add Thomas in the Cc).
>
> Added now!
>
> -- Steve
>
>
>>
>> Thanks,
>> Badhri
>>
>> On 05/05/2015 08:19 PM, Steven Rostedt wrote:
>>> [ Added Thomas to the Cc, as he maintains interrupts ]
>>>
>>> On Tue,  5 May 2015 20:06:27 -0700
>>> Badhri Jagan Sridharan <badhri@google.com> wrote:
>>>
>>>> This patch includes the ISR function name to
>>>> irq_handler_entry trace point.
>>>
>>> It states what this patch does, but not why it does it.
>>>
>>> How much more difficult to map the handler to the interrupt?
>>>
>>> Just asking, I'm not sure I needed this, and I use this tracepoint all
>>> the time. But then again, I'm a bit of an tracing expert, and can add
>>> function graph tracing to see what is happening too. I shouldn't always
>>> be the judge on usefulness of added info here ;-)
>>>
>>>>
>>>> Signed-off-by: Badhri Jagan Sridharan <Badhri@google.com>
>>>> ---
>>>> Changelog since v1:
>>>> - fixed indentation
>>>>
>>>>    include/trace/events/irq.h | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>>>> index ff8f6c0..dd8918c 100644
>>>> --- a/include/trace/events/irq.h
>>>> +++ b/include/trace/events/irq.h
>>>> @@ -58,14 +58,17 @@ TRACE_EVENT(irq_handler_entry,
>>>>    	TP_STRUCT__entry(
>>>>    		__field(	int,	irq		)
>>>>    		__string(	name,	action->name	)
>>>> +		__field(	void*,	handler		)
>>>
>>> Nit, you need a space between 'void' and '*'.
>>>
>>> -- Steve
>>>
>>>>    	),
>>>>
>>>>    	TP_fast_assign(
>>>>    		__entry->irq = irq;
>>>>    		__assign_str(name, action->name);
>>>> +		__entry->handler = action->handler;
>>>>    	),
>>>>
>>>> -	TP_printk("irq=%d name=%s", __entry->irq, __get_str(name))
>>>> +	TP_printk("irq=%d name=%s handler=%pf",
>>>> +		  __entry->irq, __get_str(name), __entry->handler)
>>>>    );
>>>>
>>>>    /**
>>>
>