Hi, Here is the 4th 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 I fixed [1/4] to cleanup set_ftrace_notrace (Thanks Steve!) 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 | 5 ++++- .../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, 19 insertions(+), 9 deletions(-) -- Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
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 in reset_ftrace_filter() and clean up only set_ftrace_notrace in initialize_ftrace(). Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- Changes in v4: - Clear set_ftrace_notrace --- tools/testing/selftests/ftrace/test.d/functions | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions index 86986c4bba54..5d4550591ff9 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,7 @@ 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_ftrace_notrace ] && echo > set_ftrace_notrace [ -f set_graph_function ] && echo | tee set_graph_* [ -f stack_trace_filter ] && echo > stack_trace_filter [ -f kprobe_events ] && echo > kprobe_events
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> --- .../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() {
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
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> --- .../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
On Tue, 26 Nov 2019 17:34:16 +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 in reset_ftrace_filter()
> and clean up only set_ftrace_notrace in initialize_ftrace().
>
>
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
On 11/26/19 1:34 AM, Masami Hiramatsu wrote:
> Hi,
>
> Here is the 4th 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 I fixed [1/4] to cleanup set_ftrace_notrace (Thanks Steve!)
>
> 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 | 5 ++++-
> .../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, 19 insertions(+), 9 deletions(-)
>
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>
>
Hi Masami and Steve,
Thanks. I will pull these in for 5.5-rc1
-- Shuah
On Tue, 26 Nov 2019 17:34:24 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> --- 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`
Strange, but the bashism test failed:
++ checkbashisms /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
possible bashism in /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc line 18 ('function' is useless):
if ! grep -q function available_tracers ; then
Not sure why it did not like that line. Maybe my bashism check got
confused by the key word "function"?
Yep!
By adding quotes around "function" it doesn't complain:
if ! grep -q "function" available_tracers ; then
May need to add that.
-- Steve
On Tue, 26 Nov 2019 12:49:01 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 26 Nov 2019 17:34:24 +0900
> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>
> > --- 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`
>
> Strange, but the bashism test failed:
>
> ++ checkbashisms /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
> possible bashism in /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc line 18 ('function' is useless):
> if ! grep -q function available_tracers ; then
>
> Not sure why it did not like that line. Maybe my bashism check got
> confused by the key word "function"?
>
> Yep!
>
> By adding quotes around "function" it doesn't complain:
>
> if ! grep -q "function" available_tracers ; then
>
> May need to add that.
Thanks! Shuah, can I update this patch?
I'll send it asap.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
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> --- Changes in v4.1 - Add double-quote to "function" word for checkbashisms clean (Thanks Steve!) --- .../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..71fa3f49e35e 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() {
On 11/26/19 4:31 PM, Masami Hiramatsu wrote:
> On Tue, 26 Nov 2019 12:49:01 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> On Tue, 26 Nov 2019 17:34:24 +0900
>> Masami Hiramatsu <mhiramat@kernel.org> wrote:
>>
>>> --- 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`
>>
>> Strange, but the bashism test failed:
>>
>> ++ checkbashisms /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc
>> possible bashism in /work/git-local/linux.git/tools/testing/selftests/ftrace/test.d/ftrace/func_cpumask.tc line 18 ('function' is useless):
>> if ! grep -q function available_tracers ; then
>>
>> Not sure why it did not like that line. Maybe my bashism check got
>> confused by the key word "function"?
>>
>> Yep!
>>
>> By adding quotes around "function" it doesn't complain:
>>
>> if ! grep -q "function" available_tracers ; then
>>
>> May need to add that.
>
> Thanks! Shuah, can I update this patch?
> I'll send it asap.
>
> Thank you,
>
No worries. Take your time. I won't pull in until things settle down.
I noticed Steve gave you review comments.
thanks,
-- Shuah
On Tue, 26 Nov 2019 16:50:54 -0700
shuah <shuah@kernel.org> wrote:
> No worries. Take your time. I won't pull in until things settle down.
> I noticed Steve gave you review comments.
Masami's last patch should be good to go.
Thanks Shuah,
-- Steve