linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test
@ 2022-09-12  8:34 Adrian Hunter
  2022-09-12  8:34 ` [PATCH 01/11] perf test: test_intel_pt.sh: Add cleanup function Adrian Hunter
                   ` (11 more replies)
  0 siblings, 12 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

Hi

Here is a new per-thread test for the test_intel_pt.sh test script.

The first 9 patches are tidy-ups for the script, mostly based on results
from the shellcheck utility.

The 10th patch adds debug prints that the script will capture to help
verify correct operation.

The final patch actually adds the new test.


Adrian Hunter (11):
      perf test: test_intel_pt.sh: Add cleanup function
      perf test: test_intel_pt.sh: Use a temp directory
      perf test: test_intel_pt.sh: Fix redirection
      perf test: test_intel_pt.sh: Stop using expr
      perf test: test_intel_pt.sh: Stop using backticks
      perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l
      perf test: test_intel_pt.sh: Use quotes around variable expansion
      perf test: test_intel_pt.sh: Fix return checking
      perf test: test_intel_pt.sh: Add more output in preparation for more tests
      perf tools: Add debug messages and comments for testing
      perf test: test_intel_pt.sh: Add per-thread test

 tools/lib/perf/evlist.c                 |   2 +
 tools/perf/builtin-record.c             |   8 +
 tools/perf/tests/shell/test_intel_pt.sh | 307 ++++++++++++++++++++++++++++++--
 tools/perf/util/evsel.c                 |   2 +
 4 files changed, 304 insertions(+), 15 deletions(-)


Regards
Adrian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 01/11] perf test: test_intel_pt.sh: Add cleanup function
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 02/11] perf test: test_intel_pt.sh: Use a temp directory Adrian Hunter
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

Add a cleanup function that will still clean up if the script is
terminated prematurely.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index a3298643884d..17338e6a6f99 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -14,6 +14,21 @@ err_cnt=0
 tmpfile=`mktemp`
 perfdatafile=`mktemp`
 
