Re: [PATCH v1 1/2] perf trace: Add tests for BTF general augmentation

[Howard Chu]

Hello Arnaldo,

On Tue, Nov 26, 2024 at 9:12 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> On Tue, Nov 26, 2024 at 01:49:44PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Fri, Nov 22, 2024 at 04:55:10PM -0800, Howard Chu wrote:
> > > Currently, we only have perf trace augmentation tests for enum
> > > arguments. This patch adds tests for more general syscall arguments,
> > > such as struct pointers, strings, and buffers.
> >
> > scripts/checkpatch.pl has some warnings here I think we can address
> > easily, some not so much, like the SPDX that we need to add logic to
> > 'perf test' to noticed its the SPDX and skip it, looking at the next
> > line for the test description.
> >
> >
> > The long lines we can just make them multiple lines with the first ones
> > ending in \

Sure, thanks.

>
> And while testing it with -vv:
>
> root@number:~# perf test -vv btf
> 110: perf trace BTF general tests:
> --- start ---
> test child forked, pid 242944
> Testing perf trace's string augmentation
> grep: warning: stray \ before /
> Testing perf trace's buffer augmentation
> grep: warning: stray \ before /
> Testing perf trace's struct augmentation
> grep: warning: stray \ before /
> ---- end(0) ----
> 110: perf trace BTF general tests                                    : Ok
> root@number:~#
>
> The long lines can be solved like:
>
> +++ b/tools/perf/tests/shell/trace_btf_general.sh
> @@ -17,7 +17,8 @@ trap cleanup EXIT TERM INT HUP
>
>  trace_test_string() {
>    echo "Testing perf trace's string augmentation"
> -  if ! perf trace -e renameat* --max-events=1 -- mv ${file1} ${file2} 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +mv\/[0-9]+ renameat(2)?\(olddfd: .*, oldname: \"${file1}\", newdfd: .*, newname: \"${file2}\", flags: .*\) += +[0-9]+$"
> +  if ! perf trace -e renameat* --max-events=1 -- mv ${file1} ${file2} 2>&1 | \
> +     grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +mv\/[0-9]+ renameat(2)?\(olddfd: .*, oldname: \"${file1}\", newdfd: .*, newname: \"${file2}\", flags: .*\) += +[0-9]+$"
>    then
>      echo "String augmentation test failed"
>      err=1
>
> And that " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +mv\/[0-9]+ : +"
>
> part is common to several such greps, so you could have it as:
>
> prefix=" +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +mv\/[0-9]+ : +"
>
> And then use
>
> grep -q -E "%{prefix}mv\/[0-9]+ renameat(2)?\(olddfd: .*, oldname: \"${file1}\", newdfd: .*, newname: \"${file2}\", flags: .*\) += +[0-9]+$"
>
> Or, since this part isn't interesting for what this test checks, BPF
> augmentation, we could make it all more compact, i.e.:
>
> From the default that is:
>
> root@number:~# rm -f before after ; touch before ; perf trace -e renameat2 mv before after
>      0.000 ( 0.037 ms): mv/243278 renameat2(olddfd: CWD, oldname: "before", newdfd: CWD, newname: "after", flags: NOREPLACE) = 0
> root@number:~#
>
> We point perf to a temporary config file, using mktemp, then tell it to
> not output the stuff we don't need while test BPF augmentation, making
> the output more compact and thus the regexps in the perf test shorter.

Sure I'll do that.

Thanks,
Howard