[4/4] (RFC) X86: add IPI tracepoints

Message ID 1405660735-13408-5-git-send-email-nicolas.pitre@linaro.org
State New
Headers show

Commit Message

Nicolas Pitre July 18, 2014, 5:18 a.m.
On X86 there are already tracepoints for IRQ vectors through which IPIs
are handled.  However this is highly X86 specific, and the IPI signaling
is not currently traced.

This is an attempt at adding generic IPI tracepoints to X86.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 arch/x86/kernel/smp.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Steven Rostedt July 18, 2014, 8:27 p.m. | #1
On Fri, 18 Jul 2014 01:18:55 -0400
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

 
> diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
> index be8e1bde07..e154d176cf 100644
> --- a/arch/x86/kernel/smp.c
> +++ b/arch/x86/kernel/smp.c
> @@ -31,6 +31,12 @@
>  #include <asm/apic.h>
>  #include <asm/nmi.h>
>  #include <asm/trace/irq_vectors.h>
> +
> +#define CREATE_TRACE_POINTS
> +#undef TRACE_INCLUDE_PATH
> +#undef TRACE_INCLUDE_FILE

I'm curious to why you added the #undefs. I wouldn't think they were
needed.

-- Steve

> +#include <trace/events/ipi.h>
> +
>  /*
>   *	Some notes on x86 processor bugs affecting SMP operation:
>   *
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre July 18, 2014, 8:59 p.m. | #2
On Fri, 18 Jul 2014, Steven Rostedt wrote:

> On Fri, 18 Jul 2014 01:18:55 -0400
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
>  
> > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
> > index be8e1bde07..e154d176cf 100644
> > --- a/arch/x86/kernel/smp.c
> > +++ b/arch/x86/kernel/smp.c
> > @@ -31,6 +31,12 @@
> >  #include <asm/apic.h>
> >  #include <asm/nmi.h>
> >  #include <asm/trace/irq_vectors.h>
> > +
> > +#define CREATE_TRACE_POINTS
> > +#undef TRACE_INCLUDE_PATH
> > +#undef TRACE_INCLUDE_FILE
> 
> I'm curious to why you added the #undefs. I wouldn't think they were
> needed.

They are needed because asm/trace/irq_vectors.h included prior that 
point defines them for itself and that makes the inclusion of 
trace/events/ipi.h fail.

> -- Steve
> 
> > +#include <trace/events/ipi.h>
> > +
> >  /*
> >   *	Some notes on x86 processor bugs affecting SMP operation:
> >   *
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Davidlohr Bueso July 20, 2014, 8:25 p.m. | #3
On Fri, 2014-07-18 at 01:18 -0400, Nicolas Pitre wrote:
> On X86 there are already tracepoints for IRQ vectors through which IPIs
> are handled.  However this is highly X86 specific, and the IPI signaling
> is not currently traced.
> 
> This is an attempt at adding generic IPI tracepoints to X86.

I welcome this, and fwiw have been trying out this patch. One thing I
would like to see, but due to overhead would probably be better suited
in userspace (trace-cmd, maybe?), is a more packed description of the
IPI. Ie: unifying ipi_init and ipi_exit and show overall cost of the
IPI.

Thanks,
Davidlohr

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre July 21, 2014, 10:35 p.m. | #4
On Sun, 20 Jul 2014, Davidlohr Bueso wrote:

> On Fri, 2014-07-18 at 01:18 -0400, Nicolas Pitre wrote:
> > On X86 there are already tracepoints for IRQ vectors through which IPIs
> > are handled.  However this is highly X86 specific, and the IPI signaling
> > is not currently traced.
> > 
> > This is an attempt at adding generic IPI tracepoints to X86.
> 
> I welcome this, and fwiw have been trying out this patch. One thing I
> would like to see, but due to overhead would probably be better suited
> in userspace (trace-cmd, maybe?), is a more packed description of the
> IPI. Ie: unifying ipi_init and ipi_exit and show overall cost of the
> IPI.

That's best suited for the tool consuming the log in user space.  The 
same is done with IRQ events already.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Steven Rostedt July 21, 2014, 10:54 p.m. | #5
On Fri, 18 Jul 2014 16:59:50 -0400 (EDT)
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> On Fri, 18 Jul 2014, Steven Rostedt wrote:
> 
> > On Fri, 18 Jul 2014 01:18:55 -0400
> > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > 
> >  
> > > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
> > > index be8e1bde07..e154d176cf 100644
> > > --- a/arch/x86/kernel/smp.c
> > > +++ b/arch/x86/kernel/smp.c
> > > @@ -31,6 +31,12 @@
> > >  #include <asm/apic.h>
> > >  #include <asm/nmi.h>
> > >  #include <asm/trace/irq_vectors.h>
> > > +
> > > +#define CREATE_TRACE_POINTS
> > > +#undef TRACE_INCLUDE_PATH
> > > +#undef TRACE_INCLUDE_FILE
> > 
> > I'm curious to why you added the #undefs. I wouldn't think they were
> > needed.
> 
> They are needed because asm/trace/irq_vectors.h included prior that 
> point defines them for itself and that makes the inclusion of 
> trace/events/ipi.h fail.
> 

Bah, I tried to get rid of the need for those, but it seems that the
macro magic is getting a bit out of hand. I need a new macro magic
hat :-p

What you got here will have to do.


-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Nicolas Pitre July 23, 2014, 10 p.m. | #6
On Mon, 21 Jul 2014, Steven Rostedt wrote:

> On Fri, 18 Jul 2014 16:59:50 -0400 (EDT)
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
> > On Fri, 18 Jul 2014, Steven Rostedt wrote:
> > 
> > > On Fri, 18 Jul 2014 01:18:55 -0400
> > > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > 
> > >  
> > > > diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
> > > > index be8e1bde07..e154d176cf 100644
> > > > --- a/arch/x86/kernel/smp.c
> > > > +++ b/arch/x86/kernel/smp.c
> > > > @@ -31,6 +31,12 @@
> > > >  #include <asm/apic.h>
> > > >  #include <asm/nmi.h>
> > > >  #include <asm/trace/irq_vectors.h>
> > > > +
> > > > +#define CREATE_TRACE_POINTS
> > > > +#undef TRACE_INCLUDE_PATH
> > > > +#undef TRACE_INCLUDE_FILE
> > > 
> > > I'm curious to why you added the #undefs. I wouldn't think they were
> > > needed.
> > 
> > They are needed because asm/trace/irq_vectors.h included prior that 
> > point defines them for itself and that makes the inclusion of 
> > trace/events/ipi.h fail.
> > 
> 
> Bah, I tried to get rid of the need for those, but it seems that the
> macro magic is getting a bit out of hand. I need a new macro magic
> hat :-p
> 
> What you got here will have to do.

OK.

Now the real question: should I submit it for mainline?  I'm a little 
bothered by the fact that all exception vectors already have tracepoints 
of their own, albeit deeply tied to X86 nomenclature.  But nothing that 
relates to IPI sources.  This patch fixes that by adding some 
tracepoints alongside existing ones which may go a long way in making 
their instrumentation with generic (arch independent) tools.

What do people think?


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

Patch

diff --git a/arch/x86/kernel/smp.c b/arch/x86/kernel/smp.c
index be8e1bde07..e154d176cf 100644
--- a/arch/x86/kernel/smp.c
+++ b/arch/x86/kernel/smp.c
@@ -31,6 +31,12 @@ 
 #include <asm/apic.h>
 #include <asm/nmi.h>
 #include <asm/trace/irq_vectors.h>
+
+#define CREATE_TRACE_POINTS
+#undef TRACE_INCLUDE_PATH
+#undef TRACE_INCLUDE_FILE
+#include <trace/events/ipi.h>
+
 /*
  *	Some notes on x86 processor bugs affecting SMP operation:
  *
@@ -124,11 +130,13 @@  static void native_smp_send_reschedule(int cpu)
 		WARN_ON(1);
 		return;
 	}
+	trace_ipi_raise(cpumask_of(cpu), tracepoint_string("RESCHEDULE"));
 	apic->send_IPI_mask(cpumask_of(cpu), RESCHEDULE_VECTOR);
 }
 
 void native_send_call_func_single_ipi(int cpu)
 {
+	trace_ipi_raise(cpumask_of(cpu), tracepoint_string("CALL_FUNCTION_SINGLE"));
 	apic->send_IPI_mask(cpumask_of(cpu), CALL_FUNCTION_SINGLE_VECTOR);
 }
 
@@ -136,6 +144,8 @@  void native_send_call_func_ipi(const struct cpumask *mask)
 {
 	cpumask_var_t allbutself;
 
+	trace_ipi_raise(mask, tracepoint_string("CALL_FUNCTION"));
+
 	if (!alloc_cpumask_var(&allbutself, GFP_ATOMIC)) {
 		apic->send_IPI_mask(mask, CALL_FUNCTION_VECTOR);
 		return;
@@ -252,8 +262,10 @@  finish:
  */
 static inline void __smp_reschedule_interrupt(void)
 {
+	trace_ipi_entry(tracepoint_string("RESCHEDULE"));
 	inc_irq_stat(irq_resched_count);
 	scheduler_ipi();
+	trace_ipi_exit(tracepoint_string("RESCHEDULE"));
 }
 
 __visible void smp_reschedule_interrupt(struct pt_regs *regs)
@@ -291,8 +303,10 @@  __visible void smp_trace_reschedule_interrupt(struct pt_regs *regs)
 
 static inline void __smp_call_function_interrupt(void)
 {
+	trace_ipi_entry(tracepoint_string("CALL_FUNCTION"));
 	generic_smp_call_function_interrupt();
 	inc_irq_stat(irq_call_count);
+	trace_ipi_exit(tracepoint_string("CALL_FUNCTION"));
 }
 
 __visible void smp_call_function_interrupt(struct pt_regs *regs)
@@ -313,8 +327,10 @@  __visible void smp_trace_call_function_interrupt(struct pt_regs *regs)
 
 static inline void __smp_call_function_single_interrupt(void)
 {
+	trace_ipi_entry(tracepoint_string("CALL_FUNCTION_SINGLE"));
 	generic_smp_call_function_single_interrupt();
 	inc_irq_stat(irq_call_count);
+	trace_ipi_exit(tracepoint_string("CALL_FUNCTION_SINGLE"));
 }
 
 __visible void smp_call_function_single_interrupt(struct pt_regs *regs)