[PATCH 0/4] basic IPI tracing

[PATCH 0/4] basic IPI tracing

[Nicolas Pitre]

Part of the energy aware scheduler work is concerned by test and
measurement tools such as idlestat[1].  This relies on the ability to
trace wake-up events, mainly IRQs and IPIs.

While IRQs are already well instrumented with tracepoints, IPIs are rather
lacking on that front, and completely invisible on ARM.

This series defines generic IPI tracepoints and adds them to ARM and
ARM64. An attempt at adding them to X86 is also included for comments.

[1] https://wiki.linaro.org/WorkingGroups/PowerManagement/Resources/Tools/Idlestat

diffstat:

 arch/arm/kernel/smp.c      | 72 ++++++++++++++++++++------------
 arch/arm64/kernel/smp.c    | 67 +++++++++++++++++++-----------
 arch/x86/kernel/smp.c      | 16 ++++++++
 include/trace/events/ipi.h | 89 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 192 insertions(+), 52 deletions(-)

[PATCH 1/4] tracepoint: add generic tracepoint definitions for IPI tracing

[Nicolas Pitre]

The Inter Processor Interrupt is used to make another processor do a
specific action such as rescheduling tasks, signal a timer event or
execute something in another CPU's context. IRQs are already traceable
but IPIs were not. Tracing them is useful for monitoring IPI latency,
or to verify when they are the source of CPU wake-ups with power
management implications.

Three trace hooks are defined: ipi_raise, ipi_entry and ipi_exit. To make
them portable, a string is used to identify them and correlate related
events. Additionally, ipi_raise records a bitmask representing targeted
CPUs.

Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
 include/trace/events/ipi.h | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 include/trace/events/ipi.h

diff --git a/include/trace/events/ipi.h b/include/trace/events/ipi.h
new file mode 100644
index 0000000000..834a7362a6
--- /dev/null
+++ b/include/trace/events/ipi.h
@@ -0,0 +1,89 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM ipi
+
+#if !defined(_TRACE_IPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_IPI_H
+
+#include <linux/tracepoint.h>
+
+/**
+ * ipi_raise - called when a smp cross call is made
+ *
+ * @mask: mask of recipient CPUs for the IPI
+ * @reason: string identifying the IPI purpose
+ *
+ * It is necessary for @reason to be a static string declared with
+ * __tracepoint_string.
+ */
+TRACE_EVENT(ipi_raise,
+
+	TP_PROTO(const struct cpumask *mask, const char *reason),
+
+	TP_ARGS(mask, reason),
+
+	TP_STRUCT__entry(
+		__bitmask(target_cpus, nr_cpumask_bits)
+		__field(const char *, reason)
+	),
+
+	TP_fast_assign(
+		__assign_bitmask(target_cpus, cpumask_bits(mask), nr_cpumask_bits);
+		__entry->reason = reason;
+	),
+
+	TP_printk("target_mask=%s (%s)", __get_bitmask(target_cpus), __entry->reason)
+);
+
+DECLARE_EVENT_CLASS(ipi_handler,
+
+	TP_PROTO(const char *reason),
+
+	TP_ARGS(reason),
+
+	TP_STRUCT__entry(
+		__field(const char *, reason)
+	),
+
+	TP_fast_assign(
+		__entry->reason = reason;
+	),
+
+	TP_printk("(%s)", __entry->reason)
+);
+
+/**
+ * ipi_entry - called immediately before the IPI handler
+ *
+ * @reason: string identifying the IPI purpose
+ *
+ * It is necessary for @reason to be a static string declared with
+ * __tracepoint_string, ideally the same as used with trace_ipi_raise
+ * for that IPI.
+ */
+DEFINE_EVENT(ipi_handler, ipi_entry,
+
+	TP_PROTO(const char *reason),
+
+	TP_ARGS(reason)
+);
+
+/**
+ * ipi_exit - called immediately after the IPI handler returns
+ *
+ * @reason: string identifying the IPI purpose
+ *
+ * It is necessary for @reason to be a static string declared with
+ * __tracepoint_string, ideally the same as used with trace_ipi_raise for
+ * that IPI.
+ */
+DEFINE_EVENT(ipi_handler, ipi_exit,
+
+	TP_PROTO(const char *reason),
+
+	TP_ARGS(reason)
+);
+
+#endif /* _TRACE_IPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
1.8.4.108.g55ea5f6

