[BUGFIX PATCH v3 0/4] selftests/ftrace: Fix ftracetest testcases for non-function tracer case

[BUGFIX PATCH v3 0/4] selftests/ftrace: Fix ftracetest testcases for non-function tracer case

[Masami Hiramatsu]

Hi,

Here is the 3rd version of patches to fix some issues which happens on
the kernel with CONFIG_FUNCTION_TRACER=n or CONFIG_DYNAMIC_FTRACE=n.

In this version and v2, I updated the descriptions of the first 2 patches
according to Steve's comment, added Steve's Reviewed-by to the 3rd patch,
and added the 4th patch which was newly found.

Thank you,

---

Masami Hiramatsu (4):
      selftests/ftrace: Fix to check the existence of set_ftrace_filter
      selftests/ftrace: Fix ftrace test cases to check unsupported
      selftests/ftrace: Do not to use absolute debugfs path
      selftests/ftrace: Fix multiple kprobe testcase


 .../ftrace/test.d/ftrace/func-filter-stacktrace.tc |    2 ++
 .../selftests/ftrace/test.d/ftrace/func_cpumask.tc |    5 +++++
 tools/testing/selftests/ftrace/test.d/functions    |    4 +++-
 .../ftrace/test.d/kprobe/multiple_kprobes.tc       |    6 +++---
 .../inter-event/trigger-action-hist-xfail.tc       |    4 ++--
 .../inter-event/trigger-onchange-action-hist.tc    |    2 +-
 .../inter-event/trigger-snapshot-action-hist.tc    |    4 ++--
 7 files changed, 18 insertions(+), 9 deletions(-)

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

[BUGFIX PATCH v3 1/4] selftests/ftrace: Fix to check the existence of set_ftrace_filter

[Masami Hiramatsu]

If we run ftracetest on the kernel with CONFIG_DYNAMIC_FTRACE=n,
there is no set_ftrace_filter and all test cases are failed,
because reset_ftrace_filter returns an error.
Let's check whether set_ftrace_filter exists and remove redundant
set_ftrace_filter from initialize_ftrace().

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 tools/testing/selftests/ftrace/test.d/functions |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
index 86986c4bba54..19d288cdf336 100644
--- a/tools/testing/selftests/ftrace/test.d/functions
+++ b/tools/testing/selftests/ftrace/test.d/functions
@@ -46,6 +46,9 @@ reset_events_filter() { # reset all current setting filters
 }
 
 reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
+    if [ ! -f set_ftrace_filter ]; then
+      return 0
+    fi
     echo > set_ftrace_filter
     grep -v '^#' set_ftrace_filter | while read t; do
 	tr=`echo $t | cut -d: -f2`
