2009-04-27 05:54:50

by Xiao Guangrong

[permalink] [raw]
Subject: [PATCH v2] ftrace: add a tracepoint for __raise_softirq_irqoff()

From: Mathieu Desnoyers <[email protected]>

This patch is modified from Mathieu Desnoyers' patch. The original patch
can be found here:
http://marc.info/?l=linux-kernel&m=123791201816245&w=2
This tracepoint can trace the time stamp when softirq action is raised.

Change log for v1 -> v2:
1: Use TRACE_EVENT instead of DEFINE_TRACE
2: Move the tracepoint from raise_softirq_irqoff() to __raise_softirq_irqoff()

Signed-off-by: Mathieu Desnoyers <[email protected]>
Signed-off-by: Xiao Guangrong <[email protected]>

---
include/linux/interrupt.h | 9 ++++++++-
include/trace/events/irq.h | 18 ++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 912c52c..86bae89 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -361,7 +361,14 @@ asmlinkage void do_softirq(void);
asmlinkage void __do_softirq(void);
extern void open_softirq(int nr, void (*action)(struct softirq_action *));
extern void softirq_init(void);
-#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
+
+#include <trace/events/irq.h>
+static inline void __raise_softirq_irqoff(unsigned int nr)
+{
+ trace_irq_softirq_raise(nr);
+ or_softirq_pending(1UL << nr);
+}
+
extern void raise_softirq_irqoff(unsigned int nr);
extern void raise_softirq(unsigned int nr);
extern void wakeup_softirqd(void);
diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
index 7686864..2628e08 100644
--- a/include/trace/events/irq.h
+++ b/include/trace/events/irq.h
@@ -90,6 +90,24 @@ TRACE_EVENT(softirq_exit,
TP_printk("softirq=%d action=%s", __entry->vec, __get_str(name))
);

+TRACE_EVENT(irq_softirq_raise,
+
+ TP_PROTO(unsigned int nr),
+
+ TP_ARGS(nr),
+
+ TP_STRUCT__entry(
+ __field( unsigned int, nr )
+ ),
+
+ TP_fast_assign(
+ __entry->nr = nr;
+ ),
+
+ TP_printk("softirq=%d action=%s is raised",
+ __entry->nr, softirq_to_name[__entry->nr])
+);
+
#endif /* _TRACE_IRQ_H */

/* This part must be outside protection */
--
1.6.1.2


2009-04-27 06:01:55

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH v2] ftrace: add a tracepoint for __raise_softirq_irqoff()

* Xiao Guangrong ([email protected]) wrote:
> From: Mathieu Desnoyers <[email protected]>
>
> This patch is modified from Mathieu Desnoyers' patch. The original patch
> can be found here:
> http://marc.info/?l=linux-kernel&m=123791201816245&w=2
> This tracepoint can trace the time stamp when softirq action is raised.
>
> Change log for v1 -> v2:
> 1: Use TRACE_EVENT instead of DEFINE_TRACE
> 2: Move the tracepoint from raise_softirq_irqoff() to __raise_softirq_irqoff()
>

Hi Xiao,

> Signed-off-by: Mathieu Desnoyers <[email protected]>
> Signed-off-by: Xiao Guangrong <[email protected]>
>
> ---
> include/linux/interrupt.h | 9 ++++++++-
> include/trace/events/irq.h | 18 ++++++++++++++++++
> 2 files changed, 26 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> index 912c52c..86bae89 100644
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -361,7 +361,14 @@ asmlinkage void do_softirq(void);
> asmlinkage void __do_softirq(void);
> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
> extern void softirq_init(void);
> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
> +
> +#include <trace/events/irq.h>

Includes are usually at the beginning of the files.

Besides that, the patch looks good.

Which tree is it for ? mainline 2.6.30-rc or Ingo's tip tree ?

Working with Ingo on -tip is probably better so the work can be
synchronized with Steven's work more easily.

Thanks a lot !

Mathieu