Re: [PATCH 1/4] tracepoint: add generic tracepoint definitions for IPI tracing

[Daniel Lezcano]

On 07/18/2014 07:18 AM, Nicolas Pitre wrote:
> The Inter Processor Interrupt is used to make another processor do a
> specific action such as rescheduling tasks, signal a timer event or
> execute something in another CPU's context. IRQs are already traceable
> but IPIs were not. Tracing them is useful for monitoring IPI latency,
> or to verify when they are the source of CPU wake-ups with power
> management implications.
>
> Three trace hooks are defined: ipi_raise, ipi_entry and ipi_exit. To make
> them portable, a string is used to identify them and correlate related
> events. Additionally, ipi_raise records a bitmask representing targeted
> CPUs.
>
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>



> ---
>   include/trace/events/ipi.h | 89 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 89 insertions(+)
>   create mode 100644 include/trace/events/ipi.h
>
> diff --git a/include/trace/events/ipi.h b/include/trace/events/ipi.h
> new file mode 100644
> index 0000000000..834a7362a6
> --- /dev/null
> +++ b/include/trace/events/ipi.h
> @@ -0,0 +1,89 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM ipi
> +
> +#if !defined(_TRACE_IPI_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_IPI_H
> +
> +#include <linux/tracepoint.h>
> +
> +/**
> + * ipi_raise - called when a smp cross call is made
> + *
> + * @mask: mask of recipient CPUs for the IPI
> + * @reason: string identifying the IPI purpose
> + *
> + * It is necessary for @reason to be a static string declared with
> + * __tracepoint_string.
> + */
> +TRACE_EVENT(ipi_raise,
> +
> +	TP_PROTO(const struct cpumask *mask, const char *reason),
> +
> +	TP_ARGS(mask, reason),
> +
> +	TP_STRUCT__entry(
> +		__bitmask(target_cpus, nr_cpumask_bits)
> +		__field(const char *, reason)
> +	),
> +
> +	TP_fast_assign(
> +		__assign_bitmask(target_cpus, cpumask_bits(mask), nr_cpumask_bits);
> +		__entry->reason = reason;
> +	),
> +
> +	TP_printk("target_mask=%s (%s)", __get_bitmask(target_cpus), __entry->reason)
> +);
> +
> +DECLARE_EVENT_CLASS(ipi_handler,
> +
> +	TP_PROTO(const char *reason),
> +
> +	TP_ARGS(reason),
> +
> +	TP_STRUCT__entry(
> +		__field(const char *, reason)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->reason = reason;
> +	),
> +
> +	TP_printk("(%s)", __entry->reason)
> +);
> +
> +/**
> + * ipi_entry - called immediately before the IPI handler
> + *
> + * @reason: string identifying the IPI purpose
> + *
> + * It is necessary for @reason to be a static string declared with
> + * __tracepoint_string, ideally the same as used with trace_ipi_raise
> + * for that IPI.
> + */
> +DEFINE_EVENT(ipi_handler, ipi_entry,
> +
> +	TP_PROTO(const char *reason),
> +
> +	TP_ARGS(reason)
> +);
> +
> +/**
> + * ipi_exit - called immediately after the IPI handler returns
> + *
> + * @reason: string identifying the IPI purpose
> + *
> + * It is necessary for @reason to be a static string declared with
> + * __tracepoint_string, ideally the same as used with trace_ipi_raise for
> + * that IPI.
> + */
> +DEFINE_EVENT(ipi_handler, ipi_exit,
> +
> +	TP_PROTO(const char *reason),
> +
> +	TP_ARGS(reason)
> +);
> +
> +#endif /* _TRACE_IPI_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH 2/4] ARM: add IPI tracepoints

[Nicolas Pitre]

The strings used to list IPIs in /proc/interrupts are reused for tracing
purposes.

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

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 7c4fada440..daedff319b 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -47,6 +47,9 @@
 #include <asm/mach/arch.h>
 #include <asm/mpu.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/ipi.h>
