| Message ID | 20140718172221.6dbe83e4@gandalf.local.home (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > >
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
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
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
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?
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
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
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;