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

Thomas Richter posted 1 patch 7 months, 4 weeks ago
tools/perf/tests/shell/lib/stat_output.sh  | 5 +++++
tools/perf/tests/shell/stat+json_output.sh | 5 +++++
2 files changed, 10 insertions(+)
[PATCH] perf/tests: Fix tests 84 and 86 Add --metric-only on s390
Posted by Thomas Richter 7 months, 4 weeks ago
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
Posted by Arnaldo Carvalho de Melo 7 months, 4 weeks ago
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