Message-ID: <Z0X8KJd5LYrgUJUh@x1>
Date: Tue, 26 Nov 2024 13:49:44 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Howard Chu <howardchu95@...il.com>
Cc: peterz@...radead.org, mingo@...hat.com, namhyung@...nel.org,
	mark.rutland@....com, alexander.shishkin@...ux.intel.com,
	jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] perf trace: Add tests for BTF general augmentation

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@...lbox 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@...lbox perf-tools-next]$

> Signed-off-by: Howard Chu <howardchu95@...il.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

Powered by blists - more mailing lists