* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.