+
 /*
  * as from 2.5, kernels no longer have an init_tasks structure
  * so we need some other way of telling a new secondary core
@@ -430,38 +433,19 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static void (*smp_cross_call)(const struct cpumask *, unsigned int);
+static void (*__smp_cross_call)(const struct cpumask *, unsigned int);
 
 void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
 {
-	if (!smp_cross_call)
-		smp_cross_call = fn;
-}
-
-void arch_send_call_function_ipi_mask(const struct cpumask *mask)
-{
-	smp_cross_call(mask, IPI_CALL_FUNC);
-}
-
-void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
-{
-	smp_cross_call(mask, IPI_WAKEUP);
-}
-
-void arch_send_call_function_single_ipi(int cpu)
-{
-	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+	if (!__smp_cross_call)
+		__smp_cross_call = fn;
 }
 
-#ifdef CONFIG_IRQ_WORK
-void arch_irq_work_raise(void)
-{
-	if (is_smp())
-		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
-}
+static const char *ipi_types[NR_IPI]
+#ifdef CONFIG_TRACING
+__tracepoint_string
 #endif
-
-static const char *ipi_types[NR_IPI] = {
+= {
 #define S(x,s)	[x] = s
 	S(IPI_WAKEUP, "CPU wakeup interrupts"),
 	S(IPI_TIMER, "Timer broadcast interrupts"),
@@ -473,6 +457,12 @@ static const char *ipi_types[NR_IPI] = {
 	S(IPI_COMPLETION, "completion interrupts"),
 };
 
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
+{
+	trace_ipi_raise(target, ipi_types[ipinr]);
+	__smp_cross_call(target, ipinr);
+}
+
 void show_ipi_list(struct seq_file *p, int prec)
 {
 	unsigned int cpu, i;
@@ -499,6 +489,29 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
 	return sum;
 }
 
+void arch_send_call_function_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_CALL_FUNC);
+}
+
+void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_WAKEUP);
+}
+
+void arch_send_call_function_single_ipi(int cpu)
+{
+	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+}
+
+#ifdef CONFIG_IRQ_WORK
+void arch_irq_work_raise(void)
+{
+	if (is_smp())
+		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
+}
+#endif
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 void tick_broadcast(const struct cpumask *mask)
 {
@@ -556,8 +569,10 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
-	if (ipinr < NR_IPI)
+	if ((unsigned)ipinr < NR_IPI) {
+		trace_ipi_entry(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
+	}
 
 	switch (ipinr) {
 	case IPI_WAKEUP:
@@ -612,6 +627,9 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		       cpu, ipinr);
 		break;
 	}
+
+	if ((unsigned)ipinr < NR_IPI)
+		trace_ipi_exit(ipi_types[ipinr]);
 	set_irq_regs(old_regs);
 }
 
-- 
1.8.4.108.g55ea5f6

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Steven Rostedt]

On Fri, 18 Jul 2014 01:18:53 -0400
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

  
> -#ifdef CONFIG_IRQ_WORK
> -void arch_irq_work_raise(void)
> -{
> -	if (is_smp())
> -		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
> -}
> +static const char *ipi_types[NR_IPI]
> +#ifdef CONFIG_TRACING
> +__tracepoint_string
>  #endif

Oh, this is ugly. I should probably add a define when !CONFIG_TRACING
is set. Something like:

#ifdef CONFIG_TRACING
...
#else
# define __tracepoint_string
#endif

Such that users of __tracepoint_string don't need to add ugly ifdefs in
the code.

If you want to add that to ftrace_event.h to this series, I'll ack it.

-- Steve


> -
> -static const char *ipi_types[NR_IPI] = {
> += {
>  #define S(x,s)	[x] = s
>  	S(IPI_WAKEUP, "CPU wakeup interrupts"),
>  	S(IPI_TIMER, "Timer broadcast interrupts"),
> @@ -473,6 +457,12 @@ static const char *ipi_types[NR_IPI] = {
>  	S(IPI_COMPLETION, "completion interrupts"),
>  };
>  

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Nicolas Pitre]

On Fri, 18 Jul 2014, Steven Rostedt wrote:

> On Fri, 18 Jul 2014 01:18:53 -0400
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
>   
> > -#ifdef CONFIG_IRQ_WORK
> > -void arch_irq_work_raise(void)
> > -{
> > -	if (is_smp())
> > -		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
> > -}
> > +static const char *ipi_types[NR_IPI]
> > +#ifdef CONFIG_TRACING
> > +__tracepoint_string
> >  #endif
> 
> Oh, this is ugly. I should probably add a define when !CONFIG_TRACING
> is set. Something like:
> 
> #ifdef CONFIG_TRACING
> ...
> #else
> # define __tracepoint_string
> #endif
> 
> Such that users of __tracepoint_string don't need to add ugly ifdefs in
> the code.
> 
> If you want to add that to ftrace_event.h to this series, I'll ack it.

Here's the patch I have at the head of the series now, with the above
ugliness changed to an unconditional __tracepoint_string attribute.

From: Nicolas Pitre <nicolas.pitre@linaro.org>
Date: Fri, 18 Jul 2014 16:34:39 -0400
Subject: [PATCH] trace: don't refer __tracepoint_string to an undefined linker section

When CONFIG_TRACING is not set, the linker script doesn't specify any
__tracepoint_str section.  Let those __tracepoint_string marked strings
live in the default rodata section in that case.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index cff3106ffe..d6346607e4 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -606,7 +606,11 @@ do {									\
 		static const char *___tp_str __tracepoint_string = str; \
 		___tp_str;						\
 	})
+#ifdef CONFIG_TRACING
 #define __tracepoint_string	__attribute__((section("__tracepoint_str")))
+#else
+#define __tracepoint_string
+#endif
 
 #ifdef CONFIG_PERF_EVENTS
 struct perf_event;

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Steven Rostedt]

On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> 
> Here's the patch I have at the head of the series now, with the above
> ugliness changed to an unconditional __tracepoint_string attribute.
> 

I was thinking of something like this. Feel free to add this to your
series.

-- Steve

From: Steven Rostedt <rostedt@goodmis.org>
Subject: [PATCH] tracing: Do not do anything special with tracepoint_string when tracing is disabled

When CONFIG_TRACING is not enabled, there's no reason to save the trace
strings either by the linker or as a static variable that can be
referenced later. Simply pass back the string that is given to
tracepoint_string().

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index cff3106..b296363 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -574,6 +574,7 @@ do {									\
 		__trace_printk(ip, fmt, ##args);			\
 } while (0)
 
+#ifdef CONFIG_TRACING
 /**
  * tracepoint_string - register constant persistent string to trace system
  * @str - a constant persistent string that will be referenced in tracepoints
@@ -607,6 +608,15 @@ do {									\
 		___tp_str;						\
 	})
 #define __tracepoint_string	__attribute__((section("__tracepoint_str")))
+#else
+/*
+ * tracepoint_string() is used to save the string address for userspace
+ * tracing tools. When tracing isn't configured, there's no need to save
+ * anything.
+ */
+# define tracepoint_string(str) str
+# define __tracepoint_string
+#endif
 
 #ifdef CONFIG_PERF_EVENTS
 struct perf_event;

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Nicolas Pitre]

