For some reason the export of the event print
format to userspace uses '#fmt' which breaks
if the format string is anything but a plain
string, for example if it is built with macros
then the macro names are exported instead of
their contents.
Use
"\"%s\"", fmt
instead of
"%s", #fmt
to export the string and not the way it is built.
Signed-off-by: Johannes Berg <[email protected]>
---
This is making the export of a bunch of events (only checked the
wireless ones but those are all affected) unusable, so please apply to
2.6.32.
include/trace/ftrace.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- wireless-testing.orig/include/trace/ftrace.h 2009-11-13 13:15:21.000000000 +0100
+++ wireless-testing/include/trace/ftrace.h 2009-11-13 13:34:03.000000000 +0100
@@ -159,7 +159,7 @@
#undef __get_str
#undef TP_printk
-#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
+#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
#undef TP_fast_assign
#define TP_fast_assign(args...) args
Johannes Berg <[email protected]> writes:
> --- wireless-testing.orig/include/trace/ftrace.h 2009-11-13 13:15:21.000000000 +0100
> +++ wireless-testing/include/trace/ftrace.h 2009-11-13 13:34:03.000000000 +0100
> @@ -159,7 +159,7 @@
> #undef __get_str
>
> #undef TP_printk
> -#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
> +#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
Would using __stringify(fmt) work? If there are double quote characters
in fmt your solution would produce output that is ambiguous.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84 5EC7 45C6 250E 6F00 984E
"And now for something completely different."
On Fri, 2009-11-13 at 14:22 +0100, Andreas Schwab wrote:
> Johannes Berg <[email protected]> writes:
>
> > --- wireless-testing.orig/include/trace/ftrace.h 2009-11-13 13:15:21.000000000 +0100
> > +++ wireless-testing/include/trace/ftrace.h 2009-11-13 13:34:03.000000000 +0100
> > @@ -159,7 +159,7 @@
> > #undef __get_str
> >
> > #undef TP_printk
> > -#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
> > +#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
>
> Would using __stringify(fmt) work? If there are double quote characters
> in fmt your solution would produce output that is ambiguous.
No, for say
#define FOO_FMT "foo:%d"
and using
FOO_FMT ", %d"
that would give
""%s", %d"
instead of
"%s, %d"
However, isn't it already ambiguous that way? I fail to see why %s, #fmt
would preserve inside " properly.
johannes
On Fri, 2009-11-13 at 14:29 +0100, Johannes Berg wrote:
> > > -#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
> > > +#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
> >
> > Would using __stringify(fmt) work? If there are double quote characters
> > in fmt your solution would produce output that is ambiguous.
> However, isn't it already ambiguous that way? I fail to see why %s, #fmt
> would preserve inside " properly.
Never mind, I missed one indirection, so yes, it would preserve that. I
guess I'll have to somehow escape the quotes when copying them out to
userspace.
johannes
( extended the Cc:s. Please try to Cc: people you think would be
interested in your patches. )
* Johannes Berg <[email protected]> wrote:
> For some reason the export of the event print
> format to userspace uses '#fmt' which breaks
> if the format string is anything but a plain
> string, for example if it is built with macros
> then the macro names are exported instead of
> their contents.
>
> Use
> "\"%s\"", fmt
> instead of
> "%s", #fmt
> to export the string and not the way it is built.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> This is making the export of a bunch of events (only checked the
> wireless ones but those are all affected) unusable, so please apply to
> 2.6.32.
>
> include/trace/ftrace.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- wireless-testing.orig/include/trace/ftrace.h 2009-11-13 13:15:21.000000000 +0100
> +++ wireless-testing/include/trace/ftrace.h 2009-11-13 13:34:03.000000000 +0100
> @@ -159,7 +159,7 @@
> #undef __get_str
>
> #undef TP_printk
> -#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
> +#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
>
> #undef TP_fast_assign
> #define TP_fast_assign(args...) args
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, 2009-11-13 at 23:40 +0100, Ingo Molnar wrote:
> ( extended the Cc:s. Please try to Cc: people you think would be
> interested in your patches. )
Thanks Ingo.
>
> * Johannes Berg <[email protected]> wrote:
>
> > For some reason the export of the event print
> > format to userspace uses '#fmt' which breaks
> > if the format string is anything but a plain
> > string, for example if it is built with macros
> > then the macro names are exported instead of
> > their contents.
> >
> > Use
> > "\"%s\"", fmt
> > instead of
> > "%s", #fmt
> > to export the string and not the way it is built.
Is there any examples of this currently in the kernel? If so could you
show that in the changlog.
But other than that, the change looks good.
Acked-by: Steven Rostedt <[email protected]>
-- Steve
> >
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
Ingo,
Please pull the latest tip/tracing/urgent tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/tracing/urgent
Johannes Berg (1):
tracing: Fix event format export
----
include/trace/ftrace.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
---------------------------
commit 811cb50baf63461ce0bdb234927046131fc7fa8b
Author: Johannes Berg <[email protected]>
Date: Fri Nov 13 23:40:09 2009 +0100
tracing: Fix event format export
For some reason the export of the event print format to userspace
uses '#fmt' which breaks if the format string is anything but a plain
string, for example if it is built with macros then the macro names
are exported instead of their contents.
Use
"\"%s\"", fmt
instead of
"%s", #fmt
to export the string and not the way it is built.
For example, in net/mac80211/driver-trace.h for the trace event drv_start
there is:
TP_printk(
LOCAL_PR_FMT, LOCAL_PR_ARG
)
Which use to produce:
print fmt: LOCAL_PR_FMT, REC->wiphy_name
Now produces:
print fmt: "%s", REC->wiphy_name
Signed-off-by: Johannes Berg <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index cc0d966..dacb8ef 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -159,7 +159,7 @@
#undef __get_str
#undef TP_printk
-#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
+#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
#undef TP_fast_assign
#define TP_fast_assign(args...) args
On Fri, 2009-11-13 at 18:28 -0500, Steven Rostedt wrote:
> > > Use
> > > "\"%s\"", fmt
> > > instead of
> > > "%s", #fmt
> > > to export the string and not the way it is built.
>
> Is there any examples of this currently in the kernel? If so could you
> show that in the changlog.
net/mac80211/driver-trace.h has some that are exported really badly and
both trace-cmd and perf fall over on them.
Incidentally, the kvm_mmu ones are also weird, but I'm not sure they can
be fixed up properly.
Do you think Andreas's objection to the now-missing escaping of " and \
is a problem, or should we either fix the parsers to parse the string
backward from REC stuff, or fix the routine that outputs them to
userspace to quote " and \?
johannes
* Steven Rostedt <[email protected]> wrote:
>
> Ingo,
>
> Please pull the latest tip/tracing/urgent tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/tracing/urgent
>
>
> Johannes Berg (1):
> tracing: Fix event format export
>
> ----
> include/trace/ftrace.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Pulled, thanks Steve!
Ingo
Commit-ID: 811cb50baf63461ce0bdb234927046131fc7fa8b
Gitweb: http://git.kernel.org/tip/811cb50baf63461ce0bdb234927046131fc7fa8b
Author: Johannes Berg <[email protected]>
AuthorDate: Fri, 13 Nov 2009 23:40:09 +0100
Committer: Steven Rostedt <[email protected]>
CommitDate: Fri, 13 Nov 2009 22:20:34 -0500
tracing: Fix event format export
For some reason the export of the event print format to userspace
uses '#fmt' which breaks if the format string is anything but a plain
string, for example if it is built with macros then the macro names
are exported instead of their contents.
Use
"\"%s\"", fmt
instead of
"%s", #fmt
to export the string and not the way it is built.
For example, in net/mac80211/driver-trace.h for the trace event drv_start
there is:
TP_printk(
LOCAL_PR_FMT, LOCAL_PR_ARG
)
Which use to produce:
print fmt: LOCAL_PR_FMT, REC->wiphy_name
Now produces:
print fmt: "%s", REC->wiphy_name
Signed-off-by: Johannes Berg <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Steven Rostedt <[email protected]>
---
include/trace/ftrace.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index cc0d966..dacb8ef 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -159,7 +159,7 @@
#undef __get_str
#undef TP_printk
-#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
+#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
#undef TP_fast_assign
#define TP_fast_assign(args...) args
On Sat, 2009-11-14 at 10:18 +0000, tip-bot for Johannes Berg wrote:
> Commit-ID: 811cb50baf63461ce0bdb234927046131fc7fa8b
> Gitweb: http://git.kernel.org/tip/811cb50baf63461ce0bdb234927046131fc7fa8b
> Author: Johannes Berg <[email protected]>
> AuthorDate: Fri, 13 Nov 2009 23:40:09 +0100
> Committer: Steven Rostedt <[email protected]>
> CommitDate: Fri, 13 Nov 2009 22:20:34 -0500
>
> tracing: Fix event format export
>
> For some reason the export of the event print format to userspace
> uses '#fmt' which breaks if the format string is anything but a plain
> string, for example if it is built with macros then the macro names
> are exported instead of their contents.
>
> Use
> "\"%s\"", fmt
> instead of
> "%s", #fmt
> to export the string and not the way it is built.
>
> For example, in net/mac80211/driver-trace.h for the trace event drv_start
> there is:
>
> TP_printk(
> LOCAL_PR_FMT, LOCAL_PR_ARG
> )
>
> Which use to produce:
>
> print fmt: LOCAL_PR_FMT, REC->wiphy_name
>
> Now produces:
>
> print fmt: "%s", REC->wiphy_name
>
> Signed-off-by: Johannes Berg <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Steven Rostedt <[email protected]>
Ingo,
Any reason that this did not make it into Mainline? 2.6.32 is out, and
Johannes Berg was depending on this for his users.
-- Steve
> ---
> include/trace/ftrace.h | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> index cc0d966..dacb8ef 100644
> --- a/include/trace/ftrace.h
> +++ b/include/trace/ftrace.h
> @@ -159,7 +159,7 @@
> #undef __get_str
>
> #undef TP_printk
> -#define TP_printk(fmt, args...) "%s, %s\n", #fmt, __stringify(args)
> +#define TP_printk(fmt, args...) "\"%s\", %s\n", fmt, __stringify(args)
>
> #undef TP_fast_assign
> #define TP_fast_assign(args...) args
* Steven Rostedt <[email protected]> wrote:
> On Sat, 2009-11-14 at 10:18 +0000, tip-bot for Johannes Berg wrote:
> > Commit-ID: 811cb50baf63461ce0bdb234927046131fc7fa8b
> > Gitweb: http://git.kernel.org/tip/811cb50baf63461ce0bdb234927046131fc7fa8b
> > Author: Johannes Berg <[email protected]>
> > AuthorDate: Fri, 13 Nov 2009 23:40:09 +0100
> > Committer: Steven Rostedt <[email protected]>
> > CommitDate: Fri, 13 Nov 2009 22:20:34 -0500
> >
> > tracing: Fix event format export
> >
> > For some reason the export of the event print format to userspace
> > uses '#fmt' which breaks if the format string is anything but a plain
> > string, for example if it is built with macros then the macro names
> > are exported instead of their contents.
> >
> > Use
> > "\"%s\"", fmt
> > instead of
> > "%s", #fmt
> > to export the string and not the way it is built.
> >
> > For example, in net/mac80211/driver-trace.h for the trace event drv_start
> > there is:
> >
> > TP_printk(
> > LOCAL_PR_FMT, LOCAL_PR_ARG
> > )
> >
> > Which use to produce:
> >
> > print fmt: LOCAL_PR_FMT, REC->wiphy_name
> >
> > Now produces:
> >
> > print fmt: "%s", REC->wiphy_name
> >
> > Signed-off-by: Johannes Berg <[email protected]>
> > LKML-Reference: <[email protected]>
> > Signed-off-by: Steven Rostedt <[email protected]>
>
>
> Ingo,
>
> Any reason that this did not make it into Mainline? 2.6.32 is out, and
> Johannes Berg was depending on this for his users.
Yeah, it narrowly missed the last -rc (-rc8), and we only push
post-final-rc fixes for serious show-stopper regressions.
v2.6.32.1 will have this (and other) fixes. Johannes can cherry-pick the
fix in a day or so once it hits Linus's tree in the merge window.
Thanks,
Ingo
On Thu, 2009-12-03 at 15:55 +0100, Ingo Molnar wrote:
> > Any reason that this did not make it into Mainline? 2.6.32 is out, and
> > Johannes Berg was depending on this for his users.
>
> Yeah, it narrowly missed the last -rc (-rc8), and we only push
> post-final-rc fixes for serious show-stopper regressions.
You have an interesting definition of "narrowly missed":
$ git show v2.6.32-rc8
...
Date: Thu Nov 19 14:32:50 2009 -0800
...
$ git show --pretty=fuller 811cb50baf63461ce0bdb234927046131fc7fa8b
...
CommitDate: Fri Nov 13 22:20:34 2009 -0500
And Steven's pull request for this very simple fix was from the 14th.
That's 5 days until -rc8.
So maybe I have a personality problem, but suck it up -- you're wrong
this time.
johannes
* Johannes Berg <[email protected]> wrote:
> On Thu, 2009-12-03 at 15:55 +0100, Ingo Molnar wrote:
>
> > > Any reason that this did not make it into Mainline? 2.6.32 is out, and
> > > Johannes Berg was depending on this for his users.
> >
> > Yeah, it narrowly missed the last -rc (-rc8), and we only push
> > post-final-rc fixes for serious show-stopper regressions.
>
> You have an interesting definition of "narrowly missed":
>
> $ git show v2.6.32-rc8
> ...
> Date: Thu Nov 19 14:32:50 2009 -0800
> ...
> $ git show --pretty=fuller 811cb50baf63461ce0bdb234927046131fc7fa8b
> ...
> CommitDate: Fri Nov 13 22:20:34 2009 -0500
Yes, it came in late, after -rc7 and just two weeks before the final
2.6.32 kernel was released. Please test linux-next or -tip more
frequently if you want fixes to go upstream sooner.
> And Steven's pull request for this very simple fix was from the 14th.
> That's 5 days until -rc8.
We are extra careful in late -rc's - and per Linus's request we are only
pushing fixes for serious regressions, and even those we only push after
careful testing. That's a few days commit life-time at minimum in the
usual case.
> So maybe I have a personality problem, but suck it up -- you're wrong
> this time.
No need to complain - the fix is upstream now and will be in 2.6.32.1 -
within 1-2 weeks. If that's not fast enough for you then you can
cherry-pick upstream commit 811cb50baf63461ce0bdb234927046131fc7fa8b
into your tree right now.
Thanks,
Ingo