+cleanup()
+{
+	trap - EXIT TERM INT
+	rm -f ${tmpfile}
+	rm -f ${perfdatafile}
+}
+
+trap_cleanup()
+{
+	cleanup
+	exit 1
+}
+
+trap trap_cleanup EXIT TERM INT
+
 can_cpu_wide()
 {
 	perf record -o ${tmpfile} -B -N --no-bpf-event -e dummy:u -C $1 true 2>&1 >/dev/null || return 2
@@ -57,8 +72,7 @@ test_system_wide_side_band
 
 count_result $?
 
-rm -f ${tmpfile}
-rm -f ${perfdatafile}
+cleanup
 
 if [ ${err_cnt} -gt 0 ] ; then
 	exit 1
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 02/11] perf test: test_intel_pt.sh: Use a temp directory
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
  2022-09-12  8:34 ` [PATCH 01/11] perf test: test_intel_pt.sh: Add cleanup function Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 03/11] perf test: test_intel_pt.sh: Fix redirection Adrian Hunter
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

Create a directory for temporary files so that mktemp needs to be used
only once. It also enables more temp files to be added without having to
add them also to the cleanup.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 17338e6a6f99..872ee0d89d38 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -11,14 +11,20 @@ skip_cnt=0
 ok_cnt=0
 err_cnt=0
 
-tmpfile=`mktemp`
-perfdatafile=`mktemp`
+temp_dir=$(mktemp -d /tmp/perf-test-intel-pt-sh.XXXXXXXXXX)
+
+tmpfile="${temp_dir}/tmp-perf.data"
+perfdatafile="${temp_dir}/test-perf.data"
 
 cleanup()
 {
 	trap - EXIT TERM INT
-	rm -f ${tmpfile}
-	rm -f ${perfdatafile}
+	sane=$(echo "${temp_dir}" | cut -b 1-26)
+	if [ "${sane}" = "/tmp/perf-test-intel-pt-sh" ] ; then
+		echo "--- Cleaning up ---"
+		rm -f "${temp_dir}/"*
+		rmdir "${temp_dir}"
+	fi
 }
 
 trap_cleanup()
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 03/11] perf test: test_intel_pt.sh: Fix redirection
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
  2022-09-12  8:34 ` [PATCH 01/11] perf test: test_intel_pt.sh: Add cleanup function Adrian Hunter
  2022-09-12  8:34 ` [PATCH 02/11] perf test: test_intel_pt.sh: Use a temp directory Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 04/11] perf test: test_intel_pt.sh: Stop using expr Adrian Hunter
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

As reported by shellcheck, 2>&1 must come after >/dev/null

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 872ee0d89d38..6e40ee7261da 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -37,7 +37,7 @@ trap trap_cleanup EXIT TERM INT
 
 can_cpu_wide()
 {
-	perf record -o ${tmpfile} -B -N --no-bpf-event -e dummy:u -C $1 true 2>&1 >/dev/null || return 2
+	perf record -o ${tmpfile} -B -N --no-bpf-event -e dummy:u -C $1 true >/dev/null 2>&1 || return 2
 	return 0
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 04/11] perf test: test_intel_pt.sh: Stop using expr
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (2 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 03/11] perf test: test_intel_pt.sh: Fix redirection Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 05/11] perf test: test_intel_pt.sh: Stop using backticks Adrian Hunter
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

As suggested by shellcheck, stop using expr.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 6e40ee7261da..2be8cb03a620 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -64,14 +64,14 @@ test_system_wide_side_band()
 count_result()
 {
 	if [ $1 -eq 2 ] ; then
-		skip_cnt=`expr ${skip_cnt} \+ 1`
+		skip_cnt=$((skip_cnt + 1))
 		return
 	fi
 	if [ $1 -eq 0 ] ; then
-		ok_cnt=`expr ${ok_cnt} \+ 1`
+		ok_cnt=$((ok_cnt + 1))
 		return
 	fi
-	err_cnt=`expr ${err_cnt} \+ 1`
+	err_cnt=$((err_cnt + 1))
 }
 
 test_system_wide_side_band
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 05/11] perf test: test_intel_pt.sh: Stop using backticks
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (3 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 04/11] perf test: test_intel_pt.sh: Stop using expr Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 06/11] perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l Adrian Hunter
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

As suggested by shellcheck, stop using backticks.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 2be8cb03a620..0273332b99e9 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -51,7 +51,7 @@ test_system_wide_side_band()
 	perf record -B -N --no-bpf-event -o ${perfdatafile} -e intel_pt//u -C 0 -- taskset --cpu-list 1 uname
 
 	# Should get MMAP events from CPU 1 because they can be needed to decode
-	mmap_cnt=`perf script -i ${perfdatafile} --no-itrace --show-mmap-events -C 1 2>/dev/null | grep MMAP | wc -l`
+	mmap_cnt=$(perf script -i ${perfdatafile} --no-itrace --show-mmap-events -C 1 2>/dev/null | grep MMAP | wc -l)
 
 	if [ ${mmap_cnt} -gt 0 ] ; then
 		return 0
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 06/11] perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (4 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 05/11] perf test: test_intel_pt.sh: Stop using backticks Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 07/11] perf test: test_intel_pt.sh: Use quotes around variable expansion Adrian Hunter
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

As suggested by shellcheck, use grep -c instead of grep plus wc -l

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 0273332b99e9..3dfdef4fa6f4 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -51,7 +51,7 @@ test_system_wide_side_band()
 	perf record -B -N --no-bpf-event -o ${perfdatafile} -e intel_pt//u -C 0 -- taskset --cpu-list 1 uname
 
 	# Should get MMAP events from CPU 1 because they can be needed to decode
-	mmap_cnt=$(perf script -i ${perfdatafile} --no-itrace --show-mmap-events -C 1 2>/dev/null | grep MMAP | wc -l)
+	mmap_cnt=$(perf script -i ${perfdatafile} --no-itrace --show-mmap-events -C 1 2>/dev/null | grep -c MMAP)
 
 	if [ ${mmap_cnt} -gt 0 ] ; then
 		return 0
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 07/11] perf test: test_intel_pt.sh: Use quotes around variable expansion
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (5 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 06/11] perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 08/11] perf test: test_intel_pt.sh: Fix return checking Adrian Hunter
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

As suggested by shellcheck, use quotes around variable expansion.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 3dfdef4fa6f4..075b780fe9ed 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -37,7 +37,7 @@ trap trap_cleanup EXIT TERM INT
 
 can_cpu_wide()
 {
-	perf record -o ${tmpfile} -B -N --no-bpf-event -e dummy:u -C $1 true >/dev/null 2>&1 || return 2
+	perf record -o "${tmpfile}" -B -N --no-bpf-event -e dummy:u -C "$1" true >/dev/null 2>&1 || return 2
 	return 0
 }
 
@@ -48,12 +48,12 @@ test_system_wide_side_band()
 	can_cpu_wide 1 || return $?
 
 	# Record on CPU 0 a task running on CPU 1
-	perf record -B -N --no-bpf-event -o ${perfdatafile} -e intel_pt//u -C 0 -- taskset --cpu-list 1 uname
+	perf record -B -N --no-bpf-event -o "${perfdatafile}" -e intel_pt//u -C 0 -- taskset --cpu-list 1 uname
 
 	# Should get MMAP events from CPU 1 because they can be needed to decode
-	mmap_cnt=$(perf script -i ${perfdatafile} --no-itrace --show-mmap-events -C 1 2>/dev/null | grep -c MMAP)
+	mmap_cnt=$(perf script -i "${perfdatafile}" --no-itrace --show-mmap-events -C 1 2>/dev/null | grep -c MMAP)
 
-	if [ ${mmap_cnt} -gt 0 ] ; then
+	if [ "${mmap_cnt}" -gt 0 ] ; then
 		return 0
 	fi
 
@@ -63,11 +63,11 @@ test_system_wide_side_band()
 
 count_result()
 {
-	if [ $1 -eq 2 ] ; then
+	if [ "$1" -eq 2 ] ; then
 		skip_cnt=$((skip_cnt + 1))
 		return
 	fi
-	if [ $1 -eq 0 ] ; then
+	if [ "$1" -eq 0 ] ; then
 		ok_cnt=$((ok_cnt + 1))
 		return
 	fi
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 08/11] perf test: test_intel_pt.sh: Fix return checking
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (6 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 07/11] perf test: test_intel_pt.sh: Use quotes around variable expansion Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 09/11] perf test: test_intel_pt.sh: Add more output in preparation for more tests Adrian Hunter
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

The use of set -e will cause a function that returns non-zero to terminate
the script unless the result is consumed by || for example. That is OK if
there is only 1 test function, but not if there are more. Prepare for more
by using ||.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 075b780fe9ed..7d2f3136ce19 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -72,11 +72,11 @@ count_result()
 		return
 	fi
 	err_cnt=$((err_cnt + 1))
+	ret=0
 }
 
-test_system_wide_side_band
-
-count_result $?
+ret=0
+test_system_wide_side_band || ret=$? ; count_result $ret
 
 cleanup
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 09/11] perf test: test_intel_pt.sh: Add more output in preparation for more tests
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (7 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 08/11] perf test: test_intel_pt.sh: Fix return checking Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 10/11] perf tools: Add debug messages and comments for testing Adrian Hunter
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

When there are more tests it won't be obvious which test failed. Add more
output so that it is.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 7d2f3136ce19..2d489de9097b 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -37,12 +37,19 @@ trap trap_cleanup EXIT TERM INT
 
 can_cpu_wide()
 {
-	perf record -o "${tmpfile}" -B -N --no-bpf-event -e dummy:u -C "$1" true >/dev/null 2>&1 || return 2
+	echo "Checking for CPU-wide recording on CPU $1"
+	if ! perf record -o "${tmpfile}" -B -N --no-bpf-event -e dummy:u -C "$1" true >/dev/null 2>&1 ; then
+		echo "No so skipping"
+		return 2
+	fi
+	echo OK
 	return 0
 }
 
 test_system_wide_side_band()
 {
+	echo "--- Test system-wide sideband ---"
+
 	# Need CPU 0 and CPU 1
 	can_cpu_wide 0 || return $?
 	can_cpu_wide 1 || return $?
@@ -54,6 +61,7 @@ test_system_wide_side_band()
 	mmap_cnt=$(perf script -i "${perfdatafile}" --no-itrace --show-mmap-events -C 1 2>/dev/null | grep -c MMAP)
 
 	if [ "${mmap_cnt}" -gt 0 ] ; then
+		echo OK
 		return 0
 	fi
 
@@ -80,6 +88,8 @@ test_system_wide_side_band || ret=$? ; count_result $ret
 
 cleanup
 
+echo "--- Done ---"
+
 if [ ${err_cnt} -gt 0 ] ; then
 	exit 1
 fi
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 10/11] perf tools: Add debug messages and comments for testing
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (8 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 09/11] perf test: test_intel_pt.sh: Add more output in preparation for more tests Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-12  8:34 ` [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
  2022-09-13 17:41 ` [PATCH 00/11] " Namhyung Kim
  11 siblings, 0 replies; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

Add debug messages to enable scripts to track aspects of perf record
behaviour. The messages will be consumed after perf record has run,
with the exception of "perf record has started" which is consequently
flushed.

Put comments so developers know which messages are also being used by test
scripts.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/lib/perf/evlist.c     | 2 ++
 tools/perf/builtin-record.c | 8 ++++++++
 tools/perf/util/evsel.c     | 2 ++
 3 files changed, 12 insertions(+)

diff --git a/tools/lib/perf/evlist.c b/tools/lib/perf/evlist.c
index 6b1bafe267a4..80cc810c5097 100644
--- a/tools/lib/perf/evlist.c
+++ b/tools/lib/perf/evlist.c
@@ -486,6 +486,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
 			if (ops->idx)
 				ops->idx(evlist, evsel, mp, idx);
 
+			/* Debug message used by test scripts */
 			pr_debug("idx %d: mmapping fd %d\n", idx, *output);
 			if (ops->mmap(map, mp, *output, evlist_cpu) < 0)
 				return -1;
@@ -495,6 +496,7 @@ mmap_per_evsel(struct perf_evlist *evlist, struct perf_evlist_mmap_ops *ops,
 			if (!idx)
 				perf_evlist__set_mmap_first(evlist, map, overwrite);
 		} else {
+			/* Debug message used by test scripts */
 			pr_debug("idx %d: set output fd %d -> %d\n", idx, fd, *output);
 			if (ioctl(fd, PERF_EVENT_IOC_SET_OUTPUT, *output) != 0)
 				return -1;
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 02e38f50a138..5b808ac7a281 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2428,10 +2428,14 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 
 	record__uniquify_name(rec);
 
+	/* Debug message used by test scripts */
+	pr_debug3("perf record opening and mmapping events\n");
 	if (record__open(rec) != 0) {
 		err = -1;
 		goto out_free_threads;
 	}
+	/* Debug message used by test scripts */
+	pr_debug3("perf record done opening and mmapping events\n");
 	session->header.env.comp_mmap_len = session->evlist->core.mmap_len;
 
 	if (rec->opts.kcore) {
@@ -2574,6 +2578,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	if (err)
 		goto out_child;
 
+	/* Debug message used by test scripts */
+	pr_debug3("perf record has started\n");
+	fflush(stderr);
+
 	trigger_ready(&auxtrace_snapshot_trigger);
 	trigger_ready(&switch_output_trigger);
 	perf_hooks__invoke_record_start();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5776bfa70f11..a27092339b81 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2089,6 +2089,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 
 			test_attr__ready();
 
+			/* Debug message used by test scripts */
 			pr_debug2_peo("sys_perf_event_open: pid %d  cpu %d  group_fd %d  flags %#lx",
 				pid, perf_cpu_map__cpu(cpus, idx).cpu, group_fd, evsel->open_flags);
 
@@ -2114,6 +2115,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,
 						fd, group_fd, evsel->open_flags);
 			}
 
+			/* Debug message used by test scripts */
 			pr_debug2_peo(" = %d\n", fd);
 
 			if (evsel->bpf_fd >= 0) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (9 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 10/11] perf tools: Add debug messages and comments for testing Adrian Hunter
@ 2022-09-12  8:34 ` Adrian Hunter
  2022-09-13 17:37   ` Namhyung Kim
  2022-09-13 17:41 ` [PATCH 00/11] " Namhyung Kim
  11 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2022-09-12  8:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Jiri Olsa, Namhyung Kim, Ian Rogers, linux-kernel,
	linux-perf-users

When tracing the kernel with Intel PT, text_poke events are recorded
per-cpu. In per-thread mode that results in a mixture of per-thread and
per-cpu events and mmaps. Check that happens correctly.

The debug output from perf record -vvv is recorded and then awk used to
process the debug messages that indicate what file descriptors were opened
and whether they were mmapped or set-output.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/shell/test_intel_pt.sh | 247 ++++++++++++++++++++++++
 1 file changed, 247 insertions(+)

diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
index 2d489de9097b..051d088c1b74 100755
--- a/tools/perf/tests/shell/test_intel_pt.sh
+++ b/tools/perf/tests/shell/test_intel_pt.sh
@@ -4,6 +4,8 @@
 
 set -e
 
+tenths=date\ +%s%1N
+
 # Skip if no Intel PT
 perf list | grep -q 'intel_pt//' || exit 2
 
@@ -15,6 +17,10 @@ temp_dir=$(mktemp -d /tmp/perf-test-intel-pt-sh.XXXXXXXXXX)
 
 tmpfile="${temp_dir}/tmp-perf.data"
 perfdatafile="${temp_dir}/test-perf.data"
+outfile="${temp_dir}/test-out.txt"
+errfile="${temp_dir}/test-err.txt"
+workload="${temp_dir}/workload"
+awkscript="${temp_dir}/awkscript"
 
 cleanup()
 {
@@ -35,6 +41,37 @@ trap_cleanup()
 
 trap trap_cleanup EXIT TERM INT
 
+have_workload=false
+cat << _end_of_file_ | /usr/bin/cc -o "${workload}" -xc - -pthread && have_workload=true
+#include <time.h>
+#include <pthread.h>
+
+void work(void) {
+	struct timespec tm = {
+		.tv_nsec = 1000000,
+	};
+	int i;
+
+	/* Run for about 30 seconds */
+	for (i = 0; i < 30000; i++)
+		nanosleep(&tm, NULL);
+}
+
+void *threadfunc(void *arg) {
+	work();
+	return NULL;
+}
+
+int main(void) {
+	pthread_t th;
+
+	pthread_create(&th, NULL, threadfunc, NULL);
+	work();
+	pthread_join(th, NULL);
+	return 0;
+}
+_end_of_file_
+
 can_cpu_wide()
 {
 	echo "Checking for CPU-wide recording on CPU $1"
@@ -69,6 +106,214 @@ test_system_wide_side_band()
 	return 1
 }
 
+can_kernel()
+{
+	perf record -o "${tmpfile}" -B -N --no-bpf-event -e dummy:k true >/dev/null 2>&1 || return 2
+	return 0
+}
+
+wait_for_threads()
+{
+	start_time=$($tenths)
+	while [ -e "/proc/$1/task" ] ; do
+		th_cnt=$(find "/proc/$1/task" -mindepth 1 -maxdepth 1 -printf x | wc -c)
+		if [ "${th_cnt}" -ge "$2" ] ; then
+			return 0
+		fi
+		# Wait at most 5 seconds
+		if [ $(($($tenths) - start_time)) -ge 50 ] ; then
+			echo "PID $1 does not have $2 threads"
+			return 1
+		fi
+	done
+	return 1
+}
+
+wait_for_perf_to_start()
+{
+	echo "Waiting for \"perf record has started\" message"
+	start_time=$($tenths)
+	while [ -e "/proc/$1" ] ; do
+		if grep -q "perf record has started" "${errfile}" ; then
+			echo OK
+			break
+		fi
+		# Wait at most 5 seconds
+		if [ $(($($tenths) - start_time)) -ge 50 ] ; then
+			echo "perf recording did not start"
+			return 1
+		fi
+	done
+	return 0
+}
+
+wait_for_process_to_exit()
+{
+	start_time=$($tenths)
+	while [ -e "/proc/$1" ] ; do
+		# Wait at most 5 seconds
+		if [ $(($($tenths) - start_time)) -ge 50 ] ; then
+			echo "PID $1 did not exit as expected"
+			return 1
+		fi
+	done
+	return 0
+}
+
+is_running()
+{
+	start_time=$($tenths)
+	while [ -e "/proc/$1" ] ; do
+		# Check for at least 0.3s
+		if [ $(($($tenths) - start_time)) -gt 3 ] ; then
+			return 0
+		fi
+	done
+	echo "PID $1 exited prematurely"
+	return 1
+}
+
+test_per_thread()
+{
+	k="$1"
+	desc="$2"
+
+	echo "--- Test per-thread ${desc}recording ---"
+
+	if ! $have_workload ; then
+		echo "No workload, so skipping"
+		return 2
+	fi
+
+	if [ "${k}" = "k" ] ; then
+		can_kernel || return 2
+	fi
+
+	cat <<- "_end_of_file_" > "${awkscript}"
+	BEGIN {
+		s = "[ ]*"
+		u = s"[0-9]+"s
+		d = s"[0-9-]+"s
+		x = s"[0-9a-fA-FxX]+"s
+		mmapping = "idx"u": mmapping fd"u
+		set_output = "idx"u": set output fd"u"->"u
+		perf_event_open = "sys_perf_event_open: pid"d"cpu"d"group_fd"d"flags"x"="u
+	}
+
+	/perf record opening and mmapping events/ {
+		if (!done)
+			active = 1
+	}
+
+	/perf record done opening and mmapping events/ {
+		active = 0
+		done = 1
+	}
+
+	$0 ~ perf_event_open && active {
+		match($0, perf_event_open)
+		$0 = substr($0, RSTART, RLENGTH)
+		pid = $3
+		cpu = $5
+		fd = $11
+		print "pid " pid " cpu " cpu " fd " fd " : " $0
+		fd_array[fd] = fd
+		pid_array[fd] = pid
+		cpu_array[fd] = cpu
+	}
+
+	$0 ~ mmapping && active  {
+		match($0, mmapping)
+		$0 = substr($0, RSTART, RLENGTH)
+		fd = $5
+		print "fd " fd " : " $0
+		if (fd in fd_array) {
+			mmap_array[fd] = 1
+		} else {
+			print "Unknown fd " fd
+			exit 1
+		}
+	}
+
+	$0 ~ set_output && active {
+		match($0, set_output)
+		$0 = substr($0, RSTART, RLENGTH)
+		fd = $6
+		fd_to = $8
+		print "fd " fd " fd_to " fd_to " : " $0
+		if (fd in fd_array) {
+			if (fd_to in fd_array) {
+				set_output_array[fd] = fd_to
+			} else {
+				print "Unknown fd " fd_to
+				exit 1
+			}
+		} else {
+			print "Unknown fd " fd
+			exit 1
+		}
+	}
+
+	END {
+		print "Checking " length(fd_array) " fds"
+		for (fd in fd_array) {
+			if (fd in mmap_array) {
+				pid = pid_array[fd]
+				if (pid != -1) {
+					if (pid in pids) {
+						print "More than 1 mmap for PID " pid
+						exit 1
+					}
+					pids[pid] = 1
+				}
+				cpu = cpu_array[fd]
+				if (cpu != -1) {
+					if (cpu in cpus) {
+						print "More than 1 mmap for CPU " cpu
+						exit 1
+					}
+					cpus[cpu] = 1
+				}
+			} else if (!(fd in set_output_array)) {
+				print "No mmap for fd " fd
+				exit 1
+			}
+		}
+		n = length(pids)
+		if (n != thread_cnt) {
+			print "Expected " thread_cnt " per-thread mmaps - found " n
+			exit 1
+		}
+	}
+	_end_of_file_
+
+	$workload &
+	w1=$!
+	$workload &
+	w2=$!
+	echo "Workload PIDs are $w1 and $w2"
+	wait_for_threads ${w1} 2
+	wait_for_threads ${w2} 2
+
+	perf record -B -N --no-bpf-event -o "${perfdatafile}" -e intel_pt//u"${k}" -vvv --per-thread -p "${w1},${w2}" 2>"${errfile}" >"${outfile}" &
+	ppid=$!
+	echo "perf PID is $ppid"
+	wait_for_perf_to_start ${ppid} || return 1
+
+	kill ${w1}
+	wait_for_process_to_exit ${w1} || return 1
+	is_running ${ppid} || return 1
+
+	kill ${w2}
+	wait_for_process_to_exit ${w2} || return 1
+	wait_for_process_to_exit ${ppid} || return 1
+
+	awk -v thread_cnt=4 -f "${awkscript}" "${errfile}" || return 1
+
+	echo OK
+	return 0
+}
+
 count_result()
 {
 	if [ "$1" -eq 2 ] ; then
@@ -85,6 +330,8 @@ count_result()
 
 ret=0
 test_system_wide_side_band || ret=$? ; count_result $ret
+test_per_thread "" "" || ret=$? ; count_result $ret
+test_per_thread "k" "(incl. kernel) " || ret=$? ; count_result $ret
 
 cleanup
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test
  2022-09-12  8:34 ` [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
@ 2022-09-13 17:37   ` Namhyung Kim
  0 siblings, 0 replies; 16+ messages in thread
From: Namhyung Kim @ 2022-09-13 17:37 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, linux-kernel,
	linux-perf-users

On Mon, Sep 12, 2022 at 1:34 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> When tracing the kernel with Intel PT, text_poke events are recorded
> per-cpu. In per-thread mode that results in a mixture of per-thread and
> per-cpu events and mmaps. Check that happens correctly.
>
> The debug output from perf record -vvv is recorded and then awk used to
> process the debug messages that indicate what file descriptors were opened
> and whether they were mmapped or set-output.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
>  tools/perf/tests/shell/test_intel_pt.sh | 247 ++++++++++++++++++++++++
>  1 file changed, 247 insertions(+)
>
> diff --git a/tools/perf/tests/shell/test_intel_pt.sh b/tools/perf/tests/shell/test_intel_pt.sh
> index 2d489de9097b..051d088c1b74 100755
> --- a/tools/perf/tests/shell/test_intel_pt.sh
> +++ b/tools/perf/tests/shell/test_intel_pt.sh
> @@ -4,6 +4,8 @@
>
>  set -e
>
> +tenths=date\ +%s%1N
> +
>  # Skip if no Intel PT
>  perf list | grep -q 'intel_pt//' || exit 2
>
> @@ -15,6 +17,10 @@ temp_dir=$(mktemp -d /tmp/perf-test-intel-pt-sh.XXXXXXXXXX)
>
>  tmpfile="${temp_dir}/tmp-perf.data"
>  perfdatafile="${temp_dir}/test-perf.data"
> +outfile="${temp_dir}/test-out.txt"
> +errfile="${temp_dir}/test-err.txt"
> +workload="${temp_dir}/workload"
> +awkscript="${temp_dir}/awkscript"
>
>  cleanup()
>  {
> @@ -35,6 +41,37 @@ trap_cleanup()
>
>  trap trap_cleanup EXIT TERM INT
>
> +have_workload=false
> +cat << _end_of_file_ | /usr/bin/cc -o "${workload}" -xc - -pthread && have_workload=true
> +#include <time.h>
> +#include <pthread.h>
> +
> +void work(void) {
> +       struct timespec tm = {
> +               .tv_nsec = 1000000,
> +       };
> +       int i;
> +
> +       /* Run for about 30 seconds */
> +       for (i = 0; i < 30000; i++)
> +               nanosleep(&tm, NULL);
> +}
> +
> +void *threadfunc(void *arg) {
> +       work();
> +       return NULL;
> +}
> +
> +int main(void) {
> +       pthread_t th;
> +
> +       pthread_create(&th, NULL, threadfunc, NULL);
> +       work();
> +       pthread_join(th, NULL);
> +       return 0;
> +}
> +_end_of_file_
> +
>  can_cpu_wide()
>  {
>         echo "Checking for CPU-wide recording on CPU $1"
> @@ -69,6 +106,214 @@ test_system_wide_side_band()
>         return 1
>  }
>
> +can_kernel()
> +{
> +       perf record -o "${tmpfile}" -B -N --no-bpf-event -e dummy:k true >/dev/null 2>&1 || return 2
> +       return 0
> +}
> +
> +wait_for_threads()
> +{
> +       start_time=$($tenths)
> +       while [ -e "/proc/$1/task" ] ; do
> +               th_cnt=$(find "/proc/$1/task" -mindepth 1 -maxdepth 1 -printf x | wc -c)
> +               if [ "${th_cnt}" -ge "$2" ] ; then
> +                       return 0
> +               fi
> +               # Wait at most 5 seconds
> +               if [ $(($($tenths) - start_time)) -ge 50 ] ; then

It's a shame that we would use this syntax but I couldn't find any better. :(


> +                       echo "PID $1 does not have $2 threads"
> +                       return 1
> +               fi
> +       done
> +       return 1
> +}
> +
> +wait_for_perf_to_start()
> +{
> +       echo "Waiting for \"perf record has started\" message"
> +       start_time=$($tenths)
> +       while [ -e "/proc/$1" ] ; do
> +               if grep -q "perf record has started" "${errfile}" ; then
> +                       echo OK
> +                       break
> +               fi
> +               # Wait at most 5 seconds
> +               if [ $(($($tenths) - start_time)) -ge 50 ] ; then
> +                       echo "perf recording did not start"
> +                       return 1
> +               fi
> +       done
> +       return 0
> +}
> +
> +wait_for_process_to_exit()
> +{
> +       start_time=$($tenths)
> +       while [ -e "/proc/$1" ] ; do
> +               # Wait at most 5 seconds
> +               if [ $(($($tenths) - start_time)) -ge 50 ] ; then
> +                       echo "PID $1 did not exit as expected"
> +                       return 1
> +               fi
> +       done
> +       return 0
> +}
> +
> +is_running()
> +{
> +       start_time=$($tenths)
> +       while [ -e "/proc/$1" ] ; do
> +               # Check for at least 0.3s
> +               if [ $(($($tenths) - start_time)) -gt 3 ] ; then
> +                       return 0
> +               fi
> +       done
> +       echo "PID $1 exited prematurely"
> +       return 1
> +}

Can we move this into the lib directory with an optional argument
specifying the timeout?

Thanks,
Namhyung


> +
> +test_per_thread()
> +{
> +       k="$1"
> +       desc="$2"
> +
> +       echo "--- Test per-thread ${desc}recording ---"
> +
> +       if ! $have_workload ; then
> +               echo "No workload, so skipping"
> +               return 2
> +       fi
> +
> +       if [ "${k}" = "k" ] ; then
> +               can_kernel || return 2
> +       fi
> +
> +       cat <<- "_end_of_file_" > "${awkscript}"
> +       BEGIN {
> +               s = "[ ]*"
> +               u = s"[0-9]+"s
> +               d = s"[0-9-]+"s
> +               x = s"[0-9a-fA-FxX]+"s
> +               mmapping = "idx"u": mmapping fd"u
> +               set_output = "idx"u": set output fd"u"->"u
> +               perf_event_open = "sys_perf_event_open: pid"d"cpu"d"group_fd"d"flags"x"="u
> +       }
> +
> +       /perf record opening and mmapping events/ {
> +               if (!done)
> +                       active = 1
> +       }
> +
> +       /perf record done opening and mmapping events/ {
> +               active = 0
> +               done = 1
> +       }
> +
> +       $0 ~ perf_event_open && active {
> +               match($0, perf_event_open)
> +               $0 = substr($0, RSTART, RLENGTH)
> +               pid = $3
> +               cpu = $5
> +               fd = $11
> +               print "pid " pid " cpu " cpu " fd " fd " : " $0
> +               fd_array[fd] = fd
> +               pid_array[fd] = pid
> +               cpu_array[fd] = cpu
> +       }
> +
> +       $0 ~ mmapping && active  {
> +               match($0, mmapping)
> +               $0 = substr($0, RSTART, RLENGTH)
> +               fd = $5
> +               print "fd " fd " : " $0
> +               if (fd in fd_array) {
> +                       mmap_array[fd] = 1
> +               } else {
> +                       print "Unknown fd " fd
> +                       exit 1
> +               }
> +       }
> +
> +       $0 ~ set_output && active {
> +               match($0, set_output)
> +               $0 = substr($0, RSTART, RLENGTH)
> +               fd = $6
> +               fd_to = $8
> +               print "fd " fd " fd_to " fd_to " : " $0
> +               if (fd in fd_array) {
> +                       if (fd_to in fd_array) {
> +                               set_output_array[fd] = fd_to
> +                       } else {
> +                               print "Unknown fd " fd_to
> +                               exit 1
> +                       }
> +               } else {
> +                       print "Unknown fd " fd
> +                       exit 1
> +               }
> +       }
> +
> +       END {
> +               print "Checking " length(fd_array) " fds"
> +               for (fd in fd_array) {
> +                       if (fd in mmap_array) {
> +                               pid = pid_array[fd]
> +                               if (pid != -1) {
> +                                       if (pid in pids) {
> +                                               print "More than 1 mmap for PID " pid
> +                                               exit 1
> +                                       }
> +                                       pids[pid] = 1
> +                               }
> +                               cpu = cpu_array[fd]
> +                               if (cpu != -1) {
> +                                       if (cpu in cpus) {
> +                                               print "More than 1 mmap for CPU " cpu
> +                                               exit 1
> +                                       }
> +                                       cpus[cpu] = 1
> +                               }
> +                       } else if (!(fd in set_output_array)) {
> +                               print "No mmap for fd " fd
> +                               exit 1
> +                       }
> +               }
> +               n = length(pids)
> +               if (n != thread_cnt) {
> +                       print "Expected " thread_cnt " per-thread mmaps - found " n
> +                       exit 1
> +               }
> +       }
> +       _end_of_file_
> +
> +       $workload &
> +       w1=$!
> +       $workload &
> +       w2=$!
> +       echo "Workload PIDs are $w1 and $w2"
> +       wait_for_threads ${w1} 2
> +       wait_for_threads ${w2} 2
> +
> +       perf record -B -N --no-bpf-event -o "${perfdatafile}" -e intel_pt//u"${k}" -vvv --per-thread -p "${w1},${w2}" 2>"${errfile}" >"${outfile}" &
> +       ppid=$!
> +       echo "perf PID is $ppid"
> +       wait_for_perf_to_start ${ppid} || return 1
> +
> +       kill ${w1}
> +       wait_for_process_to_exit ${w1} || return 1
> +       is_running ${ppid} || return 1
> +
> +       kill ${w2}
> +       wait_for_process_to_exit ${w2} || return 1
> +       wait_for_process_to_exit ${ppid} || return 1
> +
> +       awk -v thread_cnt=4 -f "${awkscript}" "${errfile}" || return 1
> +
> +       echo OK
> +       return 0
> +}
> +
>  count_result()
>  {
>         if [ "$1" -eq 2 ] ; then
> @@ -85,6 +330,8 @@ count_result()
>
>  ret=0
>  test_system_wide_side_band || ret=$? ; count_result $ret
> +test_per_thread "" "" || ret=$? ; count_result $ret
> +test_per_thread "k" "(incl. kernel) " || ret=$? ; count_result $ret
>
>  cleanup
>
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test
  2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
                   ` (10 preceding siblings ...)
  2022-09-12  8:34 ` [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
@ 2022-09-13 17:41 ` Namhyung Kim
  2022-09-14  8:25   ` Adrian Hunter
  11 siblings, 1 reply; 16+ messages in thread
From: Namhyung Kim @ 2022-09-13 17:41 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, linux-kernel,
	linux-perf-users

On Mon, Sep 12, 2022 at 1:34 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> Hi
>
> Here is a new per-thread test for the test_intel_pt.sh test script.
>
> The first 9 patches are tidy-ups for the script, mostly based on results
> from the shellcheck utility.
>
> The 10th patch adds debug prints that the script will capture to help
> verify correct operation.
>
> The final patch actually adds the new test.
>
>
> Adrian Hunter (11):
>       perf test: test_intel_pt.sh: Add cleanup function
>       perf test: test_intel_pt.sh: Use a temp directory
>       perf test: test_intel_pt.sh: Fix redirection
>       perf test: test_intel_pt.sh: Stop using expr
>       perf test: test_intel_pt.sh: Stop using backticks
>       perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l
>       perf test: test_intel_pt.sh: Use quotes around variable expansion
>       perf test: test_intel_pt.sh: Fix return checking
>       perf test: test_intel_pt.sh: Add more output in preparation for more tests
>       perf tools: Add debug messages and comments for testing
>       perf test: test_intel_pt.sh: Add per-thread test

I don't think I understood all the black magic in patch 11. :)
At least we can move some helper functions to the lib and
reuse them in other tests.  I'll test that later..

So for patch 01-10,

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


>
>  tools/lib/perf/evlist.c                 |   2 +
>  tools/perf/builtin-record.c             |   8 +
>  tools/perf/tests/shell/test_intel_pt.sh | 307 ++++++++++++++++++++++++++++++--
>  tools/perf/util/evsel.c                 |   2 +
>  4 files changed, 304 insertions(+), 15 deletions(-)
>
>
> Regards
> Adrian

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test
  2022-09-13 17:41 ` [PATCH 00/11] " Namhyung Kim
@ 2022-09-14  8:25   ` Adrian Hunter
  2022-09-26 19:35     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 16+ messages in thread
From: Adrian Hunter @ 2022-09-14  8:25 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ian Rogers, linux-kernel,
	linux-perf-users

On 13/09/22 20:41, Namhyung Kim wrote:
> On Mon, Sep 12, 2022 at 1:34 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> Hi
>>
>> Here is a new per-thread test for the test_intel_pt.sh test script.
>>
>> The first 9 patches are tidy-ups for the script, mostly based on results
>> from the shellcheck utility.
>>
>> The 10th patch adds debug prints that the script will capture to help
>> verify correct operation.
>>
>> The final patch actually adds the new test.
>>
>>
>> Adrian Hunter (11):
>>       perf test: test_intel_pt.sh: Add cleanup function
>>       perf test: test_intel_pt.sh: Use a temp directory
>>       perf test: test_intel_pt.sh: Fix redirection
>>       perf test: test_intel_pt.sh: Stop using expr
>>       perf test: test_intel_pt.sh: Stop using backticks
>>       perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l
>>       perf test: test_intel_pt.sh: Use quotes around variable expansion
>>       perf test: test_intel_pt.sh: Fix return checking
>>       perf test: test_intel_pt.sh: Add more output in preparation for more tests
>>       perf tools: Add debug messages and comments for testing
>>       perf test: test_intel_pt.sh: Add per-thread test
> 
> I don't think I understood all the black magic in patch 11. :)

It is not that bad :-)

Consider the output from the test:


	$ perf test -v " intel pt"
	Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
	102: Miscellaneous Intel PT testing                                  :
	--- start ---
	test child forked, pid 155646
<SNIP>
	--- Test per-thread recording ---
	Workload PIDs are 155669 and 155670
	perf PID is 155681
	Waiting for "perf record has started" message
	OK
	pid 155669 cpu -1 fd 5 : sys_perf_event_open: pid 155669  cpu -1 group_fd -1  flags 0x8 = 5

awk has matched the debug message and determined the values for pid, cpu and fd

	pid 155673 cpu -1 fd 6 : sys_perf_event_open: pid 155673  cpu -1 group_fd -1  flags 0x8 = 6
	pid 155670 cpu -1 fd 7 : sys_perf_event_open: pid 155670  cpu -1 group_fd -1  flags 0x8 = 7
	pid 155672 cpu -1 fd 8 : sys_perf_event_open: pid 155672  cpu -1 group_fd -1  flags 0x8 = 8
	pid 155669 cpu -1 fd 9 : sys_perf_event_open: pid 155669  cpu -1 group_fd -1  flags 0x8 = 9
	pid 155673 cpu -1 fd 10 : sys_perf_event_open: pid 155673  cpu -1 group_fd -1  flags 0x8 = 10
	pid 155670 cpu -1 fd 11 : sys_perf_event_open: pid 155670  cpu -1 group_fd -1  flags 0x8 = 11
	pid 155672 cpu -1 fd 12 : sys_perf_event_open: pid 155672  cpu -1 group_fd -1  flags 0x8 = 12
	fd 5 : idx 0: mmapping fd 5
	
awk has matched the debug message and determined the values for fd
	
	fd 9 fd_to 5 : idx 0: set output fd 9 -> 5
	
awk has matched the debug message and determined the values for fd and fd_to

	fd 6 : idx 1: mmapping fd 6
	fd 10 fd_to 6 : idx 1: set output fd 10 -> 6
	fd 7 : idx 2: mmapping fd 7
	fd 11 fd_to 7 : idx 2: set output fd 11 -> 7
	fd 8 : idx 3: mmapping fd 8
	fd 12 fd_to 8 : idx 3: set output fd 12 -> 8
	Checking 8 fds

Now awk is checking:
- every fd is mmapped or set-output
- there is 1 mmap per unique pid
- there is 1 mmap per unique cpu
- there are the right number of different pids
i.e.
	END {
		print "Checking " length(fd_array) " fds"
		for (fd in fd_array) {
			if (fd in mmap_array) {
				pid = pid_array[fd]
				if (pid != -1) {
					if (pid in pids) {
						print "More than 1 mmap for PID " pid
						exit 1
					}
					pids[pid] = 1
				}
				cpu = cpu_array[fd]
				if (cpu != -1) {
					if (cpu in cpus) {
						print "More than 1 mmap for CPU " cpu
						exit 1
					}
					cpus[cpu] = 1
				}
			} else if (!(fd in set_output_array)) {
				print "No mmap for fd " fd
				exit 1
			}
		}
		n = length(pids)
		if (n != thread_cnt) {
			print "Expected " thread_cnt " per-thread mmaps - found " n
			exit 1
		}
	}

> At least we can move some helper functions to the lib and
> reuse them in other tests.  I'll test that later..
> 
> So for patch 01-10,
> 
> Acked-by: Namhyung Kim <namhyung@kernel.org>
> 
> Thanks,
> Namhyung
> 
> 
>>
>>  tools/lib/perf/evlist.c                 |   2 +
>>  tools/perf/builtin-record.c             |   8 +
>>  tools/perf/tests/shell/test_intel_pt.sh | 307 ++++++++++++++++++++++++++++++--
>>  tools/perf/util/evsel.c                 |   2 +
>>  4 files changed, 304 insertions(+), 15 deletions(-)
>>
>>
>> Regards
>> Adrian


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test
  2022-09-14  8:25   ` Adrian Hunter
@ 2022-09-26 19:35     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 16+ messages in thread
From: Arnaldo Carvalho de Melo @ 2022-09-26 19:35 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Namhyung Kim, Jiri Olsa, Ian Rogers, linux-kernel,
	linux-perf-users

Em Wed, Sep 14, 2022 at 11:25:07AM +0300, Adrian Hunter escreveu:
> On 13/09/22 20:41, Namhyung Kim wrote:
> > On Mon, Sep 12, 2022 at 1:34 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
> >>
> >> Hi
> >>
> >> Here is a new per-thread test for the test_intel_pt.sh test script.
> >>
> >> The first 9 patches are tidy-ups for the script, mostly based on results
> >> from the shellcheck utility.
> >>
> >> The 10th patch adds debug prints that the script will capture to help
> >> verify correct operation.
> >>
> >> The final patch actually adds the new test.
> >>
> >>
> >> Adrian Hunter (11):
> >>       perf test: test_intel_pt.sh: Add cleanup function
> >>       perf test: test_intel_pt.sh: Use a temp directory
> >>       perf test: test_intel_pt.sh: Fix redirection
> >>       perf test: test_intel_pt.sh: Stop using expr
> >>       perf test: test_intel_pt.sh: Stop using backticks
> >>       perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l
> >>       perf test: test_intel_pt.sh: Use quotes around variable expansion
> >>       perf test: test_intel_pt.sh: Fix return checking
> >>       perf test: test_intel_pt.sh: Add more output in preparation for more tests
> >>       perf tools: Add debug messages and comments for testing
> >>       perf test: test_intel_pt.sh: Add per-thread test
> > 
> > I don't think I understood all the black magic in patch 11. :)
> 
> It is not that bad :-)

:-)

Thanks, applied the series.

- Arnaldo
 
> Consider the output from the test:
> 
> 
> 	$ perf test -v " intel pt"
> 	Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> 	102: Miscellaneous Intel PT testing                                  :
> 	--- start ---
> 	test child forked, pid 155646
> <SNIP>
> 	--- Test per-thread recording ---
> 	Workload PIDs are 155669 and 155670
> 	perf PID is 155681
> 	Waiting for "perf record has started" message
> 	OK
> 	pid 155669 cpu -1 fd 5 : sys_perf_event_open: pid 155669  cpu -1 group_fd -1  flags 0x8 = 5
> 
> awk has matched the debug message and determined the values for pid, cpu and fd
> 
> 	pid 155673 cpu -1 fd 6 : sys_perf_event_open: pid 155673  cpu -1 group_fd -1  flags 0x8 = 6
> 	pid 155670 cpu -1 fd 7 : sys_perf_event_open: pid 155670  cpu -1 group_fd -1  flags 0x8 = 7
> 	pid 155672 cpu -1 fd 8 : sys_perf_event_open: pid 155672  cpu -1 group_fd -1  flags 0x8 = 8
> 	pid 155669 cpu -1 fd 9 : sys_perf_event_open: pid 155669  cpu -1 group_fd -1  flags 0x8 = 9
> 	pid 155673 cpu -1 fd 10 : sys_perf_event_open: pid 155673  cpu -1 group_fd -1  flags 0x8 = 10
> 	pid 155670 cpu -1 fd 11 : sys_perf_event_open: pid 155670  cpu -1 group_fd -1  flags 0x8 = 11
> 	pid 155672 cpu -1 fd 12 : sys_perf_event_open: pid 155672  cpu -1 group_fd -1  flags 0x8 = 12
> 	fd 5 : idx 0: mmapping fd 5
> 	
> awk has matched the debug message and determined the values for fd
> 	
> 	fd 9 fd_to 5 : idx 0: set output fd 9 -> 5
> 	
> awk has matched the debug message and determined the values for fd and fd_to
> 
> 	fd 6 : idx 1: mmapping fd 6
> 	fd 10 fd_to 6 : idx 1: set output fd 10 -> 6
> 	fd 7 : idx 2: mmapping fd 7
> 	fd 11 fd_to 7 : idx 2: set output fd 11 -> 7
> 	fd 8 : idx 3: mmapping fd 8
> 	fd 12 fd_to 8 : idx 3: set output fd 12 -> 8
> 	Checking 8 fds
> 
> Now awk is checking:
> - every fd is mmapped or set-output
> - there is 1 mmap per unique pid
> - there is 1 mmap per unique cpu
> - there are the right number of different pids
> i.e.
> 	END {
> 		print "Checking " length(fd_array) " fds"
> 		for (fd in fd_array) {
> 			if (fd in mmap_array) {
> 				pid = pid_array[fd]
> 				if (pid != -1) {
> 					if (pid in pids) {
> 						print "More than 1 mmap for PID " pid
> 						exit 1
> 					}
> 					pids[pid] = 1
> 				}
> 				cpu = cpu_array[fd]
> 				if (cpu != -1) {
> 					if (cpu in cpus) {
> 						print "More than 1 mmap for CPU " cpu
> 						exit 1
> 					}
> 					cpus[cpu] = 1
> 				}
> 			} else if (!(fd in set_output_array)) {
> 				print "No mmap for fd " fd
> 				exit 1
> 			}
> 		}
> 		n = length(pids)
> 		if (n != thread_cnt) {
> 			print "Expected " thread_cnt " per-thread mmaps - found " n
> 			exit 1
> 		}
> 	}
> 
> > At least we can move some helper functions to the lib and
> > reuse them in other tests.  I'll test that later..
> > 
> > So for patch 01-10,
> > 
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> > 
> > Thanks,
> > Namhyung
> > 
> > 
> >>
> >>  tools/lib/perf/evlist.c                 |   2 +
> >>  tools/perf/builtin-record.c             |   8 +
> >>  tools/perf/tests/shell/test_intel_pt.sh | 307 ++++++++++++++++++++++++++++++--
> >>  tools/perf/util/evsel.c                 |   2 +
> >>  4 files changed, 304 insertions(+), 15 deletions(-)
> >>
> >>
> >> Regards
> >> Adrian

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-09-26 19:35 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-12  8:34 [PATCH 00/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
2022-09-12  8:34 ` [PATCH 01/11] perf test: test_intel_pt.sh: Add cleanup function Adrian Hunter
2022-09-12  8:34 ` [PATCH 02/11] perf test: test_intel_pt.sh: Use a temp directory Adrian Hunter
2022-09-12  8:34 ` [PATCH 03/11] perf test: test_intel_pt.sh: Fix redirection Adrian Hunter
2022-09-12  8:34 ` [PATCH 04/11] perf test: test_intel_pt.sh: Stop using expr Adrian Hunter
2022-09-12  8:34 ` [PATCH 05/11] perf test: test_intel_pt.sh: Stop using backticks Adrian Hunter
2022-09-12  8:34 ` [PATCH 06/11] perf test: test_intel_pt.sh: Use grep -c instead of grep plus wc -l Adrian Hunter
2022-09-12  8:34 ` [PATCH 07/11] perf test: test_intel_pt.sh: Use quotes around variable expansion Adrian Hunter
2022-09-12  8:34 ` [PATCH 08/11] perf test: test_intel_pt.sh: Fix return checking Adrian Hunter
2022-09-12  8:34 ` [PATCH 09/11] perf test: test_intel_pt.sh: Add more output in preparation for more tests Adrian Hunter
2022-09-12  8:34 ` [PATCH 10/11] perf tools: Add debug messages and comments for testing Adrian Hunter
2022-09-12  8:34 ` [PATCH 11/11] perf test: test_intel_pt.sh: Add per-thread test Adrian Hunter
2022-09-13 17:37   ` Namhyung Kim
2022-09-13 17:41 ` [PATCH 00/11] " Namhyung Kim
2022-09-14  8:25   ` Adrian Hunter
2022-09-26 19:35     ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).