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

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

[Howard Chu]

The previous version of the perf trace BTF general augmentation tests
didn't pass Shellcheck (thanks to Arnaldo Carvalho de Melo
<acme@kernel.org> for pointing this out), this version uses bash instead
of POSIX shell to pass Shellcheck.

This patch series also adds documentation for the new option
--force-btf, which is used in the tests.

Link: https://lore.kernel.org/linux-perf-users/Zt9yiQq-n-W6I274@x1/

Howard Chu (2):
  perf trace: Add tests for BTF general augmentation
  perf docs: Add documentation for --force-btf option

 tools/perf/Documentation/perf-trace.txt     |  3 +
 tools/perf/tests/shell/trace_btf_general.sh | 68 +++++++++++++++++++++
 2 files changed, 71 insertions(+)
 create mode 100755 tools/perf/tests/shell/trace_btf_general.sh

-- 
2.43.0

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

[Howard Chu]

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.

Signed-off-by: Howard Chu <howardchu95@gmail.com>
---
 tools/perf/tests/shell/trace_btf_general.sh | 68 +++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100755 tools/perf/tests/shell/trace_btf_general.sh

diff --git a/tools/perf/tests/shell/trace_btf_general.sh b/tools/perf/tests/shell/trace_btf_general.sh
new file mode 100755
index 000000000000..7bcca81a40d8
--- /dev/null
+++ b/tools/perf/tests/shell/trace_btf_general.sh
@@ -0,0 +1,68 @@
+#!/bin/bash
+# perf trace BTF general tests
+# SPDX-License-Identifier: GPL-2.0
+
+err=0
+set -e
+
+. "$(dirname $0)"/lib/probe.sh
+skip_if_no_perf_trace || exit 2
+
+file1=$(mktemp /tmp/file1_XXXXX)
+file2=$(echo $file1 | sed 's/file1/file2/g')
+
+buffer="the content of the buffer"
+
+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]+$"
+  then
+    echo "String augmentation test failed"
+    err=1
+  fi
+}
+
+trace_test_buffer() {
+  echo "Testing perf trace's buffer augmentation"
+  # echo will insert a newline (\10) at the end of the buffer
+  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"
+  then
+    echo "Buffer augmentation test failed"
+    err=1
+  fi
+}
+
+trace_test_struct_btf() {
+  echo "Testing perf trace's struct augmentation"
+  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"
+  then
+    echo "BTF struct augmentation test failed"
+    err=1
+  fi
+}
+
+cleanup() {
+	rm -rf ${file1} ${file2}
+}
+
+trap_cleanup() {
+	echo "Unexpected signal in ${FUNCNAME[1]}"
+	cleanup
+	exit 1
+}
+
+trace_test_string
+
+if [ $err = 0 ]; then
+  trace_test_buffer
+fi
+
+if [ $err = 0 ]; then
+  trace_test_struct_btf
+fi
+
+cleanup
+
+exit $err
-- 
2.43.0

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

[Howard Chu]

For the first patch, there is:


Suggested-by: Ian Rogers <irogers@google.com>

Thanks,
Howard

On Fri, Nov 22, 2024 at 4:55 PM Howard Chu <howardchu95@gmail.com> 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.
>
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/tests/shell/trace_btf_general.sh | 68 +++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100755 tools/perf/tests/shell/trace_btf_general.sh
>
> diff --git a/tools/perf/tests/shell/trace_btf_general.sh b/tools/perf/tests/shell/trace_btf_general.sh
> new file mode 100755
> index 000000000000..7bcca81a40d8
> --- /dev/null
> +++ b/tools/perf/tests/shell/trace_btf_general.sh
> @@ -0,0 +1,68 @@
> +#!/bin/bash
> +# perf trace BTF general tests
> +# SPDX-License-Identifier: GPL-2.0
> +
> +err=0
> +set -e
> +
> +. "$(dirname $0)"/lib/probe.sh
> +skip_if_no_perf_trace || exit 2
> +
> +file1=$(mktemp /tmp/file1_XXXXX)
> +file2=$(echo $file1 | sed 's/file1/file2/g')
> +
> +buffer="the content of the buffer"
> +
> +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]+$"
> +  then
> +    echo "String augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +trace_test_buffer() {
> +  echo "Testing perf trace's buffer augmentation"
> +  # echo will insert a newline (\10) at the end of the buffer
> +  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"
> +  then
> +    echo "Buffer augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +trace_test_struct_btf() {
> +  echo "Testing perf trace's struct augmentation"
> +  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"
> +  then
> +    echo "BTF struct augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +cleanup() {
> +       rm -rf ${file1} ${file2}
> +}
> +
> +trap_cleanup() {
> +       echo "Unexpected signal in ${FUNCNAME[1]}"
> +       cleanup
> +       exit 1
> +}
> +
> +trace_test_string
> +
> +if [ $err = 0 ]; then
> +  trace_test_buffer
> +fi
> +
> +if [ $err = 0 ]; then
> +  trace_test_struct_btf
> +fi
> +
> +cleanup
> +
> +exit $err
> --
> 2.43.0
>

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