On Fri, 18 Jul 2014, Steven Rostedt wrote:

> On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> 
> > 
> > Here's the patch I have at the head of the series now, with the above
> > ugliness changed to an unconditional __tracepoint_string attribute.
> > 
> 
> I was thinking of something like this. Feel free to add this to your
> series.

OK.  Same end result, but much clearer.  Thanks.

Any comments / ACKs on the other patches?  I'd like to see 1/4 to 3/4 
(and your patch) merged upstream during the next window.  4/4 is up for 
debate.

> -- Steve
> 
> From: Steven Rostedt <rostedt@goodmis.org>
> Subject: [PATCH] tracing: Do not do anything special with tracepoint_string when tracing is disabled
> 
> When CONFIG_TRACING is not enabled, there's no reason to save the trace
> strings either by the linker or as a static variable that can be
> referenced later. Simply pass back the string that is given to
> tracepoint_string().
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index cff3106..b296363 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -574,6 +574,7 @@ do {									\
>  		__trace_printk(ip, fmt, ##args);			\
>  } while (0)
>  
> +#ifdef CONFIG_TRACING
>  /**
>   * tracepoint_string - register constant persistent string to trace system
>   * @str - a constant persistent string that will be referenced in tracepoints
> @@ -607,6 +608,15 @@ do {									\
>  		___tp_str;						\
>  	})
>  #define __tracepoint_string	__attribute__((section("__tracepoint_str")))
> +#else
> +/*
> + * tracepoint_string() is used to save the string address for userspace
> + * tracing tools. When tracing isn't configured, there's no need to save
> + * anything.
> + */
> +# define tracepoint_string(str) str
> +# define __tracepoint_string
> +#endif
>  
>  #ifdef CONFIG_PERF_EVENTS
>  struct perf_event;
> 
> 

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Steven Rostedt]

