* [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
@ 2014-07-11 11:38 Ard Biesheuvel
2014-07-11 15:12 ` Paul E. McKenney
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-11 11:38 UTC (permalink / raw)
To: linux-kernel; +Cc: Ard Biesheuvel, rostedt, dipankar, paulmck
Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
infrastructure") unconditionally populates the __tracepoint_str input section,
but this section is not assigned an output section if CONFIG_TRACING is not set.
This results in the __tracepoint_str turning up in unexpected places, i.e.,
after _edata.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: rostedt@goodmis.org
Cc: dipankar@in.ibm.com
Cc: paulmck@linux.vnet.ibm.com
---
kernel/rcu/tree.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index f1ba77363fbb..ac1984149eb5 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
* the tracing userspace tools to be able to decipher the string
* address to the matching string.
*/
+#ifdef CONFIG_TRACING
+#define DEFINE_TPS(sname) \
+static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
+#else
+#define DEFINE_TPS(sname)
+#endif
+
#define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
static char sname##_varname[] = #sname; \
-static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
+DEFINE_TPS(sname) \
struct rcu_state sname##_state = { \
.level = { &sname##_state.node[0] }, \
.call = cr, \
--
1.8.3.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
2014-07-11 11:38 [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set Ard Biesheuvel
@ 2014-07-11 15:12 ` Paul E. McKenney
2014-07-11 15:30 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Paul E. McKenney @ 2014-07-11 15:12 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-kernel, rostedt, dipankar
On Fri, Jul 11, 2014 at 01:38:12PM +0200, Ard Biesheuvel wrote:
> Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
> infrastructure") unconditionally populates the __tracepoint_str input section,
> but this section is not assigned an output section if CONFIG_TRACING is not set.
> This results in the __tracepoint_str turning up in unexpected places, i.e.,
> after _edata.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: rostedt@goodmis.org
> Cc: dipankar@in.ibm.com
> Cc: paulmck@linux.vnet.ibm.com
If you get a Reviewed-by from Steven Rostedt, I will be happy to queue
this one.
Thanx, Paul
> ---
> kernel/rcu/tree.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index f1ba77363fbb..ac1984149eb5 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
> * the tracing userspace tools to be able to decipher the string
> * address to the matching string.
> */
> +#ifdef CONFIG_TRACING
> +#define DEFINE_TPS(sname) \
> +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
> +#else
> +#define DEFINE_TPS(sname)
> +#endif
> +
> #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
> static char sname##_varname[] = #sname; \
> -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
> +DEFINE_TPS(sname) \
> struct rcu_state sname##_state = { \
> .level = { &sname##_state.node[0] }, \
> .call = cr, \
> --
> 1.8.3.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
2014-07-11 15:12 ` Paul E. McKenney
@ 2014-07-11 15:30 ` Steven Rostedt
2014-07-11 15:40 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2014-07-11 15:30 UTC (permalink / raw)
To: paulmck; +Cc: Ard Biesheuvel, linux-kernel, dipankar
On Fri, 11 Jul 2014 08:12:40 -0700
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> On Fri, Jul 11, 2014 at 01:38:12PM +0200, Ard Biesheuvel wrote:
> > Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
> > infrastructure") unconditionally populates the __tracepoint_str input section,
> > but this section is not assigned an output section if CONFIG_TRACING is not set.
> > This results in the __tracepoint_str turning up in unexpected places, i.e.,
> > after _edata.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: rostedt@goodmis.org
> > Cc: dipankar@in.ibm.com
> > Cc: paulmck@linux.vnet.ibm.com
>
> If you get a Reviewed-by from Steven Rostedt, I will be happy to queue
> this one.
I'm fine with it, but it should add a comment.
>
> Thanx, Paul
>
> > ---
> > kernel/rcu/tree.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index f1ba77363fbb..ac1984149eb5 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
> > * the tracing userspace tools to be able to decipher the string
> > * address to the matching string.
> > */
> > +#ifdef CONFIG_TRACING
/*
* When tracing is enabled, DEFINE_TPS() will export the string to
* userspace via the tracing debugfs directory. This allows userspace
* tools to read the binary tracepoints that reference the pointer
* to the string and not the string itself.
*/
> > +#define DEFINE_TPS(sname) \
> > +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
> > +#else
> > +#define DEFINE_TPS(sname)
> > +#endif
Also, perhaps we should call it:
DEFINE_RCU_TPS()
Other than that, I'm fine with the patch.
I'll review v2 ;-)
-- Steve
> > +
> > #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
> > static char sname##_varname[] = #sname; \
> > -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
> > +DEFINE_TPS(sname) \
> > struct rcu_state sname##_state = { \
> > .level = { &sname##_state.node[0] }, \
> > .call = cr, \
> > --
> > 1.8.3.2
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
2014-07-11 15:30 ` Steven Rostedt
@ 2014-07-11 15:40 ` Ard Biesheuvel
2014-07-11 18:09 ` Ard Biesheuvel
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-11 15:40 UTC (permalink / raw)
To: Steven Rostedt; +Cc: paulmck, linux-kernel, dipankar
On 11 July 2014 17:30, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 11 Jul 2014 08:12:40 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
>> On Fri, Jul 11, 2014 at 01:38:12PM +0200, Ard Biesheuvel wrote:
>> > Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
>> > infrastructure") unconditionally populates the __tracepoint_str input section,
>> > but this section is not assigned an output section if CONFIG_TRACING is not set.
>> > This results in the __tracepoint_str turning up in unexpected places, i.e.,
>> > after _edata.
>> >
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: rostedt@goodmis.org
>> > Cc: dipankar@in.ibm.com
>> > Cc: paulmck@linux.vnet.ibm.com
>>
>> If you get a Reviewed-by from Steven Rostedt, I will be happy to queue
>> this one.
>
> I'm fine with it, but it should add a comment.
>
>
>>
>> Thanx, Paul
>>
>> > ---
>> > kernel/rcu/tree.c | 9 ++++++++-
>> > 1 file changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>> > index f1ba77363fbb..ac1984149eb5 100644
>> > --- a/kernel/rcu/tree.c
>> > +++ b/kernel/rcu/tree.c
>> > @@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
>> > * the tracing userspace tools to be able to decipher the string
>> > * address to the matching string.
>> > */
>> > +#ifdef CONFIG_TRACING
>
> /*
> * When tracing is enabled, DEFINE_TPS() will export the string to
> * userspace via the tracing debugfs directory. This allows userspace
> * tools to read the binary tracepoints that reference the pointer
> * to the string and not the string itself.
> */
>
>
>> > +#define DEFINE_TPS(sname) \
>> > +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
>> > +#else
>> > +#define DEFINE_TPS(sname)
>> > +#endif
>
> Also, perhaps we should call it:
>
> DEFINE_RCU_TPS()
>
> Other than that, I'm fine with the patch.
>
> I'll review v2 ;-)
>
Thanks gents, v2 coming up.
--
Ard.
>> > +
>> > #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
>> > static char sname##_varname[] = #sname; \
>> > -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
>> > +DEFINE_TPS(sname) \
>> > struct rcu_state sname##_state = { \
>> > .level = { &sname##_state.node[0] }, \
>> > .call = cr, \
>> > --
>> > 1.8.3.2
>> >
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
2014-07-11 15:40 ` Ard Biesheuvel
@ 2014-07-11 18:09 ` Ard Biesheuvel
2014-07-11 18:36 ` Steven Rostedt
0 siblings, 1 reply; 6+ messages in thread
From: Ard Biesheuvel @ 2014-07-11 18:09 UTC (permalink / raw)
To: Steven Rostedt; +Cc: paulmck, linux-kernel, dipankar
On 11 July 2014 17:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 11 July 2014 17:30, Steven Rostedt <rostedt@goodmis.org> wrote:
>> On Fri, 11 Jul 2014 08:12:40 -0700
>> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>>
>>> On Fri, Jul 11, 2014 at 01:38:12PM +0200, Ard Biesheuvel wrote:
>>> > Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
>>> > infrastructure") unconditionally populates the __tracepoint_str input section,
>>> > but this section is not assigned an output section if CONFIG_TRACING is not set.
>>> > This results in the __tracepoint_str turning up in unexpected places, i.e.,
>>> > after _edata.
>>> >
>>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> > Cc: rostedt@goodmis.org
>>> > Cc: dipankar@in.ibm.com
>>> > Cc: paulmck@linux.vnet.ibm.com
>>>
>>> If you get a Reviewed-by from Steven Rostedt, I will be happy to queue
>>> this one.
>>
>> I'm fine with it, but it should add a comment.
>>
>>
>>>
>>> Thanx, Paul
>>>
>>> > ---
>>> > kernel/rcu/tree.c | 9 ++++++++-
>>> > 1 file changed, 8 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
>>> > index f1ba77363fbb..ac1984149eb5 100644
>>> > --- a/kernel/rcu/tree.c
>>> > +++ b/kernel/rcu/tree.c
>>> > @@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
>>> > * the tracing userspace tools to be able to decipher the string
>>> > * address to the matching string.
>>> > */
>>> > +#ifdef CONFIG_TRACING
>>
>> /*
>> * When tracing is enabled, DEFINE_TPS() will export the string to
>> * userspace via the tracing debugfs directory. This allows userspace
>> * tools to read the binary tracepoints that reference the pointer
>> * to the string and not the string itself.
>> */
>>
>>
Ehm, actually, a comment to that effect is already right there in the file.
Would you still like me to add this additional comment?
>>> > +#define DEFINE_TPS(sname) \
>>> > +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
>>> > +#else
>>> > +#define DEFINE_TPS(sname)
>>> > +#endif
>>
>> Also, perhaps we should call it:
>>
>> DEFINE_RCU_TPS()
>>
>> Other than that, I'm fine with the patch.
>>
>> I'll review v2 ;-)
>>
>
> Thanks gents, v2 coming up.
>
> --
> Ard.
>
>
>>> > +
>>> > #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
>>> > static char sname##_varname[] = #sname; \
>>> > -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
>>> > +DEFINE_TPS(sname) \
>>> > struct rcu_state sname##_state = { \
>>> > .level = { &sname##_state.node[0] }, \
>>> > .call = cr, \
>>> > --
>>> > 1.8.3.2
>>> >
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set
2014-07-11 18:09 ` Ard Biesheuvel
@ 2014-07-11 18:36 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2014-07-11 18:36 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: paulmck, linux-kernel, dipankar
On Fri, 11 Jul 2014 20:09:26 +0200
Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 11 July 2014 17:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 11 July 2014 17:30, Steven Rostedt <rostedt@goodmis.org> wrote:
> >> On Fri, 11 Jul 2014 08:12:40 -0700
> >> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
> >>
> >>> On Fri, Jul 11, 2014 at 01:38:12PM +0200, Ard Biesheuvel wrote:
> >>> > Commit f7f7bac9cb1c ("rcu: Have the RCU tracepoints use the tracepoint_string
> >>> > infrastructure") unconditionally populates the __tracepoint_str input section,
> >>> > but this section is not assigned an output section if CONFIG_TRACING is not set.
> >>> > This results in the __tracepoint_str turning up in unexpected places, i.e.,
> >>> > after _edata.
> >>> >
> >>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>> > Cc: rostedt@goodmis.org
> >>> > Cc: dipankar@in.ibm.com
> >>> > Cc: paulmck@linux.vnet.ibm.com
> >>>
> >>> If you get a Reviewed-by from Steven Rostedt, I will be happy to queue
> >>> this one.
> >>
> >> I'm fine with it, but it should add a comment.
> >>
> >>
> >>>
> >>> Thanx, Paul
> >>>
> >>> > ---
> >>> > kernel/rcu/tree.c | 9 ++++++++-
> >>> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >>> >
> >>> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> >>> > index f1ba77363fbb..ac1984149eb5 100644
> >>> > --- a/kernel/rcu/tree.c
> >>> > +++ b/kernel/rcu/tree.c
> >>> > @@ -79,9 +79,16 @@ static struct lock_class_key rcu_fqs_class[RCU_NUM_LVLS];
> >>> > * the tracing userspace tools to be able to decipher the string
> >>> > * address to the matching string.
> >>> > */
> >>> > +#ifdef CONFIG_TRACING
> >>
> >> /*
> >> * When tracing is enabled, DEFINE_TPS() will export the string to
> >> * userspace via the tracing debugfs directory. This allows userspace
> >> * tools to read the binary tracepoints that reference the pointer
> >> * to the string and not the string itself.
> >> */
> >>
> >>
>
> Ehm, actually, a comment to that effect is already right there in the file.
> Would you still like me to add this additional comment?
Heh, I surprised myself with my own documentation :-)
No, I think the existing comment is good enough. Just rename the macro
then.
Thanks!
-- Steve
>
> >>> > +#define DEFINE_TPS(sname) \
> >>> > +static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname;
> >>> > +#else
> >>> > +#define DEFINE_TPS(sname)
> >>> > +#endif
> >>
> >> Also, perhaps we should call it:
> >>
> >> DEFINE_RCU_TPS()
> >>
> >> Other than that, I'm fine with the patch.
> >>
> >> I'll review v2 ;-)
> >>
> >
> > Thanks gents, v2 coming up.
> >
> > --
> > Ard.
> >
> >
> >>> > +
> >>> > #define RCU_STATE_INITIALIZER(sname, sabbr, cr) \
> >>> > static char sname##_varname[] = #sname; \
> >>> > -static const char *tp_##sname##_varname __used __tracepoint_string = sname##_varname; \
> >>> > +DEFINE_TPS(sname) \
> >>> > struct rcu_state sname##_state = { \
> >>> > .level = { &sname##_state.node[0] }, \
> >>> > .call = cr, \
> >>> > --
> >>> > 1.8.3.2
> >>> >
> >>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-07-11 18:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-11 11:38 [PATCH] rcu: define tracepoint strings only if CONFIG_TRACING is set Ard Biesheuvel
2014-07-11 15:12 ` Paul E. McKenney
2014-07-11 15:30 ` Steven Rostedt
2014-07-11 15:40 ` Ard Biesheuvel
2014-07-11 18:09 ` Ard Biesheuvel
2014-07-11 18:36 ` Steven Rostedt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®