* [PATCH 0/3] tracing: Fix synthetic event parser
@ 2018-10-18 12:11 Masami Hiramatsu
2018-10-18 12:11 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 12: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] 6+ messages in thread
* [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier
2018-10-18 12:11 [PATCH 0/3] tracing: Fix synthetic event parser Masami Hiramatsu
@ 2018-10-18 12:11 ` Masami Hiramatsu
2018-10-18 13:07 ` Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu
2 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 12:11 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@vgar.kernel.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
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] 6+ messages in thread
* [PATCH 2/3] tracing: Fix synthetic event to allow semicolon at end
2018-10-18 12:11 [PATCH 0/3] tracing: Fix synthetic event parser Masami Hiramatsu
2018-10-18 12:11 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
@ 2018-10-18 12:12 ` Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu
2 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 12: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] 6+ messages in thread
* [PATCH 3/3] selftests: ftrace: Add synthetic event syntax testcase
2018-10-18 12:11 [PATCH 0/3] tracing: Fix synthetic event parser Masami Hiramatsu
2018-10-18 12:11 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu
@ 2018-10-18 12:12 ` Masami Hiramatsu
2 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 12:12 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] 6+ messages in thread
* Re: [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier
2018-10-18 12:11 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
@ 2018-10-18 13:07 ` Masami Hiramatsu
0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2018-10-18 13:07 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Shuah Khan, Tom Zanussi, linux-kernel, stable,
Tom Zanussi
On Thu, 18 Oct 2018 21:11:35 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 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@vgar.kernel.org>
Oops, I typo it... will resend v2 soon.
Thanks,
> Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> ---
> 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) {
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier
2018-10-20 1:29 [PATCH 0/3] [GIT PULL] tracing: A few small fixes to synthetic events Steven Rostedt
@ 2018-10-20 1:29 ` Steven Rostedt
0 siblings, 0 replies; 6+ messages in thread
From: Steven Rostedt @ 2018-10-20 1:29 UTC (permalink / raw)
To: linux-kernel
Cc: Linus Torvalds, Greg Kroah-Hartman, Ingo Molnar, Andrew Morton,
Shuah Khan, Tom Zanussi, stable, Masami Hiramatsu
From: Masami Hiramatsu <mhiramat@kernel.org>
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
Link: http://lkml.kernel.org/r/153986832571.18251.8448135724590496531.stgit@devbox
Cc: Shuah Khan <shuah@kernel.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: stable@vger.kernel.org
Fixes: commit 4b147936fa50 ("tracing: Add support for 'synthetic' events")
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
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) {
--
2.19.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-10-20 1:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-18 12:11 [PATCH 0/3] tracing: Fix synthetic event parser Masami Hiramatsu
2018-10-18 12:11 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Masami Hiramatsu
2018-10-18 13:07 ` Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 2/3] tracing: Fix synthetic event to allow semicolon at end Masami Hiramatsu
2018-10-18 12:12 ` [PATCH 3/3] selftests: ftrace: Add synthetic event syntax testcase Masami Hiramatsu
2018-10-20 1:29 [PATCH 0/3] [GIT PULL] tracing: A few small fixes to synthetic events Steven Rostedt
2018-10-20 1:29 ` [PATCH 1/3] tracing: Fix synthetic event to accept unsigned modifier Steven Rostedt
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).