On Fri, 18 Jul 2014 22:55:12 -0400 (EDT)
Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> Any comments / ACKs on the other patches?  I'd like to see 1/4 to 3/4 
> (and your patch) merged upstream during the next window.  4/4 is up for 
> debate.

You can add my Acked-by for patches 1,2 and 3, after the clean up of
the #ifdefs there.

I still don't like the fact that you need to add the #undef in patch 4.
I'm looking at other ways to fix that. I tried to do a few different
clean ups in the tracing infrastructure, but it seems that it may be
required. The other method I may try is to move the #undefs into the
ipi.h header itself.

I'll play more with this on Monday.

-- Steve

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Ard Biesheuvel]

On 18 July 2014 23:22, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
> Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>
>>
>> Here's the patch I have at the head of the series now, with the above
>> ugliness changed to an unconditional __tracepoint_string attribute.
>>
>
> I was thinking of something like this. Feel free to add this to your
> series.
>
> -- Steve
>

Nico,

If this patch addresses the issue where 3 RCU related tracepoint
strings turn up /after/ _edata on !CONFIG_TRACING, there is already a
patch queued up here

http://marc.info/?l=linux-kernel&m=140518452623148&w=2

As far as In know, these were the only occurrences using a __used
modifier, which is why they weren't dropped by the compiler in the
!CONFIG_TRACING case.

Regards,
Ard.



> From: Steven Rostedt <rostedt@goodmis.org>
> Subject: [PATCH] tracing: Do not do anything special with tracepoint_string when tracing is disabled
>
> When CONFIG_TRACING is not enabled, there's no reason to save the trace
> strings either by the linker or as a static variable that can be
> referenced later. Simply pass back the string that is given to
> tracepoint_string().
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
> index cff3106..b296363 100644
> --- a/include/linux/ftrace_event.h
> +++ b/include/linux/ftrace_event.h
> @@ -574,6 +574,7 @@ do {                                                                        \
>                 __trace_printk(ip, fmt, ##args);                        \
>  } while (0)
>
> +#ifdef CONFIG_TRACING
>  /**
>   * tracepoint_string - register constant persistent string to trace system
>   * @str - a constant persistent string that will be referenced in tracepoints
> @@ -607,6 +608,15 @@ do {                                                                       \
>                 ___tp_str;                                              \
>         })
>  #define __tracepoint_string    __attribute__((section("__tracepoint_str")))
> +#else
> +/*
> + * tracepoint_string() is used to save the string address for userspace
> + * tracing tools. When tracing isn't configured, there's no need to save
> + * anything.
> + */
> +# define tracepoint_string(str) str
> +# define __tracepoint_string
> +#endif
>
>  #ifdef CONFIG_PERF_EVENTS
>  struct perf_event;
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Steven Rostedt]

On Sat, 19 Jul 2014 21:10:37 +0200
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 18 July 2014 23:22, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
> > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> >>
> >> Here's the patch I have at the head of the series now, with the above
> >> ugliness changed to an unconditional __tracepoint_string attribute.
> >>
> >
> > I was thinking of something like this. Feel free to add this to your
> > series.
> >
> > -- Steve
> >
> 
> Nico,
> 
> If this patch addresses the issue where 3 RCU related tracepoint
> strings turn up /after/ _edata on !CONFIG_TRACING, there is already a
> patch queued up here
> 
> http://marc.info/?l=linux-kernel&m=140518452623148&w=2
> 
> As far as In know, these were the only occurrences using a __used
> modifier, which is why they weren't dropped by the compiler in the
> !CONFIG_TRACING case.
> 