@@ -93,7 +96,6 @@ initialize_ftrace() { # Reset ftrace to initial-state
     disable_events
     [ -f set_event_pid ] && echo > set_event_pid
     [ -f set_ftrace_pid ] && echo > set_ftrace_pid
-    [ -f set_ftrace_filter ] && echo | tee set_ftrace_*
     [ -f set_graph_function ] && echo | tee set_graph_*
     [ -f stack_trace_filter ] && echo > stack_trace_filter
     [ -f kprobe_events ] && echo > kprobe_events

Re: [BUGFIX PATCH v3 1/4] selftests/ftrace: Fix to check the existence of set_ftrace_filter

[Steven Rostedt]

On Mon, 25 Nov 2019 15:57:00 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> If we run ftracetest on the kernel with CONFIG_DYNAMIC_FTRACE=n,
> there is no set_ftrace_filter and all test cases are failed,
> because reset_ftrace_filter returns an error.
> Let's check whether set_ftrace_filter exists and remove redundant
> set_ftrace_filter from initialize_ftrace().
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  tools/testing/selftests/ftrace/test.d/functions |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> index 86986c4bba54..19d288cdf336 100644
> --- a/tools/testing/selftests/ftrace/test.d/functions
> +++ b/tools/testing/selftests/ftrace/test.d/functions
> @@ -46,6 +46,9 @@ reset_events_filter() { # reset all current setting filters
>  }
>  
>  reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
> +    if [ ! -f set_ftrace_filter ]; then
> +      return 0
> +    fi
>      echo > set_ftrace_filter
>      grep -v '^#' set_ftrace_filter | while read t; do
>  	tr=`echo $t | cut -d: -f2`
> @@ -93,7 +96,6 @@ initialize_ftrace() { # Reset ftrace to initial-state
>      disable_events
>      [ -f set_event_pid ] && echo > set_event_pid
>      [ -f set_ftrace_pid ] && echo > set_ftrace_pid
> -    [ -f set_ftrace_filter ] && echo | tee set_ftrace_*

The above should be changed to:

	[ -f set_ftrace_notrace ] && echo > set_ftrace_notrace

-- Steve


>      [ -f set_graph_function ] && echo | tee set_graph_*
>      [ -f stack_trace_filter ] && echo > stack_trace_filter
>      [ -f kprobe_events ] && echo > kprobe_events

Re: [BUGFIX PATCH v3 1/4] selftests/ftrace: Fix to check the existence of set_ftrace_filter

[Masami Hiramatsu]

On Mon, 25 Nov 2019 09:44:45 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Mon, 25 Nov 2019 15:57:00 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > If we run ftracetest on the kernel with CONFIG_DYNAMIC_FTRACE=n,
> > there is no set_ftrace_filter and all test cases are failed,
> > because reset_ftrace_filter returns an error.
> > Let's check whether set_ftrace_filter exists and remove redundant
> > set_ftrace_filter from initialize_ftrace().
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/testing/selftests/ftrace/test.d/functions |    4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> > index 86986c4bba54..19d288cdf336 100644
> > --- a/tools/testing/selftests/ftrace/test.d/functions
> > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > @@ -46,6 +46,9 @@ reset_events_filter() { # reset all current setting filters
> >  }
> >  
> >  reset_ftrace_filter() { # reset all triggers in set_ftrace_filter
> > +    if [ ! -f set_ftrace_filter ]; then
> > +      return 0
> > +    fi
> >      echo > set_ftrace_filter
> >      grep -v '^#' set_ftrace_filter | while read t; do
> >  	tr=`echo $t | cut -d: -f2`
> > @@ -93,7 +96,6 @@ initialize_ftrace() { # Reset ftrace to initial-state
> >      disable_events
> >      [ -f set_event_pid ] && echo > set_event_pid
> >      [ -f set_ftrace_pid ] && echo > set_ftrace_pid
> > -    [ -f set_ftrace_filter ] && echo | tee set_ftrace_*
> 
> The above should be changed to:
> 
> 	[ -f set_ftrace_notrace ] && echo > set_ftrace_notrace

Ah, good point! I think that should be done by another patch, since
it will improve ftracetest.

Thank you,

> 
> -- Steve
> 
> 
> >      [ -f set_graph_function ] && echo | tee set_graph_*
> >      [ -f stack_trace_filter ] && echo > stack_trace_filter
> >      [ -f kprobe_events ] && echo > kprobe_events
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [BUGFIX PATCH v3 1/4] selftests/ftrace: Fix to check the existence of set_ftrace_filter

[Steven Rostedt]

On Tue, 26 Nov 2019 09:13:45 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> > > @@ -93,7 +96,6 @@ initialize_ftrace() { # Reset ftrace to initial-state
> > >      disable_events
> > >      [ -f set_event_pid ] && echo > set_event_pid
> > >      [ -f set_ftrace_pid ] && echo > set_ftrace_pid
> > > -    [ -f set_ftrace_filter ] && echo | tee set_ftrace_*  
> > 
> > The above should be changed to:
> > 
> > 	[ -f set_ftrace_notrace ] && echo > set_ftrace_notrace  
> 
> Ah, good point! I think that should be done by another patch, since
> it will improve ftracetest.

No, it belongs in this patch, because you are removing:

	[ -f set_ftrace_filter ] && echo | tee set_ftrace_*

which is equivalent to:

	[ -f set_ftrace_filter ] && echo > set_ftrace_filter
	[ -f set_ftrace_filter ] && echo > set_ftrace_notrace

as the "tee set_ftrace_*" covers both.

Without this change, this patch removes the update to
set_ftrace_notrace.

-- Steve

Re: [BUGFIX PATCH v3 1/4] selftests/ftrace: Fix to check the existence of set_ftrace_filter

[Masami Hiramatsu]

On Mon, 25 Nov 2019 20:54:29 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 26 Nov 2019 09:13:45 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
> 
> > > > @@ -93,7 +96,6 @@ initialize_ftrace() { # Reset ftrace to initial-state
> > > >      disable_events
> > > >      [ -f set_event_pid ] && echo > set_event_pid
> > > >      [ -f set_ftrace_pid ] && echo > set_ftrace_pid
> > > > -    [ -f set_ftrace_filter ] && echo | tee set_ftrace_*  
> > > 
> > > The above should be changed to:
> > > 
> > > 	[ -f set_ftrace_notrace ] && echo > set_ftrace_notrace  
> > 
> > Ah, good point! I think that should be done by another patch, since
> > it will improve ftracetest.
> 
> No, it belongs in this patch, because you are removing:
> 
> 	[ -f set_ftrace_filter ] && echo | tee set_ftrace_*
> 
> which is equivalent to:
> 
> 	[ -f set_ftrace_filter ] && echo > set_ftrace_filter
> 	[ -f set_ftrace_filter ] && echo > set_ftrace_notrace
> 
> as the "tee set_ftrace_*" covers both.

Ah, I see. 

> 
> Without this change, this patch removes the update to
> set_ftrace_notrace.

OK, I'll update it.

Thank you!

> 
> -- Steve


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[BUGFIX PATCH v3 2/4] selftests/ftrace: Fix ftrace test cases to check unsupported

[Masami Hiramatsu]

Since dynamic function tracer can be disabled, set_ftrace_filter
can be disappeared. Test cases which depends on it, must check
whether the set_ftrace_filter exists or not before testing
and if not, return as unsupported.

Also, if the function tracer itself is disabled, we can not
set "function" to current_tracer. Test cases must check it
before testing, and return as unsupported.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 .../ftrace/test.d/ftrace/func-filter-stacktrace.tc |    2 ++
 .../selftests/ftrace/test.d/ftrace/func_cpumask.tc |    5 +++++
 2 files changed, 7 insertions(+)

diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-stacktrace.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-stacktrace.tc
index 36fb59f886ea..1a52f2883fe0 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-stacktrace.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func-filter-stacktrace.tc
@@ -3,6 +3,8 @@
 # description: ftrace - stacktrace filter command
 # flags: instance
 
+[ ! -f set_ftrace_filter ] && exit_unsupported
+
 echo _do_fork:stacktrace >> set_ftrace_filter
 
 grep -q "_do_fork:stacktrace:unlimited" set_ftrace_filter
diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
index 86a1f07ef2ca..7757b549f0b6 100644
--- a/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
+++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
@@ -15,6 +15,11 @@ if [ $NP -eq 1 ] ;then
   exit_unresolved
 fi
 
+if ! grep -q function available_tracers ; then
+  echo "Function trace is not enabled"
+  exit_unsupported
+fi
+
 ORIG_CPUMASK=`cat tracing_cpumask`
 
 do_reset() {

Re: [BUGFIX PATCH v3 2/4] selftests/ftrace: Fix ftrace test cases to check unsupported

[Steven Rostedt]

On Mon, 25 Nov 2019 15:57:09 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Since dynamic function tracer can be disabled, set_ftrace_filter
> can be disappeared. Test cases which depends on it, must check
> whether the set_ftrace_filter exists or not before testing
> and if not, return as unsupported.
> 
> Also, if the function tracer itself is disabled, we can not
> set "function" to current_tracer. Test cases must check it
> before testing, and return as unsupported.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve

[BUGFIX PATCH v3 3/4] selftests/ftrace: Do not to use absolute debugfs path

[Masami Hiramatsu]

Use relative path to trigger file instead of absolute debugfs path,
because if the user uses tracefs instead of debugfs, it can be
mounted at /sys/kernel/tracing.
Anyway, since the ftracetest is designed to be run at the tracing
directory, user doesn't need to use absolute path.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 .../inter-event/trigger-action-hist-xfail.tc       |    4 ++--
 .../inter-event/trigger-onchange-action-hist.tc    |    2 +-
 .../inter-event/trigger-snapshot-action-hist.tc    |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-action-hist-xfail.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-action-hist-xfail.tc
index 1221240f8cf6..3f2aee115f6e 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-action-hist-xfail.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-action-hist-xfail.tc
@@ -21,10 +21,10 @@ grep -q "snapshot()" README || exit_unsupported # version issue
 
 echo "Test expected snapshot action failure"
 
-echo 'hist:keys=comm:onmatch(sched.sched_wakeup).snapshot()' >> /sys/kernel/debug/tracing/events/sched/sched_waking/trigger && exit_fail
+echo 'hist:keys=comm:onmatch(sched.sched_wakeup).snapshot()' >> events/sched/sched_waking/trigger && exit_fail
 
 echo "Test expected save action failure"
 
-echo 'hist:keys=comm:onmatch(sched.sched_wakeup).save(comm,prio)' >> /sys/kernel/debug/tracing/events/sched/sched_waking/trigger && exit_fail
+echo 'hist:keys=comm:onmatch(sched.sched_wakeup).save(comm,prio)' >> events/sched/sched_waking/trigger && exit_fail
 
 exit_xfail
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onchange-action-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onchange-action-hist.tc
index 064a284e4e75..c80007aa9f86 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onchange-action-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-onchange-action-hist.tc
@@ -16,7 +16,7 @@ grep -q "onchange(var)" README || exit_unsupported # version issue
 
 echo "Test onchange action"
 
-echo 'hist:keys=comm:newprio=prio:onchange($newprio).save(comm,prio) if comm=="ping"' >> /sys/kernel/debug/tracing/events/sched/sched_waking/trigger
+echo 'hist:keys=comm:newprio=prio:onchange($newprio).save(comm,prio) if comm=="ping"' >> events/sched/sched_waking/trigger
 
 ping $LOCALHOST -c 3
 nice -n 1 ping $LOCALHOST -c 3
diff --git a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-snapshot-action-hist.tc b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-snapshot-action-hist.tc
index 18fff69fc433..f546c1b66a9b 100644
--- a/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-snapshot-action-hist.tc
+++ b/tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-snapshot-action-hist.tc
@@ -23,9 +23,9 @@ grep -q "snapshot()" README || exit_unsupported # version issue
 
 echo "Test snapshot action"
 
-echo 1 > /sys/kernel/debug/tracing/events/sched/enable
+echo 1 > events/sched/enable
 
-echo 'hist:keys=comm:newprio=prio:onchange($newprio).save(comm,prio):onchange($newprio).snapshot() if comm=="ping"' >> /sys/kernel/debug/tracing/events/sched/sched_waking/trigger
+echo 'hist:keys=comm:newprio=prio:onchange($newprio).save(comm,prio):onchange($newprio).snapshot() if comm=="ping"' >> events/sched/sched_waking/trigger
 
 ping $LOCALHOST -c 3
 nice -n 1 ping $LOCALHOST -c 3

[BUGFIX PATCH v3 4/4] selftests/ftrace: Fix multiple kprobe testcase

[Masami Hiramatsu]

Fix multiple kprobe event testcase to work it correctly.
There are 2 bugfixes.
 - Since `wc -l FILE` returns not only line number but also
   FILE filename, following "if" statement always failed.
   Fix this bug by replacing it with 'cat FILE | wc -l'
 - Since "while do-done loop" block with pipeline becomes a
   subshell, $N local variable is not update outside of
   the loop.
   Fix this bug by using actual target number (256) instead
   of $N.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 .../ftrace/test.d/kprobe/multiple_kprobes.tc       |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
index 5862eee91e1d..6e3dbe5f96b7 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
@@ -20,9 +20,9 @@ while read i; do
   test $N -eq 256 && break
 done
 
-L=`wc -l kprobe_events`
-if [ $L -ne $N ]; then
-  echo "The number of kprobes events ($L) is not $N"
+L=`cat kprobe_events | wc -l`
+if [ $L -ne 256 ]; then
+  echo "The number of kprobes events ($L) is not 256"
   exit_fail
 fi
 

Re: [BUGFIX PATCH v3 4/4] selftests/ftrace: Fix multiple kprobe testcase

[Steven Rostedt]

On Mon, 25 Nov 2019 15:57:27 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:

> Fix multiple kprobe event testcase to work it correctly.
> There are 2 bugfixes.
>  - Since `wc -l FILE` returns not only line number but also
>    FILE filename, following "if" statement always failed.
>    Fix this bug by replacing it with 'cat FILE | wc -l'
>  - Since "while do-done loop" block with pipeline becomes a
>    subshell, $N local variable is not update outside of
>    the loop.
>    Fix this bug by using actual target number (256) instead
>    of $N.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>

Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

-- Steve