> +static inline void __raise_softirq_irqoff(unsigned int nr)
> +{
> + trace_irq_softirq_raise(nr);
> + or_softirq_pending(1UL << nr);
> +}
> +
> extern void raise_softirq_irqoff(unsigned int nr);
> extern void raise_softirq(unsigned int nr);
> extern void wakeup_softirqd(void);
> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
> index 7686864..2628e08 100644
> --- a/include/trace/events/irq.h
> +++ b/include/trace/events/irq.h
> @@ -90,6 +90,24 @@ TRACE_EVENT(softirq_exit,
> TP_printk("softirq=%d action=%s", __entry->vec, __get_str(name))
> );
>
> +TRACE_EVENT(irq_softirq_raise,
> +
> + TP_PROTO(unsigned int nr),
> +
> + TP_ARGS(nr),
> +
> + TP_STRUCT__entry(
> + __field( unsigned int, nr )
> + ),
> +
> + TP_fast_assign(
> + __entry->nr = nr;
> + ),
> +
> + TP_printk("softirq=%d action=%s is raised",
> + __entry->nr, softirq_to_name[__entry->nr])
> +);
> +
> #endif /* _TRACE_IRQ_H */
>
> /* This part must be outside protection */
> --
> 1.6.1.2
>

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68

2009-04-27 06:21:18

by Xiao Guangrong

[permalink] [raw]
Subject: Re: [PATCH v2] ftrace: add a tracepoint for __raise_softirq_irqoff()



Mathieu Desnoyers 写道:
> * Xiao Guangrong ([email protected]) wrote:
>> From: Mathieu Desnoyers <[email protected]>
>>
>> This patch is modified from Mathieu Desnoyers' patch. The original patch
>> can be found here:
>> http://marc.info/?l=linux-kernel&m=123791201816245&w=2
>> This tracepoint can trace the time stamp when softirq action is raised.
>>
>> Change log for v1 -> v2:
>> 1: Use TRACE_EVENT instead of DEFINE_TRACE
>> 2: Move the tracepoint from raise_softirq_irqoff() to __raise_softirq_irqoff()
>>
>
> Hi Xiao,
>
>> Signed-off-by: Mathieu Desnoyers <[email protected]>
>> Signed-off-by: Xiao Guangrong <[email protected]>
>>
>> ---
>> include/linux/interrupt.h | 9 ++++++++-
>> include/trace/events/irq.h | 18 ++++++++++++++++++
>> 2 files changed, 26 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
>> index 912c52c..86bae89 100644
>> --- a/include/linux/interrupt.h
>> +++ b/include/linux/interrupt.h
>> @@ -361,7 +361,14 @@ asmlinkage void do_softirq(void);
>> asmlinkage void __do_softirq(void);
>> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
>> extern void softirq_init(void);
>> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
>> +
>> +#include <trace/events/irq.h>
>
> Includes are usually at the beginning of the files.
>
> Besides that, the patch looks good.
>
Hi Mathieu:
I attempted to include <trace/events/irq.h> as your suggestion, but this results
in a cross references:
struct softirq_action have been used in tarce/event/irq.h.
so I move “include <trace/events/irq.h>” in the front of __raise_softirq_irqoff.

> Which tree is it for ? mainline 2.6.30-rc or Ingo's tip tree ?
>
> Working with Ingo on -tip is probably better so the work can be
> synchronized with Steven's work more easily.
>
This patch is work with Ingo on -tip tree

Thank you!
Xiao Guangrong
> Thanks a lot !
>
> Mathieu
>
>> +static inline void __raise_softirq_irqoff(unsigned int nr)
>> +{
>> + trace_irq_softirq_raise(nr);
>> + or_softirq_pending(1UL << nr);
>> +}
>> +
>> extern void raise_softirq_irqoff(unsigned int nr);
>> extern void raise_softirq(unsigned int nr);
>> extern void wakeup_softirqd(void);
>> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
>> index 7686864..2628e08 100644
>> --- a/include/trace/events/irq.h
>> +++ b/include/trace/events/irq.h
>> @@ -90,6 +90,24 @@ TRACE_EVENT(softirq_exit,
>> TP_printk("softirq=%d action=%s", __entry->vec, __get_str(name))
>> );
>>
>> +TRACE_EVENT(irq_softirq_raise,
>> +
>> + TP_PROTO(unsigned int nr),
>> +
>> + TP_ARGS(nr),
>> +
>> + TP_STRUCT__entry(
>> + __field( unsigned int, nr )
>> + ),
>> +
>> + TP_fast_assign(
>> + __entry->nr = nr;
>> + ),
>> +
>> + TP_printk("softirq=%d action=%s is raised",
>> + __entry->nr, softirq_to_name[__entry->nr])
>> +);
>> +
>> #endif /* _TRACE_IRQ_H */
>>
>> /* This part must be outside protection */
>> --
>> 1.6.1.2
>>
>