[Arnaldo Carvalho de Melo]

On Fri, Nov 22, 2024 at 04:58:00PM -0800, Howard Chu wrote:
> For the first patch, there is:
> 
> 
> Suggested-by: Ian Rogers <irogers@google.com>

b4 somehow didn't like parts of this process:

⬢ [acme@toolbox perf-tools-next]$ b4 am -ctsl --cc-trailers 20241123005512.342079-1-howardchu95@gmail.com
Grabbing thread from lore.kernel.org/all/20241123005512.342079-1-howardchu95@gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 4 messages in the thread
Looking for additional code-review trailers on lore.kernel.org
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH v1 1/2] perf trace: Add tests for BTF general augmentation
    + Link: https://lore.kernel.org/r/20241123005512.342079-2-howardchu95@gmail.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ✓ [PATCH v1 2/2] perf docs: Add documentation for --force-btf option
    + Link: https://lore.kernel.org/r/20241123005512.342079-3-howardchu95@gmail.com
    + Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
  ---
  ✓ Signed: DKIM/gmail.com
---
Total patches: 2
---
NOTE: some trailers ignored due to from/email mismatches:
    ! Trailer: Suggested-by: Ian Rogers <irogers@google.com>
     Msg From: Howard Chu <howardchu95@gmail.com>
NOTE: Rerun with -S to apply them anyway
---
Cover: ./20241122_howardchu95_perf_trace_add_tests_for_btf_general_augmentation.cover
 Link: https://lore.kernel.org/r/20241123005512.342079-1-howardchu95@gmail.com
 Base: applies clean to current tree
       git checkout -b 20241122_howardchu95_gmail_com HEAD
       git am ./20241122_howardchu95_perf_trace_add_tests_for_btf_general_augmentation.mbx
⬢ [acme@toolbox perf-tools-next]$

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

[Arnaldo Carvalho de Melo]

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 \


⬢ [acme@toolbox perf-tools-next]$ rm -f 0001-* ; git format-patch --no-cover-letter HEAD~ ; scripts/checkpatch.pl 0001-*
0001-perf-trace-Add-tests-for-BTF-general-augmentation.patch
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#31: 
new file mode 100755

WARNING: Missing or malformed SPDX-License-Identifier tag in line 2
#37: FILE: tools/perf/tests/shell/trace_btf_general.sh:2:
+# perf trace BTF general tests

WARNING: Misplaced SPDX-License-Identifier tag - use line 2 instead
#38: FILE: tools/perf/tests/shell/trace_btf_general.sh:3:
+# SPDX-License-Identifier: GPL-2.0

WARNING: line length of 252 exceeds 100 columns
#55: FILE: tools/perf/tests/shell/trace_btf_general.sh:20:
+  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]+$"

WARNING: line length of 203 exceeds 100 columns
#65: FILE: tools/perf/tests/shell/trace_btf_general.sh:30:
+  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"

WARNING: line length of 275 exceeds 100 columns
#74: FILE: tools/perf/tests/shell/trace_btf_general.sh:39:
+  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"

total: 0 errors, 6 warnings, 68 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

0001-perf-trace-Add-tests-for-BTF-general-augmentation.patch has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
⬢ [acme@toolbox perf-tools-next]$

> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/tests/shell/trace_btf_general.sh | 68 +++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>  create mode 100755 tools/perf/tests/shell/trace_btf_general.sh
> 
> diff --git a/tools/perf/tests/shell/trace_btf_general.sh b/tools/perf/tests/shell/trace_btf_general.sh
> new file mode 100755
> index 000000000000..7bcca81a40d8
> --- /dev/null
> +++ b/tools/perf/tests/shell/trace_btf_general.sh
> @@ -0,0 +1,68 @@
> +#!/bin/bash
> +# perf trace BTF general tests
> +# SPDX-License-Identifier: GPL-2.0
> +
> +err=0
> +set -e
> +
> +. "$(dirname $0)"/lib/probe.sh
> +skip_if_no_perf_trace || exit 2
> +
> +file1=$(mktemp /tmp/file1_XXXXX)
> +file2=$(echo $file1 | sed 's/file1/file2/g')
> +
> +buffer="the content of the buffer"
> +
> +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]+$"
> +  then
> +    echo "String augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +trace_test_buffer() {
> +  echo "Testing perf trace's buffer augmentation"
> +  # echo will insert a newline (\10) at the end of the buffer
> +  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"
> +  then
> +    echo "Buffer augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +trace_test_struct_btf() {
> +  echo "Testing perf trace's struct augmentation"
> +  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"
> +  then
> +    echo "BTF struct augmentation test failed"
> +    err=1
> +  fi
> +}
> +
> +cleanup() {
> +	rm -rf ${file1} ${file2}
> +}
> +
> +trap_cleanup() {
> +	echo "Unexpected signal in ${FUNCNAME[1]}"
> +	cleanup
> +	exit 1
> +}
> +
> +trace_test_string
> +
> +if [ $err = 0 ]; then
> +  trace_test_buffer
> +fi
> +
> +if [ $err = 0 ]; then
> +  trace_test_struct_btf
> +fi
> +
> +cleanup
> +
> +exit $err
> -- 
> 2.43.0

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

