[2/4] ARM: add IPI tracepoints

Message ID alpine.LFD.2.11.1407181652080.3647@knanqh.ubzr
State New
Headers show

Commit Message

Nicolas Pitre July 18, 2014, 8:55 p.m.
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>

--
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/

Comments

Nicolas Pitre July 19, 2014, 2:55 a.m. | #1
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;
> 
> 
--
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 19, 2014, 3:30 a.m. | #2
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
--
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/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;