[PATCH 0/2] tracing: Fix synthetic event parser

[PATCH 0/2] tracing: Fix synthetic event parser

[Masami Hiramatsu]

Hi,

I found another bug in synthetic event. This is a small fix, but
confusingly, there is also a bug in a test case.

Steve, since the testcase bugfix ([2/2]) breaks the test result
unless corresponding fix ([1/2]), I would like to ask you to send
these fixes from your tree.
Shuah, could you give this your ack?

Thank you,

---

Masami Hiramatsu (2):
      tracing: Return -ENOENT if there is no target synthetic event
      selftests/ftrace: Fix synthetic event test to delete event correctly


 kernel/trace/trace_events_hist.c                   |    4 +++-
 .../trigger-synthetic-event-createremove.tc        |   12 ++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

[PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event

[Masami Hiramatsu]

Return -ENOENT error if there is no target synthetic event.
This notices an operation failure to user as below;

  # echo 'wakeup_latency u64 lat; pid_t pid;' > synthetic_events
  # echo '!wakeup' >> synthetic_events
  sh: write error: No such file or directory

Fixes: 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 |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index d239004aaf29..eb908ef2ecec 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -1063,8 +1063,10 @@ static int create_synth_event(int argc, char **argv)
 		event = NULL;
 		ret = -EEXIST;
 		goto out;
-	} else if (delete_event)
+	} else if (delete_event) {
+		ret = -ENOENT;
 		goto out;
+	}
 
 	if (argc < 2) {
 		ret = -EINVAL;

[PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly

[Masami Hiramatsu]

Fix the synthetic event test case to remove event correctly.
If redirecting command to synthetic_event file without append
mode, it cleans up all existing events and execute (parse) the
command. This means "delete event" always fails to find the
target event.

Since previous synthetic event has a bug which doesn't return
-ENOENT even if it fails to find the deleting event, this test
passed. But fixing that bug, this test fails because this test
itself has a bug.

This fixes that bug by trying to delete event right after
adding an event, and use append mode redirection ('>>') instead
of normal redirection ('>').

Fixes: f06eec4d0f2c ('selftests: ftrace: Add inter-event hist triggers testcases')
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
Cc: Rajvi Jingar <rajvi.jingar@intel.com>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../trigger-synthetic-event-createremove.tc        |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-createremove.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-createremove.tc
index cef11377dcbd..c604438df13b 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-createremove.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-event-createremove.tc
@@ -35,18 +35,18 @@ fi
 
 reset_trigger
 
-echo "Test create synthetic event with an error"
-echo 'wakeup_latency  u64 lat pid_t pid char' > synthetic_events > /dev/null
+echo "Test remove synthetic event"
+echo '!wakeup_latency  u64 lat pid_t pid char comm[16]' >> synthetic_events
 if [ -d events/synthetic/wakeup_latency ]; then
-    fail "Created wakeup_latency synthetic event with an invalid format"
+    fail "Failed to delete wakeup_latency synthetic event"
 fi
 
 reset_trigger
 
-echo "Test remove synthetic event"
-echo '!wakeup_latency  u64 lat pid_t pid char comm[16]' > synthetic_events
+echo "Test create synthetic event with an error"
+echo 'wakeup_latency  u64 lat pid_t pid char' > synthetic_events > /dev/null
 if [ -d events/synthetic/wakeup_latency ]; then
-    fail "Failed to delete wakeup_latency synthetic event"
+    fail "Created wakeup_latency synthetic event with an invalid format"
 fi
 
 do_reset

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Tom Zanussi]

Hi Masami,

On Mon, 2018-10-22 at 00:07 +0900, Masami Hiramatsu wrote:
> Hi,
> 
> I found another bug in synthetic event. This is a small fix, but
> confusingly, there is also a bug in a test case.
> 
> Steve, since the testcase bugfix ([2/2]) breaks the test result
> unless corresponding fix ([1/2]), I would like to ask you to send
> these fixes from your tree.
> Shuah, could you give this your ack?
> 

Yes, I think the silent failing mode is confusing to users, and so
these changes make more sense.

Acked-by: Tom Zanussi <zanussi@linux.intel.com>
Tested-by: Tom Zanussi <zanussi@linux.intel.com>

Thanks,

Tom


> Thank you,
> 
> ---
> 
> Masami Hiramatsu (2):
>       tracing: Return -ENOENT if there is no target synthetic event
>       selftests/ftrace: Fix synthetic event test to delete event
> correctly
> 
> 
>  kernel/trace/trace_events_hist.c                   |    4 +++-
>  .../trigger-synthetic-event-createremove.tc        |   12 ++++++--
> ----
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Steven Rostedt]