[Arnaldo Carvalho de Melo]

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 \

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.

With this we also test setting the "PERF_CONFIG" environment variable
and, 'perf config' and 'perf trace' output customization code:

root@number:~# export PERF_CONFIG=my-tmp-perf-config
root@number:~# perf config trace.show_arg_names=false
root@number:~# cat my-tmp-perf-config 
# this file is auto-generated.
[trace]
	show_arg_names = false
root@number:~# rm -f before after ; touch before ; perf trace -e renameat2 mv before after
     0.000 ( 0.032 ms): :243836/243836 renameat2(CWD, "before", CWD, "after", NOREPLACE)                     = 0
root@number:~# perf config trace.show_duration=false
root@number:~# rm -f before after ; touch before ; perf trace -e renameat2 mv before after
     0.000 mv/243871 renameat2(CWD, "before", CWD, "after", NOREPLACE)                     = 0
root@number:~# perf config trace.show_timestamp=false
root@number:~# rm -f before after ; touch before ; perf trace -e renameat2 mv before after
:243888/243888 renameat2(CWD, "before", CWD, "after", NOREPLACE)                     = 0
root@number:~# perf config trace.args_alignment=0
root@number:~# rm -f before after ; touch before ; perf trace -e renameat2 mv before after
mv/243925 renameat2(CWD, "before", CWD, "after", NOREPLACE) = 0
root@number:~# cat my-tmp-perf-config 
# this file is auto-generated.
[trace]
	show_arg_names = false
	show_duration = false
	show_timestamp = false
	args_alignment = 0
root@number:~#  
> 
> ⬢ [acme@toolbox perf-tools-next]$ rm -f 0001-* ; git format-patch --no-cover-letter HEAD~ ; scripts/checkpatch.pl 0001-*
> 0001-perf-trace-Add-tests-for-BTF-general-augmentation.patch
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #31: 
> new file mode 100755
> 
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 2
> #37: FILE: tools/perf/tests/shell/trace_btf_general.sh:2:
> +# perf trace BTF general tests
> 
> WARNING: Misplaced SPDX-License-Identifier tag - use line 2 instead
> #38: FILE: tools/perf/tests/shell/trace_btf_general.sh:3:
> +# SPDX-License-Identifier: GPL-2.0
> 
> WARNING: line length of 252 exceeds 100 columns
> #55: FILE: tools/perf/tests/shell/trace_btf_general.sh:20:
> +  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]+$"
> 
> WARNING: line length of 203 exceeds 100 columns
> #65: FILE: tools/perf/tests/shell/trace_btf_general.sh:30:
> +  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"
> 
> WARNING: line length of 275 exceeds 100 columns
> #74: FILE: tools/perf/tests/shell/trace_btf_general.sh:39:
> +  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"
> 
> total: 0 errors, 6 warnings, 68 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
> 
> 0001-perf-trace-Add-tests-for-BTF-general-augmentation.patch has style problems, please review.
> 
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
> ⬢ [acme@toolbox perf-tools-next]$
> 
> > Signed-off-by: Howard Chu <howardchu95@gmail.com>
> > ---
> >  tools/perf/tests/shell/trace_btf_general.sh | 68 +++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100755 tools/perf/tests/shell/trace_btf_general.sh
> > 
> > diff --git a/tools/perf/tests/shell/trace_btf_general.sh b/tools/perf/tests/shell/trace_btf_general.sh
> > new file mode 100755
> > index 000000000000..7bcca81a40d8
> > --- /dev/null
> > +++ b/tools/perf/tests/shell/trace_btf_general.sh
> > @@ -0,0 +1,68 @@
> > +#!/bin/bash
> > +# perf trace BTF general tests
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +err=0
> > +set -e
> > +
> > +. "$(dirname $0)"/lib/probe.sh
> > +skip_if_no_perf_trace || exit 2
> > +
> > +file1=$(mktemp /tmp/file1_XXXXX)
> > +file2=$(echo $file1 | sed 's/file1/file2/g')
> > +
> > +buffer="the content of the buffer"
> > +
> > +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]+$"
> > +  then
> > +    echo "String augmentation test failed"
> > +    err=1
> > +  fi
> > +}
> > +
> > +trace_test_buffer() {
> > +  echo "Testing perf trace's buffer augmentation"
> > +  # echo will insert a newline (\10) at the end of the buffer
> > +  if ! perf trace -e write --max-events=1 -- echo "${buffer}" 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +echo\/[0-9]+ write\(fd: [0-9]+, buf: ${buffer}.*, count: [0-9]+\) += +[0-9]+$"
> > +  then
> > +    echo "Buffer augmentation test failed"
> > +    err=1
> > +  fi
> > +}
> > +
> > +trace_test_struct_btf() {
> > +  echo "Testing perf trace's struct augmentation"
> > +  if ! perf trace -e clock_nanosleep --force-btf --max-events=1 -- sleep 1 2>&1 | grep -q -E " +[0-9]+\.[0-9]+ +\( *[0-9]+\.[0-9]+ ms\): +sleep\/[0-9]+ clock_nanosleep\(rqtp: \(struct __kernel_timespec\)\{\.tv_sec = \(__kernel_time64_t\)1,\}, rmtp: 0x[0-9a-f]+\) += +[0-9]+$"
> > +  then
> > +    echo "BTF struct augmentation test failed"
> > +    err=1
> > +  fi
> > +}
> > +
> > +cleanup() {
> > +	rm -rf ${file1} ${file2}
> > +}
> > +
> > +trap_cleanup() {
> > +	echo "Unexpected signal in ${FUNCNAME[1]}"
> > +	cleanup
> > +	exit 1
> > +}
> > +
> > +trace_test_string
> > +
> > +if [ $err = 0 ]; then
> > +  trace_test_buffer
> > +fi
> > +
> > +if [ $err = 0 ]; then
> > +  trace_test_struct_btf
> > +fi
> > +
> > +cleanup
> > +
> > +exit $err
> > -- 
> > 2.43.0

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@kernel.org> 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