Ard,

Similar but different problem. Nicolas's problem was with new use cases
for tracepoint_string. My patch fixes the issue for the general case.

-- Steve

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Ard Biesheuvel]

On 19 July 2014 22:28, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sat, 19 Jul 2014 21:10:37 +0200
> Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
>> On 18 July 2014 23:22, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
>> > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>> >
>> >>
>> >> Here's the patch I have at the head of the series now, with the above
>> >> ugliness changed to an unconditional __tracepoint_string attribute.
>> >>
>> >
>> > I was thinking of something like this. Feel free to add this to your
>> > series.
>> >
>> > -- Steve
>> >
>>
>> Nico,
>>
>> If this patch addresses the issue where 3 RCU related tracepoint
>> strings turn up /after/ _edata on !CONFIG_TRACING, there is already a
>> patch queued up here
>>
>> http://marc.info/?l=linux-kernel&m=140518452623148&w=2
>>
>> As far as In know, these were the only occurrences using a __used
>> modifier, which is why they weren't dropped by the compiler in the
>> !CONFIG_TRACING case.
>>
>
> Ard,
>
> Similar but different problem. Nicolas's problem was with new use cases
> for tracepoint_string. My patch fixes the issue for the general case.
>

OK, so if the general case has been fixed, perhaps we should ask Paul
to drop my patch?

-- 
Ard.

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Steven Rostedt]

On Sat, 19 Jul 2014 22:50:16 +0200
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

 
> OK, so if the general case has been fixed, perhaps we should ask Paul
> to drop my patch?
> 

No, for a few reasons. One, this patch still needs to get in to fix the
problem for RCU. Two, RCU basically open codes the creation of the
string, thus this wont solve it for RCU.

-- Steve

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Nicolas Pitre]

On Sat, 19 Jul 2014, Ard Biesheuvel wrote:

> On 18 July 2014 23:22, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Fri, 18 Jul 2014 16:55:42 -0400 (EDT)
> > Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> >
> >>
> >> Here's the patch I have at the head of the series now, with the above
> >> ugliness changed to an unconditional __tracepoint_string attribute.
> >>
> >
> > I was thinking of something like this. Feel free to add this to your
> > series.
> >
> > -- Steve
> >
> 
> Nico,
> 
> If this patch addresses the issue where 3 RCU related tracepoint
> strings turn up /after/ _edata on !CONFIG_TRACING, there is already a
> patch queued up here
> 
> http://marc.info/?l=linux-kernel&m=140518452623148&w=2

No, that doesn't help my case.  Please see the initial comment from 
Steven in this thread and you'll understand.


Nicolas

Re: [PATCH 2/4] ARM: add IPI tracepoints

[Nicolas Pitre]

@Russell: Can I have your ACK on this patch before I send the series 
upstream?  Here's the revised version accounting for the changes the 
discussion in this thread brought about

Author: Nicolas Pitre <nicolas.pitre@linaro.org>
Date:   Fri Jul 18 00:07:05 2014 -0400

    ARM: add IPI tracepoints
    
    The strings used to list IPIs in /proc/interrupts are reused for tracing
    purposes.
    
    While at it, prevent a negative ipinr from escaping the range check
    in handle_IPI().
    
    Signed-off-by: Nicolas Pitre <nico@linaro.org>
    Acked-by: Steven Rostedt <rostedt@goodmis.org>

diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 7c4fada440..9388a3d479 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -47,6 +47,9 @@
 #include <asm/mach/arch.h>
 #include <asm/mpu.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/ipi.h>
