linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] tracing: Fix synthetic event parser
@ 2018-10-21 15:07 Masami Hiramatsu
  2018-10-21 15:08 ` [PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event Masami Hiramatsu
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2018-10-21 15:07 UTC (permalink / raw)
  To: Steven Rostedt, Shuah Khan
  Cc: Rajvi Jingar, Tom Zanussi, Masami Hiramatsu, linux-kernel

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>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event
  2018-10-21 15:07 [PATCH 0/2] tracing: Fix synthetic event parser Masami Hiramatsu
@ 2018-10-21 15:08 ` Masami Hiramatsu
  2018-10-21 15:08 ` [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly Masami Hiramatsu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2018-10-21 15:08 UTC (permalink / raw)
  To: Steven Rostedt, Shuah Khan
  Cc: Rajvi Jingar, Tom Zanussi, Masami Hiramatsu, linux-kernel,
	stable, Tom Zanussi

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;


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly
  2018-10-21 15:07 [PATCH 0/2] tracing: Fix synthetic event parser Masami Hiramatsu
  2018-10-21 15:08 ` [PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event Masami Hiramatsu
@ 2018-10-21 15:08 ` Masami Hiramatsu
  2018-10-22 20:14 ` [PATCH 0/2] tracing: Fix synthetic event parser Tom Zanussi
  2018-10-28  8:01 ` Steven Rostedt
  3 siblings, 0 replies; 11+ messages in thread
From: Masami Hiramatsu @ 2018-10-21 15:08 UTC (permalink / raw)
  To: Steven Rostedt, Shuah Khan
  Cc: Rajvi Jingar, Tom Zanussi, Masami Hiramatsu, linux-kernel,
	stable, Tom Zanussi

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-21 15:07 [PATCH 0/2] tracing: Fix synthetic event parser Masami Hiramatsu
  2018-10-21 15:08 ` [PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event Masami Hiramatsu
  2018-10-21 15:08 ` [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly Masami Hiramatsu
@ 2018-10-22 20:14 ` Tom Zanussi
  2018-10-28  8:01 ` Steven Rostedt
  3 siblings, 0 replies; 11+ messages in thread
From: Tom Zanussi @ 2018-10-22 20:14 UTC (permalink / raw)
  To: Masami Hiramatsu, Steven Rostedt, Shuah Khan; +Cc: Rajvi Jingar, linux-kernel

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>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-21 15:07 [PATCH 0/2] tracing: Fix synthetic event parser Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2018-10-22 20:14 ` [PATCH 0/2] tracing: Fix synthetic event parser Tom Zanussi
@ 2018-10-28  8:01 ` Steven Rostedt
  2018-10-28  8:09   ` Steven Rostedt
  2018-10-28 17:33   ` Shuah Khan
  3 siblings, 2 replies; 11+ messages in thread
From: Steven Rostedt @ 2018-10-28  8:01 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Shuah Khan, Rajvi Jingar, Tom Zanussi, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-28  8:01 ` Steven Rostedt
@ 2018-10-28  8:09   ` Steven Rostedt
  2018-10-28 18:10     ` Shuah Khan
  2018-10-28 17:33   ` Shuah Khan
  1 sibling, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2018-10-28  8:09 UTC (permalink / raw)
  To: Masami Hiramatsu; +Cc: Shuah Khan, Rajvi Jingar, Tom Zanussi, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-28  8:01 ` Steven Rostedt
  2018-10-28  8:09   ` Steven Rostedt
@ 2018-10-28 17:33   ` Shuah Khan
  2018-10-28 18:07     ` Steven Rostedt
  1 sibling, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2018-10-28 17:33 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Rajvi Jingar, Tom Zanussi, linux-kernel, 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-28 17:33   ` Shuah Khan
@ 2018-10-28 18:07     ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2018-10-28 18:07 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Masami Hiramatsu, Rajvi Jingar, Tom Zanussi, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-28  8:09   ` Steven Rostedt
@ 2018-10-28 18:10     ` Shuah Khan
  2018-10-28 19:13       ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Shuah Khan @ 2018-10-28 18:10 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu
  Cc: Rajvi Jingar, Tom Zanussi, linux-kernel, 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] tracing: Fix synthetic event parser
  2018-10-28 18:10     ` Shuah Khan
@ 2018-10-28 19:13       ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2018-10-28 19:13 UTC (permalink / raw)
  To: Shuah Khan; +Cc: Masami Hiramatsu, Rajvi Jingar, Tom Zanussi, linux-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly
  2018-10-30 13:13 [PATCH 0/2] [GIT PULL] tracing: Fixes to synthetic events Steven Rostedt
@ 2018-10-30 13:13 ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2018-10-30 13:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Linus Torvalds, Ingo Molnar, Andrew Morton, Shuah Khan,
	Tom Zanussi, Tom Zanussi, Tom Zanussi, Rajvi Jingar, stable,
	Masami Hiramatsu

From: Masami Hiramatsu <mhiramat@kernel.org>

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 ('>').

Link: http://lkml.kernel.org/r/154013452832.25576.2305459545429386517.stgit@devbox

Acked-by: Shuah Khan <shuah@kernel.org>
Acked-by: Tom Zanussi <zanussi@linux.intel.com>
Tested-by: Tom Zanussi <zanussi@linux.intel.com>
Cc: Tom Zanussi <zanussi@kernel.org>
Cc: Tom Zanussi <tom.zanussi@linux.intel.com>
Cc: Rajvi Jingar <rajvi.jingar@intel.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: stable@vger.kernel.org
Fixes: f06eec4d0f2c ('selftests: ftrace: Add inter-event hist triggers testcases')
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: 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
-- 
2.19.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-10-30 13:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-21 15:07 [PATCH 0/2] tracing: Fix synthetic event parser Masami Hiramatsu
2018-10-21 15:08 ` [PATCH 1/2] tracing: Return -ENOENT if there is no target synthetic event Masami Hiramatsu
2018-10-21 15:08 ` [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly Masami Hiramatsu
2018-10-22 20:14 ` [PATCH 0/2] tracing: Fix synthetic event parser Tom Zanussi
2018-10-28  8:01 ` Steven Rostedt
2018-10-28  8:09   ` Steven Rostedt
2018-10-28 18:10     ` Shuah Khan
2018-10-28 19:13       ` Steven Rostedt
2018-10-28 17:33   ` Shuah Khan
2018-10-28 18:07     ` Steven Rostedt
2018-10-30 13:13 [PATCH 0/2] [GIT PULL] tracing: Fixes to synthetic events Steven Rostedt
2018-10-30 13:13 ` [PATCH 2/2] selftests/ftrace: Fix synthetic event test to delete event correctly 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).