[PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[Thomas Richter]

On s390x z/VM machines the CPU Measurement Facility is not available.
Events cycles and instructions do not exist.
Running above tests on s390 z/VM always fails with this error:

 # ./perf test 84 86
 84: perf stat JSON output linter          : FAILED!
 86: perf stat STD output linter           : FAILED!
 #

Root cause is command
 # perf stat -j --metric-only -e instructions,cycles -- true
 {"metric-value" : "none"}
 #
which fails due to unsupported events and returns "none".
Do not execute this test case on s390 z/VM machines.

Output after:
 # ./perf test 84 86
 84: perf stat JSON output linter          : Ok
 86: perf stat STD output linter           : Ok
 #

Fixes: 45a86d017adf ("perf test: Add --metric-only to perf stat output tests")
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Reviewed-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lib/stat_output.sh  | 5 +++++
 tools/perf/tests/shell/stat+json_output.sh | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh
index 4d4aac547f01..a708dedf7d9d 100644
--- a/tools/perf/tests/shell/lib/stat_output.sh
+++ b/tools/perf/tests/shell/lib/stat_output.sh
@@ -151,6 +151,11 @@ check_per_socket()
 check_metric_only()
 {
 	echo -n "Checking $1 output: metric only "
+	if [ "$(uname -m)" = "s390x" ] && grep -q z/VM /proc/sysinfo
+	then
+		echo "[Skip] not supported on z/VM"
+		return
+	fi
 	perf stat --metric-only $2 -e instructions,cycles true
 	commachecker --metric-only
 	echo "[Success]"
diff --git a/tools/perf/tests/shell/stat+json_output.sh b/tools/perf/tests/shell/stat+json_output.sh
index a4f257ea839e..d78e06636a3a 100755
--- a/tools/perf/tests/shell/stat+json_output.sh
+++ b/tools/perf/tests/shell/stat+json_output.sh
@@ -176,6 +176,11 @@ check_per_socket()
 check_metric_only()
 {
 	echo -n "Checking json output: metric only "
+	if [ "$(uname -m)" = "s390x" ] && grep -q z/VM /proc/sysinfo
+	then
+		echo "[Skip] not supported on z/VM"
+		return
+	fi
 	perf stat -j --metric-only -e instructions,cycles -o "${stat_output}" true
 	$PYTHON $pythonchecker --metric-only --file "${stat_output}"
 	echo "[Success]"
-- 
2.49.0

Re: [PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[Heiko Carstens]

On Tue, Apr 15, 2025 at 03:45:53PM +0200, Thomas Richter wrote:
> On s390x z/VM machines the CPU Measurement Facility is not available.
> Events cycles and instructions do not exist.
> Running above tests on s390 z/VM always fails with this error:
> 
>  # ./perf test 84 86
>  84: perf stat JSON output linter          : FAILED!
>  86: perf stat STD output linter           : FAILED!
>  #

I would guess this fails also for KVM guests?

> diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh
> index 4d4aac547f01..a708dedf7d9d 100644
> --- a/tools/perf/tests/shell/lib/stat_output.sh
> +++ b/tools/perf/tests/shell/lib/stat_output.sh
> @@ -151,6 +151,11 @@ check_per_socket()
>  check_metric_only()
>  {
>  	echo -n "Checking $1 output: metric only "
> +	if [ "$(uname -m)" = "s390x" ] && grep -q z/VM /proc/sysinfo
> +	then
> +		echo "[Skip] not supported on z/VM"
> +		return
> +	fi

Wouldn't it be better to test for the availability of the CPU-measurement
counter facility? That is: test if facility number 67 is present in the
facilities field of /proc/cpuinfo.

Re: [PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[Thomas Richter]

On 4/17/25 13:52, Heiko Carstens wrote:
> On Tue, Apr 15, 2025 at 03:45:53PM +0200, Thomas Richter wrote:
>> On s390x z/VM machines the CPU Measurement Facility is not available.
>> Events cycles and instructions do not exist.
>> Running above tests on s390 z/VM always fails with this error:
>>
>>  # ./perf test 84 86
>>  84: perf stat JSON output linter          : FAILED!
>>  86: perf stat STD output linter           : FAILED!
>>  #
> 
> I would guess this fails also for KVM guests?

Yes
> 
>> diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh
>> index 4d4aac547f01..a708dedf7d9d 100644
>> --- a/tools/perf/tests/shell/lib/stat_output.sh
>> +++ b/tools/perf/tests/shell/lib/stat_output.sh
>> @@ -151,6 +151,11 @@ check_per_socket()
>>  check_metric_only()
>>  {
>>  	echo -n "Checking $1 output: metric only "
>> +	if [ "$(uname -m)" = "s390x" ] && grep -q z/VM /proc/sysinfo
>> +	then
>> +		echo "[Skip] not supported on z/VM"
>> +		return
>> +	fi
> 
> Wouldn't it be better to test for the availability of the CPU-measurement
> counter facility? That is: test if facility number 67 is present in the
> facilities field of /proc/cpuinfo.
> 

I can certains change the check and grep for facilities number 67.
Will send v2.

-- 
Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany
--
IBM Deutschland Research & Development GmbH

Vorsitzender des Aufsichtsrats: Wolfgang Wendt

Geschäftsführung: David Faller

Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294

[PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[Thomas Richter]

On s390x KVM and z/VM machines the CPU Measurement Facility is
not available. Events cycles and instructions do not exist.
Running above tests on s390 KVM and z/VM guests always fail
with this error:

 # ./perf test 84 86
 84: perf stat JSON output linter          : FAILED!
 86: perf stat STD output linter           : FAILED!
 #

Root cause is command
 # perf stat -j --metric-only -e instructions,cycles -- true
 {"metric-value" : "none"}
 #
which fails due to unsupported events and returns "none".
Do not execute this test case on s390 KVM and z/VM machines.

Output after:
 # ./perf test 84 86
 84: perf stat JSON output linter          : Ok
 86: perf stat STD output linter           : Ok
 #

Fixes: 45a86d017adf ("perf test: Add --metric-only to perf stat output tests")
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Suggested-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
Suggested-by: Heiko Carstens <hca@linux.ibm.com>
Reviewed-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lib/stat_output.sh  | 5 +++++
 tools/perf/tests/shell/stat+json_output.sh | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh
index 4d4aac547f01..c2ec7881ec1d 100644
--- a/tools/perf/tests/shell/lib/stat_output.sh
+++ b/tools/perf/tests/shell/lib/stat_output.sh
@@ -151,6 +151,11 @@ check_per_socket()
 check_metric_only()
 {
 	echo -n "Checking $1 output: metric only "
+	if [ "$(uname -m)" = "s390x" ] && ! grep '^facilities' /proc/cpuinfo  | grep -qw 67
+	then
+		echo "[Skip] CPU-measurement counter facility not installed"
+		return
+	fi
 	perf stat --metric-only $2 -e instructions,cycles true
 	commachecker --metric-only
 	echo "[Success]"
diff --git a/tools/perf/tests/shell/stat+json_output.sh b/tools/perf/tests/shell/stat+json_output.sh
index a4f257ea839e..98fb65274ac4 100755
--- a/tools/perf/tests/shell/stat+json_output.sh
+++ b/tools/perf/tests/shell/stat+json_output.sh
@@ -176,6 +176,11 @@ check_per_socket()
 check_metric_only()
 {
 	echo -n "Checking json output: metric only "
+	if [ "$(uname -m)" = "s390x" ] && ! grep '^facilities' /proc/cpuinfo  | grep -qw 67
+	then
+		echo "[Skip] CPU-measurement counter facility not installed"
+		return
+	fi
 	perf stat -j --metric-only -e instructions,cycles -o "${stat_output}" true
 	$PYTHON $pythonchecker --metric-only --file "${stat_output}"
 	echo "[Success]"
-- 
2.49.0

Re: [PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390

[Arnaldo Carvalho de Melo]

On Thu, Apr 24, 2025 at 03:33:10PM +0200, Thomas Richter wrote:
> Fixes: 45a86d017adf ("perf test: Add --metric-only to perf stat output tests")
> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
> Suggested-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
> Suggested-by: Heiko Carstens <hca@linux.ibm.com>
> Reviewed-by: Sumanth Korikkar <sumanthk@linux.ibm.com>
> Cc: Namhyung Kim <namhyung@kernel.org>

Thanks, applied to perf-tools-next.

But please try to look at how patches are applied, specifically in how
commit log messages are rewritten, what is modified in the commit log
messages.

Specifically: When we do a 'git log --oneline' what we see should help
we find whatever we're trying to find. Twitter (not-X) style.

While I agree with Namhyung that whatever reduces the work a maintainer
should (have to) care about, and doing this is just some muscle memory
from sending patches to Ingo, I do think that trying to be consistent on
how we describe the problem, how the solution being proposed fixes the
problem, and then, when that is read, and the code is read, all matches,
bingo, patch accepted, tests pass, lets focus on the next issue.

This is not something aimed at you, but its something that takes time
when I'm processing patches, maybe I should just ignore this if the code
is good enough (I do this more than I want), but I think getting this
out of my mind is important.

Lowering the bar invites more people to contribute, but then it bites us
later.

- Arnaldo