+
 /*
  * as from 2.5, kernels no longer have an init_tasks structure
  * so we need some other way of telling a new secondary core
@@ -430,38 +433,15 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
-static void (*smp_cross_call)(const struct cpumask *, unsigned int);
+static void (*__smp_cross_call)(const struct cpumask *, unsigned int);
 
 void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
 {
-	if (!smp_cross_call)
-		smp_cross_call = fn;
-}
-
-void arch_send_call_function_ipi_mask(const struct cpumask *mask)
-{
-	smp_cross_call(mask, IPI_CALL_FUNC);
-}
-
-void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
-{
-	smp_cross_call(mask, IPI_WAKEUP);
-}
-
-void arch_send_call_function_single_ipi(int cpu)
-{
-	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+	if (!__smp_cross_call)
+		__smp_cross_call = fn;
 }
 
-#ifdef CONFIG_IRQ_WORK
-void arch_irq_work_raise(void)
-{
-	if (is_smp())
-		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
-}
-#endif
-
-static const char *ipi_types[NR_IPI] = {
+static const char *ipi_types[NR_IPI] __tracepoint_string = {
 #define S(x,s)	[x] = s
 	S(IPI_WAKEUP, "CPU wakeup interrupts"),
 	S(IPI_TIMER, "Timer broadcast interrupts"),
@@ -473,6 +453,12 @@ static const char *ipi_types[NR_IPI] = {
 	S(IPI_COMPLETION, "completion interrupts"),
 };
 
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
+{
+	trace_ipi_raise(target, ipi_types[ipinr]);
+	__smp_cross_call(target, ipinr);
+}
+
 void show_ipi_list(struct seq_file *p, int prec)
 {
 	unsigned int cpu, i;
@@ -499,6 +485,29 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
 	return sum;
 }
 
+void arch_send_call_function_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_CALL_FUNC);
+}
+
+void arch_send_wakeup_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_WAKEUP);
+}
+
+void arch_send_call_function_single_ipi(int cpu)
+{
+	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+}
+
+#ifdef CONFIG_IRQ_WORK
+void arch_irq_work_raise(void)
+{
+	if (is_smp())
+		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
+}
+#endif
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 void tick_broadcast(const struct cpumask *mask)
 {
@@ -556,8 +565,10 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
-	if (ipinr < NR_IPI)
+	if ((unsigned)ipinr < NR_IPI) {
+		trace_ipi_entry(ipi_types[ipinr]);
 		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
+	}
 
 	switch (ipinr) {
 	case IPI_WAKEUP:
@@ -612,6 +623,9 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		       cpu, ipinr);
 		break;
 	}
+
+	if ((unsigned)ipinr < NR_IPI)
+		trace_ipi_exit(ipi_types[ipinr]);
 	set_irq_regs(old_regs);
 }
 

[PATCH 3/4] ARM64: add IPI tracepoints

[Nicolas Pitre]

The strings used to list IPIs in /proc/interrupts are reused for tracing
purposes.

While at it, the code is slightly cleaned up so the ipi_types array
indices are no longer offset by IPI_RESCHEDULE whose value is 0 anyway.

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

diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 40f38f46c8..78a5904994 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -50,6 +50,9 @@
 #include <asm/tlbflush.h>
 #include <asm/ptrace.h>
 
+#define CREATE_TRACE_POINTS
+#include <trace/events/ipi.h>
+
 /*
  * as from 2.5, kernels no longer have an init_tasks structure
  * so we need some other way of telling a new secondary core
@@ -307,8 +310,6 @@ void __init smp_prepare_boot_cpu(void)
 	set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
 }
 
-static void (*smp_cross_call)(const struct cpumask *, unsigned int);
-
 /*
  * Enumerate the possible CPU set from the device tree and build the
  * cpu logical map array containing MPIDR values related to logical
@@ -463,32 +464,19 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
 	}
 }
 
+static void (*__smp_cross_call)(const struct cpumask *, unsigned int);
 
 void __init set_smp_cross_call(void (*fn)(const struct cpumask *, unsigned int))
 {
-	smp_cross_call = fn;
-}
-
-void arch_send_call_function_ipi_mask(const struct cpumask *mask)
-{
-	smp_cross_call(mask, IPI_CALL_FUNC);
-}
-
-void arch_send_call_function_single_ipi(int cpu)
-{
-	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+	__smp_cross_call = fn;
 }
 
-#ifdef CONFIG_IRQ_WORK
-void arch_irq_work_raise(void)
-{
-	if (smp_cross_call)
-		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
-}
+static const char *ipi_types[NR_IPI] 
+#ifdef CONFIG_TRACING
+__tracepoint_string
 #endif
-
-static const char *ipi_types[NR_IPI] = {
-#define S(x,s)	[x - IPI_RESCHEDULE] = s
+= {
+#define S(x,s)	[x] = s
 	S(IPI_RESCHEDULE, "Rescheduling interrupts"),
 	S(IPI_CALL_FUNC, "Function call interrupts"),
 	S(IPI_CALL_FUNC_SINGLE, "Single function call interrupts"),
@@ -497,12 +485,18 @@ static const char *ipi_types[NR_IPI] = {
 	S(IPI_IRQ_WORK, "IRQ work interrupts"),
 };
 
+static void smp_cross_call(const struct cpumask *target, unsigned int ipinr)
+{
+	trace_ipi_raise(target, ipi_types[ipinr]);
+	__smp_cross_call(target, ipinr);
+}
+
 void show_ipi_list(struct seq_file *p, int prec)
 {
 	unsigned int cpu, i;
 
 	for (i = 0; i < NR_IPI; i++) {
-		seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i + IPI_RESCHEDULE,
+		seq_printf(p, "%*s%u:%s", prec - 1, "IPI", i,
 			   prec >= 4 ? " " : "");
 		for_each_online_cpu(cpu)
 			seq_printf(p, "%10u ",
@@ -522,6 +516,24 @@ u64 smp_irq_stat_cpu(unsigned int cpu)
 	return sum;
 }
 
+void arch_send_call_function_ipi_mask(const struct cpumask *mask)
+{
+	smp_cross_call(mask, IPI_CALL_FUNC);
+}
+
+void arch_send_call_function_single_ipi(int cpu)
+{
+	smp_cross_call(cpumask_of(cpu), IPI_CALL_FUNC_SINGLE);
+}
+
+#ifdef CONFIG_IRQ_WORK
+void arch_irq_work_raise(void)
+{
+	if (__smp_cross_call)
+		smp_cross_call(cpumask_of(smp_processor_id()), IPI_IRQ_WORK);
+}
+#endif
+
 static DEFINE_RAW_SPINLOCK(stop_lock);
 
 /*
@@ -553,8 +565,10 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 	unsigned int cpu = smp_processor_id();
 	struct pt_regs *old_regs = set_irq_regs(regs);
 
-	if (ipinr >= IPI_RESCHEDULE && ipinr < IPI_RESCHEDULE + NR_IPI)
-		__inc_irq_stat(cpu, ipi_irqs[ipinr - IPI_RESCHEDULE]);
+	if ((unsigned)ipinr < NR_IPI) {
+		trace_ipi_entry(ipi_types[ipinr]);
+		__inc_irq_stat(cpu, ipi_irqs[ipinr]);
+	}
 
 	switch (ipinr) {
 	case IPI_RESCHEDULE:
@@ -599,6 +613,9 @@ void handle_IPI(int ipinr, struct pt_regs *regs)
 		pr_crit("CPU%u: Unknown IPI message 0x%x\n", cpu, ipinr);
 		break;
 	}
+
+	if ((unsigned)ipinr < NR_IPI)
+		trace_ipi_exit(ipi_types[ipinr]);
 	set_irq_regs(old_regs);
 }
 
-- 
1.8.4.108.g55ea5f6

Re: [PATCH 3/4] ARM64: add IPI tracepoints

[Catalin Marinas]

On Fri, Jul 18, 2014 at 06:18:54AM +0100, Nicolas Pitre wrote:
> The strings used to list IPIs in /proc/interrupts are reused for tracing
> purposes.
> 
> While at it, the code is slightly cleaned up so the ipi_types array
> indices are no longer offset by IPI_RESCHEDULE whose value is 0 anyway.
> 
> Signed-off-by: Nicolas Pitre <nico@linaro.org>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

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

[Nicolas Pitre]

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(+)

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)
-- 
1.8.4.108.g55ea5f6

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

[Steven Rostedt]

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:
>   *

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

[Nicolas Pitre]

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:
> >   *
> 
> 

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

[Steven Rostedt]

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

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

[Nicolas Pitre]

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

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

[Davidlohr Bueso]

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

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

[Nicolas Pitre]

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