On Mon, 22 Oct 2018 00:07:50 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Hi,
> 
> I found another bug in synthetic event. This is a small fix, but
> confusingly, there is also a bug in a test case.
> 
> Steve, since the testcase bugfix ([2/2]) breaks the test result
> unless corresponding fix ([1/2]), I would like to ask you to send
> these fixes from your tree.

Hi Masami,

I'm currently in travel mode, but I'm letting you know that I'll be
working on these today. It might take a bit to get out.

I'll add them on top of my linux-next code, and include them in the
pull request I'm hoping to do on Tuesday.

-- Steve

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Steven Rostedt]

On Sun, 28 Oct 2018 04:01:35 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> I'll add them on top of my linux-next code, and include them in the
> pull request I'm hoping to do on Tuesday.

Bah, I think this is on a different branch, and that will make it go as
a separate pull. Anyway, it probably will still be on Tuesday where I
push it to Linus.

-- Steve

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Shuah Khan]

On 10/28/2018 02:09 AM, Steven Rostedt wrote:
> On Sun, 28 Oct 2018 04:01:35 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
>> I'll add them on top of my linux-next code, and include them in the
>> pull request I'm hoping to do on Tuesday.
> 
> Bah, I think this is on a different branch, and that will make it go as
> a separate pull. Anyway, it probably will still be on Tuesday where I
> push it to Linus.
> 
> -- Steve
> 

Here it is

Acked-by: Shuah Khan <shuah@kernel.org>

thanks,
-- Shuah

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Steven Rostedt]

On Sun, 28 Oct 2018 12:10:17 -0600
Shuah Khan <shuah@kernel.org> wrote:

> Here it is
> 
> Acked-by: Shuah Khan <shuah@kernel.org>
>

Thanks a lot Shuah!

-- Steve

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Shuah Khan]

On 10/28/2018 02:01 AM, Steven Rostedt wrote:
> On Mon, 22 Oct 2018 00:07:50 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
>> Hi,
>>
>> I found another bug in synthetic event. This is a small fix, but
>> confusingly, there is also a bug in a test case.
>>
>> Steve, since the testcase bugfix ([2/2]) breaks the test result
>> unless corresponding fix ([1/2]), I would like to ask you to send
>> these fixes from your tree.
> 
> Hi Masami,
> 
> I'm currently in travel mode, but I'm letting you know that I'll be
> working on these today. It might take a bit to get out.
> 
> I'll add them on top of my linux-next code, and include them in the
> pull request I'm hoping to do on Tuesday.
> 
> -- Steve
> 

Hi Steve,

Are there any dependencies on my pull request I sent to Linus? It hasn't
been pulled in yet. If there is one, could please you mention that in your
pull request for this.

thanks,
-- Shuah

Re: [PATCH 0/2] tracing: Fix synthetic event parser

[Steven Rostedt]

On Sun, 28 Oct 2018 11:33:35 -0600
Shuah Khan <shuah@kernel.org> wrote:

> Hi Steve,
> 
> Are there any dependencies on my pull request I sent to Linus? It hasn't
> been pulled in yet. If there is one, could please you mention that in your
> pull request for this.

There's no dependency on the patches you took. But if you could ack the
selftest one that would be awesome.

Thanks,

-- Steve