Re: [PATCH] selftests/ftrace: let traceonoff_triggers testcase without md5sum

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 14 Aug 2023 10:59:18 +0800
Yipeng Zou <zouyipeng@huawei.com> wrote:

> So I directly dump the trace file before md5sum, the diff shows that:
> 
> [root@localhost]# diff trace_1.log trace_2.log -y --suppress-common-lines
> dockerd-12285   [036] d.... 18385.510290: sched_stat | <...>-12285   [036] d.... 18385.510290: sched_stat
> dockerd-12285   [036] d.... 18385.510291: sched_swit | <...>-12285   [036] d.... 18385.510291: sched_swit
> <...>-740       [044] d.... 18385.602859: sched_stat | kworker/44:1-740 [044] d.... 18385.602859: sched_stat
> <...>-740       [044] d.... 18385.602860: sched_swit | kworker/44:1-740 [044] d.... 18385.602860: sched_swit

So the problem is the <..> being added?

> 
> And we can see that the saved_cmdlines in kernel was changed.
> 
> So Maybe there is no safe to use md5sum here to verify whether
> the file whether it has changed.
> 
> >From my point of view, we only need to verify traceoff event  
> is valid there.
> 
> Maybe it is enough to check first that the tracing_on equals 0,
> and then check that the total number of rows of trace does not
> increase?
> 
> Fixes: d87b29179aa0 ("selftests: ftrace: Use md5sum to take less time of checking logs")
> Signed-off-by: Yipeng Zou <zouyipeng@huawei.com>
> ---
>  .../test.d/ftrace/func_traceonoff_triggers.tc    | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/ftrace/func_traceonoff_triggers.tc b/tools/testing/selftests/ftrace/test.d/ftrace/func_traceonoff_triggers.tc
> index aee22289536b..180c60605d7b 100644
> --- a/tools/testing/selftests/ftrace/test.d/ftrace/func_traceonoff_triggers.tc
> +++ b/tools/testing/selftests/ftrace/test.d/ftrace/func_traceonoff_triggers.tc
> @@ -77,25 +77,17 @@ if [ $cnt -ne 1 ]; then
>     fail "Did not find traceoff trigger"
>  fi
>  
> -cnt=`cnt_trace`
> -sleep $SLEEP_TIME
> -cnt2=`cnt_trace`
> -
> -if [ $cnt -ne $cnt2 ]; then
> -   fail "Tracing is not stopped"
> -fi
> -
>  on=`cat tracing_on`
>  if [ $on != "0" ]; then
>      fail "Tracing is not off"
>  fi
>  
> -csum1=`md5sum trace`

What about just changing the above to:

 # Can not rely on names being around as they are only cached
 csum1=`cat trace | sed -e 's/^ *[^ ]*\(-[0-9][0-9]*\)/\1/' | md5sum`

> +cnt=`cnt_trace`
>  sleep $SLEEP_TIME
> -csum2=`md5sum trace`

And this:

 csum2=`cat trace | sed -e 's/^ *[^ ]*\(-[0-9][0-9]*\)/\1/' | md5sum`

That will strip off the names an leave just the -[PID]... part of the line?

-- Steve



> +cnt2=`cnt_trace`
>  
> -if [ "$csum1" != "$csum2" ]; then
> -    fail "Tracing file is still changing"
> +if [ $cnt -ne $cnt2 ]; then
> +   fail "Tracing is not stopped"
>  fi
>  
>  clear_trace