[PATCH v1 2/2] perf docs: Add documentation for --force-btf option

[Howard Chu]

The --force-btf option is intended for debugging purposes and is
currently undocumented. Add documentation for it.

Signed-off-by: Howard Chu <howardchu95@gmail.com>
---
 tools/perf/Documentation/perf-trace.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/Documentation/perf-trace.txt b/tools/perf/Documentation/perf-trace.txt
index 6e0cc50bbc13..529081599cc7 100644
--- a/tools/perf/Documentation/perf-trace.txt
+++ b/tools/perf/Documentation/perf-trace.txt
@@ -241,6 +241,9 @@ the thread executes on the designated CPUs. Default is to monitor all CPUs.
 	printing using the existing 'perf trace' syscall arg beautifiers to map integer
 	arguments to strings (pid to comm, syscall id to syscall name, etc).
 
+--force-btf::
+	Use btf_dump to pretty print syscall argument data, instead of using hand-crafted pretty printers.
+
 
 PAGEFAULTS
 ----------
-- 
2.43.0

Re: [PATCH v1 2/2] perf docs: Add documentation for --force-btf option

[Arnaldo Carvalho de Melo]

On Fri, Nov 22, 2024 at 04:55:11PM -0800, Howard Chu wrote:
> The --force-btf option is intended for debugging purposes and is
> currently undocumented. Add documentation for it.
> 
> Signed-off-by: Howard Chu <howardchu95@gmail.com>
> ---
>  tools/perf/Documentation/perf-trace.txt | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-trace.txt b/tools/perf/Documentation/perf-trace.txt
> index 6e0cc50bbc13..529081599cc7 100644
> --- a/tools/perf/Documentation/perf-trace.txt
> +++ b/tools/perf/Documentation/perf-trace.txt
> @@ -241,6 +241,9 @@ the thread executes on the designated CPUs. Default is to monitor all CPUs.
>  	printing using the existing 'perf trace' syscall arg beautifiers to map integer
>  	arguments to strings (pid to comm, syscall id to syscall name, etc).
>  
> +--force-btf::
> +	Use btf_dump to pretty print syscall argument data, instead of using hand-crafted pretty printers.
> +

I think here we could expand a bit and explain that we want this for
testing the BTF code, and also mention that the hand-crafted pretty
printers know about integers -> strings, how to pretty print flags, etc
that BTF doesn't.

- Arnaldo

>  
>  PAGEFAULTS
>  ----------
> -- 
> 2.43.0