* [PATCH v2 0/3] tracing: Fix synthetic event parser
@ 2018-10-18 13:11 Masami Hiramatsu
2018-10-18 13:12 ` [PATCH v2 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 13:11 UTC (permalink / raw)
To: Steven Rostedt, Shuah Khan; +Cc: Tom Zanussi, Masami Hiramatsu, linux-kernel
Hi,
Here are 2 fixes for synthetic event parser and its testcase.
One bug is that the synthetic_events can not accept "unsigned"
modifier for the field type, and another one is not allowing
the last independent semicolon.
I found that there is no testcase for checking it too. So the
last patch in this series adding a testcase for synthetic
event parser.
Thank you,
---
Masami Hiramatsu (3):
tracing: Fix synthetic event to accept unsigned modifier
tracing: Fix synthetic event to allow semicolon at end
selftests: ftrace: Add synthetic event syntax testcase
kernel/trace/trace_events_hist.c | 32 ++++++--
.../inter-event/trigger-synthetic-event-syntax.tc | 80 ++++++++++++++++++++
2 files changed, 105 insertions(+), 7 deletions(-)
create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc
--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/3] tracing: Fix synthetic event to accept unsigned modifier 2018-10-18 13:11 [PATCH v2 0/3] tracing: Fix synthetic event parser Masami Hiramatsu @ 2018-10-18 13:12 ` Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Masami Hiramatsu @ 2018-10-18 13:12 UTC (permalink / raw) To: Steven Rostedt, Shuah Khan Cc: Tom Zanussi, Masami Hiramatsu, linux-kernel, stable, Tom Zanussi Fix synthetic event to accept unsigned modifier for its field type correctly. Currently, synthetic_events interface returns error for "unsigned" modifiers as below; # echo "myevent unsigned long var" >> synthetic_events sh: write error: Invalid argument This is because argv_split() breaks "unsigned long" into "unsigned" and "long", but parse_synth_field() doesn't expected it. With this fix, synthetic_events can handle the "unsigned long" correctly like as below; # echo "myevent unsigned long var" >> synthetic_events # cat synthetic_events myevent unsigned long var Fixes: commit 4b147936fa50 ("tracing: Add support for 'synthetic' events") Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Cc: stable@vger.kernel.org Cc: Tom Zanussi <tom.zanussi@linux.intel.com> Cc: Steven Rostedt <rostedt@goodmis.org> --- Changes in v2: - Fix a typo of stable ML. --- kernel/trace/trace_events_hist.c | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 85f6b01431c7..6ff83941065a 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -738,16 +738,30 @@ static void free_synth_field(struct synth_field *field) kfree(field); } -static struct synth_field *parse_synth_field(char *field_type, - char *field_name) +static struct synth_field *parse_synth_field(int argc, char **argv, + int *consumed) { struct synth_field *field; + const char *prefix = NULL; + char *field_type = argv[0], *field_name; int len, ret = 0; char *array; if (field_type[0] == ';') field_type++; + if (!strcmp(field_type, "unsigned")) { + if (argc < 3) + return ERR_PTR(-EINVAL); + prefix = "unsigned "; + field_type = argv[1]; + field_name = argv[2]; + *consumed = 3; + } else { + field_name = argv[1]; + *consumed = 2; + } + len = strlen(field_name); if (field_name[len - 1] == ';') field_name[len - 1] = '\0'; @@ -760,11 +774,15 @@ static struct synth_field *parse_synth_field(char *field_type, array = strchr(field_name, '['); if (array) len += strlen(array); + if (prefix) + len += strlen(prefix); field->type = kzalloc(len, GFP_KERNEL); if (!field->type) { ret = -ENOMEM; goto free; } + if (prefix) + strcat(field->type, prefix); strcat(field->type, field_type); if (array) { strcat(field->type, array); @@ -1009,7 +1027,7 @@ static int create_synth_event(int argc, char **argv) struct synth_field *field, *fields[SYNTH_FIELDS_MAX]; struct synth_event *event = NULL; bool delete_event = false; - int i, n_fields = 0, ret = 0; + int i, consumed = 0, n_fields = 0, ret = 0; char *name; mutex_lock(&synth_event_mutex); @@ -1061,13 +1079,13 @@ static int create_synth_event(int argc, char **argv) goto err; } - field = parse_synth_field(argv[i], argv[i + 1]); + field = parse_synth_field(argc - i, &argv[i], &consumed); if (IS_ERR(field)) { ret = PTR_ERR(field); goto err; } - fields[n_fields] = field; - i++; n_fields++; + fields[n_fields++] = field; + i += consumed - 1; } if (i < argc) { ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/3] tracing: Fix synthetic event to allow semicolon at end 2018-10-18 13:11 [PATCH v2 0/3] tracing: Fix synthetic event parser Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu @ 2018-10-18 13:12 ` Masami Hiramatsu 2018-10-18 13:13 ` [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu 2018-10-22 20:10 ` [PATCH v2 0/3] tracing: Fix synthetic event parser Tom Zanussi 3 siblings, 0 replies; 10+ messages in thread From: Masami Hiramatsu @ 2018-10-18 13:12 UTC (permalink / raw) To: Steven Rostedt, Shuah Khan Cc: Tom Zanussi, Masami Hiramatsu, linux-kernel, stable, Tom Zanussi Fix synthetic event to allow independent semicolon at end. The synthetic_events interface accepts a semicolon after the last word if there is no space. # echo "myevent u64 var;" >> synthetic_events But if there is a space, it returns an error. # echo "myevent u64 var ;" > synthetic_events sh: write error: Invalid argument This behavior is difficult for users to understand. Let's allow the last independent semicolon too. Fixes: commit 4b147936fa50 ("tracing: Add support for 'synthetic' events") Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> Cc: stable@vger.kernel.org Cc: Tom Zanussi <tom.zanussi@linux.intel.com> Cc: Steven Rostedt <rostedt@goodmis.org> --- kernel/trace/trace_events_hist.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 6ff83941065a..d239004aaf29 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -1088,7 +1088,7 @@ static int create_synth_event(int argc, char **argv) i += consumed - 1; } - if (i < argc) { + if (i < argc && strcmp(argv[i], ";") != 0) { ret = -EINVAL; goto err; } ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-18 13:11 [PATCH v2 0/3] tracing: Fix synthetic event parser Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu @ 2018-10-18 13:13 ` Masami Hiramatsu 2018-10-18 20:34 ` Steven Rostedt 2018-10-22 20:10 ` [PATCH v2 0/3] tracing: Fix synthetic event parser Tom Zanussi 3 siblings, 1 reply; 10+ messages in thread From: Masami Hiramatsu @ 2018-10-18 13:13 UTC (permalink / raw) To: Steven Rostedt, Shuah Khan; +Cc: Tom Zanussi, Masami Hiramatsu, linux-kernel Add a testcase to check the syntax and field types for synthetic_events interface. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- .../inter-event/trigger-synthetic-event-syntax.tc | 80 ++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc new file mode 100644 index 000000000000..88e6c3f43006 --- /dev/null +++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc @@ -0,0 +1,80 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0 +# description: event trigger - test synthetic_events syntax parser + +do_reset() { + reset_trigger + echo > set_event + clear_trace +} + +fail() { #msg + do_reset + echo $1 + exit_fail +} + +if [ ! -f set_event ]; then + echo "event tracing is not supported" + exit_unsupported +fi + +if [ ! -f synthetic_events ]; then + echo "synthetic event is not supported" + exit_unsupported +fi + +reset_tracer +do_reset + +echo "Test synthetic_events syntax parser" + +echo > synthetic_events + +# synthetic event must have a field +! echo "myevent" >> synthetic_events +echo "myevent u64 var1" >> synthetic_events + +# synthetic event must be found in synthetic_events +grep "myevent[[:space:]]u64 var1" synthetic_events + +# it is not possible to add same name event +! echo "myevent u64 var2" >> synthetic_events + +# Non-append open will cleanup all events and add new one +echo "myevent u64 var2" > synthetic_events + +# multiple fields with different spaces +echo "myevent u64 var1; u64 var2;" > synthetic_events +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events +echo "myevent u64 var1 ; u64 var2 ;" > synthetic_events +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events +echo "myevent u64 var1 ;u64 var2" > synthetic_events +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events + +# test field types +echo "myevent u32 var" > synthetic_events +echo "myevent u16 var" > synthetic_events +echo "myevent u8 var" > synthetic_events +echo "myevent s64 var" > synthetic_events +echo "myevent s32 var" > synthetic_events +echo "myevent s16 var" > synthetic_events +echo "myevent s8 var" > synthetic_events + +echo "myevent char var" > synthetic_events +echo "myevent int var" > synthetic_events +echo "myevent long var" > synthetic_events +echo "myevent pid_t var" > synthetic_events + +echo "myevent unsigned char var" > synthetic_events +echo "myevent unsigned int var" > synthetic_events +echo "myevent unsigned long var" > synthetic_events +grep "myevent[[:space:]]unsigned long var" synthetic_events + +# test string type +echo "myevent char var[10]" > synthetic_events +grep "myevent[[:space:]]char\[10\] var" synthetic_events + +do_reset + +exit 0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-18 13:13 ` [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu @ 2018-10-18 20:34 ` Steven Rostedt 2018-10-18 20:38 ` Shuah Khan 2018-10-19 0:38 ` Masami Hiramatsu 0 siblings, 2 replies; 10+ messages in thread From: Steven Rostedt @ 2018-10-18 20:34 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Shuah Khan, Tom Zanussi, linux-kernel Hi Masami, Thanks! I pulled this into my urgent queue and will be running tests on it over night. Shuah, can you ack this patch, as I want to add it to this release, along with the other changes. Thanks! -- Steve On Thu, 18 Oct 2018 22:13:02 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > Add a testcase to check the syntax and field types for > synthetic_events interface. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > .../inter-event/trigger-synthetic-event-syntax.tc | 80 ++++++++++++++++++++ > 1 file changed, 80 insertions(+) > create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc > > diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc > new file mode 100644 > index 000000000000..88e6c3f43006 > --- /dev/null > +++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-syntax.tc > @@ -0,0 +1,80 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0 > +# description: event trigger - test synthetic_events syntax parser > + > +do_reset() { > + reset_trigger > + echo > set_event > + clear_trace > +} > + > +fail() { #msg > + do_reset > + echo $1 > + exit_fail > +} > + > +if [ ! -f set_event ]; then > + echo "event tracing is not supported" > + exit_unsupported > +fi > + > +if [ ! -f synthetic_events ]; then > + echo "synthetic event is not supported" > + exit_unsupported > +fi > + > +reset_tracer > +do_reset > + > +echo "Test synthetic_events syntax parser" > + > +echo > synthetic_events > + > +# synthetic event must have a field > +! echo "myevent" >> synthetic_events > +echo "myevent u64 var1" >> synthetic_events > + > +# synthetic event must be found in synthetic_events > +grep "myevent[[:space:]]u64 var1" synthetic_events > + > +# it is not possible to add same name event > +! echo "myevent u64 var2" >> synthetic_events > + > +# Non-append open will cleanup all events and add new one > +echo "myevent u64 var2" > synthetic_events > + > +# multiple fields with different spaces > +echo "myevent u64 var1; u64 var2;" > synthetic_events > +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events > +echo "myevent u64 var1 ; u64 var2 ;" > synthetic_events > +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events > +echo "myevent u64 var1 ;u64 var2" > synthetic_events > +grep "myevent[[:space:]]u64 var1; u64 var2" synthetic_events > + > +# test field types > +echo "myevent u32 var" > synthetic_events > +echo "myevent u16 var" > synthetic_events > +echo "myevent u8 var" > synthetic_events > +echo "myevent s64 var" > synthetic_events > +echo "myevent s32 var" > synthetic_events > +echo "myevent s16 var" > synthetic_events > +echo "myevent s8 var" > synthetic_events > + > +echo "myevent char var" > synthetic_events > +echo "myevent int var" > synthetic_events > +echo "myevent long var" > synthetic_events > +echo "myevent pid_t var" > synthetic_events > + > +echo "myevent unsigned char var" > synthetic_events > +echo "myevent unsigned int var" > synthetic_events > +echo "myevent unsigned long var" > synthetic_events > +grep "myevent[[:space:]]unsigned long var" synthetic_events > + > +# test string type > +echo "myevent char var[10]" > synthetic_events > +grep "myevent[[:space:]]char\[10\] var" synthetic_events > + > +do_reset > + > +exit 0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-18 20:34 ` Steven Rostedt @ 2018-10-18 20:38 ` Shuah Khan 2018-10-18 20:45 ` Steven Rostedt 2018-10-19 0:38 ` Masami Hiramatsu 1 sibling, 1 reply; 10+ messages in thread From: Shuah Khan @ 2018-10-18 20:38 UTC (permalink / raw) To: Steven Rostedt, Masami Hiramatsu; +Cc: Tom Zanussi, linux-kernel, Shuah Khan On 10/18/2018 02:34 PM, Steven Rostedt wrote: > > Hi Masami, > > Thanks! I pulled this into my urgent queue and will be running tests on > it over night. > > Shuah, can you ack this patch, as I want to add it to this release, > along with the other changes. > Acked-by: Shuah Khan <shuah@kernel.org> thanks, -- Shuah ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-18 20:38 ` Shuah Khan @ 2018-10-18 20:45 ` Steven Rostedt 0 siblings, 0 replies; 10+ messages in thread From: Steven Rostedt @ 2018-10-18 20:45 UTC (permalink / raw) To: Shuah Khan; +Cc: Masami Hiramatsu, Tom Zanussi, linux-kernel On Thu, 18 Oct 2018 14:38:22 -0600 Shuah Khan <shuah@kernel.org> wrote: > On 10/18/2018 02:34 PM, Steven Rostedt wrote: > > > > Hi Masami, > > > > Thanks! I pulled this into my urgent queue and will be running tests on > > it over night. > > > > Shuah, can you ack this patch, as I want to add it to this release, > > along with the other changes. > > > > Acked-by: Shuah Khan <shuah@kernel.org> > Thanks for the quick response! -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-18 20:34 ` Steven Rostedt 2018-10-18 20:38 ` Shuah Khan @ 2018-10-19 0:38 ` Masami Hiramatsu 2018-10-19 0:46 ` Steven Rostedt 1 sibling, 1 reply; 10+ messages in thread From: Masami Hiramatsu @ 2018-10-19 0:38 UTC (permalink / raw) To: Steven Rostedt; +Cc: Shuah Khan, Tom Zanussi, linux-kernel On Thu, 18 Oct 2018 16:34:27 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > > Hi Masami, > > Thanks! I pulled this into my urgent queue and will be running tests on > it over night. Thanks! I hope to pass the test :) > Shuah, can you ack this patch, as I want to add it to this release, > along with the other changes. Shuah, just a note that this test case must fail unless these fixes are applied. Thank you, -- Masami Hiramatsu <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase 2018-10-19 0:38 ` Masami Hiramatsu @ 2018-10-19 0:46 ` Steven Rostedt 0 siblings, 0 replies; 10+ messages in thread From: Steven Rostedt @ 2018-10-19 0:46 UTC (permalink / raw) To: Masami Hiramatsu; +Cc: Shuah Khan, Tom Zanussi, linux-kernel On Fri, 19 Oct 2018 09:38:18 +0900 Masami Hiramatsu <mhiramat@kernel.org> wrote: > > Shuah, can you ack this patch, as I want to add it to this release, > > along with the other changes. > > Shuah, just a note that this test case must fail unless these fixes are applied. This is why I wanted to attache them to my pull request :-) -- Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/3] tracing: Fix synthetic event parser 2018-10-18 13:11 [PATCH v2 0/3] tracing: Fix synthetic event parser Masami Hiramatsu ` (2 preceding siblings ...) 2018-10-18 13:13 ` [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu @ 2018-10-22 20:10 ` Tom Zanussi 3 siblings, 0 replies; 10+ messages in thread From: Tom Zanussi @ 2018-10-22 20:10 UTC (permalink / raw) To: Masami Hiramatsu, Steven Rostedt, Shuah Khan; +Cc: linux-kernel Hi Masami, Sorry for the late reply on this - was out on vacation last week. On Thu, 2018-10-18 at 22:11 +0900, Masami Hiramatsu wrote: > Hi, > > Here are 2 fixes for synthetic event parser and its testcase. > One bug is that the synthetic_events can not accept "unsigned" > modifier for the field type, and another one is not allowing > the last independent semicolon. > > I found that there is no testcase for checking it too. So the > last patch in this series adding a testcase for synthetic > event parser. > These patches look good to me, so for the lot: Acked-by: Tom Zanussi <zanussi@linux.intel.com> Tested-by: Tom Zanussi <zanussi@linux.intel.com> Thanks, Tom > Thank you, > > --- > > Masami Hiramatsu (3): > tracing: Fix synthetic event to accept unsigned modifier > tracing: Fix synthetic event to allow semicolon at end > selftests: ftrace: Add synthetic event syntax testcase > > > kernel/trace/trace_events_hist.c | 32 ++++++-- > .../inter-event/trigger-synthetic-event-syntax.tc | 80 > ++++++++++++++++++++ > 2 files changed, 105 insertions(+), 7 deletions(-) > create mode 100644 > tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger- > synthetic-event-syntax.tc > > -- > Masami Hiramatsu (Linaro) <mhiramat@kernel.org> ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-22 20:10 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-10-18 13:11 [PATCH v2 0/3] tracing: Fix synthetic event parser Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu 2018-10-18 13:12 ` [PATCH v2 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu 2018-10-18 13:13 ` [PATCH v2 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu 2018-10-18 20:34 ` Steven Rostedt 2018-10-18 20:38 ` Shuah Khan 2018-10-18 20:45 ` Steven Rostedt 2018-10-19 0:38 ` Masami Hiramatsu 2018-10-19 0:46 ` Steven Rostedt 2018-10-22 20:10 ` [PATCH v2 0/3] tracing: Fix synthetic event parser Tom Zanussi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).