2009-04-27 15:32:29

by Mathieu Desnoyers

[permalink] [raw]
Subject: Re: [PATCH v2] ftrace: add a tracepoint for __raise_softirq_irqoff()

* Xiao Guangrong ([email protected]) wrote:
>
>
> Mathieu Desnoyers 写道:
> > * Xiao Guangrong ([email protected]) wrote:
> >> From: Mathieu Desnoyers <[email protected]>
> >>
> >> This patch is modified from Mathieu Desnoyers' patch. The original patch
> >> can be found here:
> >> http://marc.info/?l=linux-kernel&m=123791201816245&w=2
> >> This tracepoint can trace the time stamp when softirq action is raised.
> >>
> >> Change log for v1 -> v2:
> >> 1: Use TRACE_EVENT instead of DEFINE_TRACE
> >> 2: Move the tracepoint from raise_softirq_irqoff() to __raise_softirq_irqoff()
> >>
> >
> > Hi Xiao,
> >
> >> Signed-off-by: Mathieu Desnoyers <[email protected]>
> >> Signed-off-by: Xiao Guangrong <[email protected]>
> >>
> >> ---
> >> include/linux/interrupt.h | 9 ++++++++-
> >> include/trace/events/irq.h | 18 ++++++++++++++++++
> >> 2 files changed, 26 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
> >> index 912c52c..86bae89 100644
> >> --- a/include/linux/interrupt.h
> >> +++ b/include/linux/interrupt.h
> >> @@ -361,7 +361,14 @@ asmlinkage void do_softirq(void);
> >> asmlinkage void __do_softirq(void);
> >> extern void open_softirq(int nr, void (*action)(struct softirq_action *));
> >> extern void softirq_init(void);
> >> -#define __raise_softirq_irqoff(nr) do { or_softirq_pending(1UL << (nr)); } while (0)
> >> +
> >> +#include <trace/events/irq.h>
> >
> > Includes are usually at the beginning of the files.
> >
> > Besides that, the patch looks good.
> >
> Hi Mathieu:
> I attempted to include <trace/events/irq.h> as your suggestion, but this results
> in a cross references:
> struct softirq_action have been used in tarce/event/irq.h.
> so I move “include <trace/events/irq.h>” in the front of __raise_softirq_irqoff.

Steven : so, what's the current deal with

- #include <trace/irq.h> vs #include <trace/events/irq.h> ?

- Having header dependency in the ftrace-specific parts of the
TRACE_EVENT macros ?

Mathieu

>
> > Which tree is it for ? mainline 2.6.30-rc or Ingo's tip tree ?
> >
> > Working with Ingo on -tip is probably better so the work can be
> > synchronized with Steven's work more easily.
> >
> This patch is work with Ingo on -tip tree
>
> Thank you!
> Xiao Guangrong
> > Thanks a lot !
> >
> > Mathieu
> >
> >> +static inline void __raise_softirq_irqoff(unsigned int nr)
> >> +{
> >> + trace_irq_softirq_raise(nr);
> >> + or_softirq_pending(1UL << nr);
> >> +}
> >> +
> >> extern void raise_softirq_irqoff(unsigned int nr);
> >> extern void raise_softirq(unsigned int nr);
> >> extern void wakeup_softirqd(void);
> >> diff --git a/include/trace/events/irq.h b/include/trace/events/irq.h
> >> index 7686864..2628e08 100644
> >> --- a/include/trace/events/irq.h
> >> +++ b/include/trace/events/irq.h
> >> @@ -90,6 +90,24 @@ TRACE_EVENT(softirq_exit,
> >> TP_printk("softirq=%d action=%s", __entry->vec, __get_str(name))
> >> );
> >>
> >> +TRACE_EVENT(irq_softirq_raise,
> >> +
> >> + TP_PROTO(unsigned int nr),
> >> +
> >> + TP_ARGS(nr),
> >> +
> >> + TP_STRUCT__entry(
> >> + __field( unsigned int, nr )
> >> + ),
> >> +
> >> + TP_fast_assign(
> >> + __entry->nr = nr;
> >> + ),
> >> +
> >> + TP_printk("softirq=%d action=%s is raised",
> >> + __entry->nr, softirq_to_name[__entry->nr])
> >> +);
> >> +
> >> #endif /* _TRACE_IRQ_H */
> >>
> >> /* This part must be outside protection */
> >> --
> >> 1.6.1.2
> >>
> >

--
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68