Re: [PATCH] perf/test: perf test 83 fails on s390

From: Ian Rogers
Date: Fri Jun 03 2022 - 17:00:20 EST

Next message: Andrii Nakryiko: "Re: [PATCH v2 0/9] bpf: Per-operation map permissions"
Previous message: Andrii Nakryiko: "Re: [PATCH v2 9/9] selftests/bpf: Add map access tests"
In reply to: Thomas Richter: "[PATCH] perf/test: perf test 83 fails on s390"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, Jun 3, 2022 at 4:33 AM Thomas Richter <tmricht@xxxxxxxxxxxxx> wrote:
>
> perf test -F 83 ("perf stat CSV output linter") fails on s390.
> Reason is the wrong number of fields for certain CPU core/die/socket
> related output.
>
> On x84_64 the output of command
> # ./perf stat -x, -A -a --no-merge true
> CPU0,1.50,msec,cpu-clock,1502781,100.00,1.052,CPUs utilized
> CPU1,1.48,msec,cpu-clock,1476113,100.00,1.034,CPUs utilized
> ...
> results in 8 fields with 7 comma separators.
>
> On s390 the output of command
> # ./perf stat -x, -A -a --no-merge -- true
> 0.95,msec,cpu-clock,949800,100.00,1.060,CPUs utilized
> ...
> results in 7 fields with 6 comma separators. Therefore this tests
> fails on s390. Similar issues exist for per-die and per-socket output
> which is not supported on s390.
>
> I have rewritten the python program to count commas in each output line
> into a bash function to achieve the same result. I hope this makes
> it a bit easier.
>
> Output before:
> # ./perf test -F 83
> 83: perf stat CSV output linter :
> Checking CSV output: no args [Success]
> Checking CSV output: system wide [Success]
> Checking CSV output: system wide Checking CSV output: \
> system wide no aggregation 6.92,msec,cpu-clock,\
> 6918131,100.00,6.972,CPUs utilized
> ...
> RuntimeError: wrong number of fields. expected 7 in \
> 6.92,msec,cpu-clock,6918131,100.00,6.972,CPUs utilized
>
> FAILED!
> #
>
> Output after:
> # ./perf test -F 83
> 83: perf stat CSV output linter :
> Checking CSV output: no args [Success]
> Checking CSV output: system wide [Success]
> Checking CSV output: system wide Checking CSV output:\
> system wide no aggregation [Success]
> Checking CSV output: interval [Success]
> Checking CSV output: event [Success]
> Checking CSV output: per core [Success]
> Checking CSV output: per thread [Success]
> Checking CSV output: per die [Success]
> Checking CSV output: per node [Success]
> Checking CSV output: per socket [Success]
> Ok
> #
>
> Signed-off-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>

Acked-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
> .../tests/shell/lib/perf_csv_output_lint.py | 48 -------------
> tools/perf/tests/shell/stat+csv_output.sh | 69 ++++++++++++-------
> 2 files changed, 45 insertions(+), 72 deletions(-)
> delete mode 100644 tools/perf/tests/shell/lib/perf_csv_output_lint.py
>
> diff --git a/tools/perf/tests/shell/lib/perf_csv_output_lint.py b/tools/perf/tests/shell/lib/perf_csv_output_lint.py
> deleted file mode 100644
> index 714f283cfb1b..000000000000
> --- a/tools/perf/tests/shell/lib/perf_csv_output_lint.py
> +++ /dev/null
> @@ -1,48 +0,0 @@
> -#!/usr/bin/python
> -# SPDX-License-Identifier: GPL-2.0
> -
> -import argparse
> -import sys
> -
> -# Basic sanity check of perf CSV output as specified in the man page.
> -# Currently just checks the number of fields per line in output.
> -
> -ap = argparse.ArgumentParser()
> -ap.add_argument('--no-args', action='store_true')
> -ap.add_argument('--interval', action='store_true')
> -ap.add_argument('--system-wide-no-aggr', action='store_true')
> -ap.add_argument('--system-wide', action='store_true')
> -ap.add_argument('--event', action='store_true')
> -ap.add_argument('--per-core', action='store_true')
> -ap.add_argument('--per-thread', action='store_true')
> -ap.add_argument('--per-die', action='store_true')
> -ap.add_argument('--per-node', action='store_true')
> -ap.add_argument('--per-socket', action='store_true')
> -ap.add_argument('--separator', default=',', nargs='?')
> -args = ap.parse_args()
> -
> -Lines = sys.stdin.readlines()
> -
> -def check_csv_output(exp):
> - for line in Lines:
> - if 'failed' not in line:
> - count = line.count(args.separator)
> - if count != exp:
> - sys.stdout.write(''.join(Lines))
> - raise RuntimeError(f'wrong number of fields. expected {exp} in {line}')
> -
> -try:
> - if args.no_args or args.system_wide or args.event:
> - expected_items = 6
> - elif args.interval or args.per_thread or args.system_wide_no_aggr:
> - expected_items = 7
> - elif args.per_core or args.per_socket or args.per_node or args.per_die:
> - expected_items = 8
> - else:
> - ap.print_help()
> - raise RuntimeError('No checking option specified')
> - check_csv_output(expected_items)
> -
> -except:
> - sys.stdout.write('Test failed for input: ' + ''.join(Lines))
> - raise
> diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> index 983220ef3cb4..38c26f3ef4c1 100755
> --- a/tools/perf/tests/shell/stat+csv_output.sh
> +++ b/tools/perf/tests/shell/stat+csv_output.sh
> @@ -6,20 +6,41 @@
>
> set -e
>
> -pythonchecker=$(dirname $0)/lib/perf_csv_output_lint.py
> -if [ "x$PYTHON" == "x" ]
> -then
> - if which python3 > /dev/null
> - then
> - PYTHON=python3
> - elif which python > /dev/null
> - then
> - PYTHON=python
> - else
> - echo Skipping test, python not detected please set environment variable PYTHON.
> - exit 2
> - fi
> -fi
> +function commachecker()
> +{
> + local -i cnt=0 exp=0
> +
> + case "$1"
> + in "--no-args") exp=6
> + ;; "--system-wide") exp=6
> + ;; "--event") exp=6
> + ;; "--interval") exp=7
> + ;; "--per-thread") exp=7
> + ;; "--system-wide-no-aggr") exp=7
> + [ $(uname -m) = "s390x" ] && exp=6
> + ;; "--per-core") exp=8
> + ;; "--per-socket") exp=8
> + ;; "--per-node") exp=8
> + ;; "--per-die") exp=8
> + esac
> +
> + while read line
> + do
> + # Check for lines beginning with Failed
> + x=${line:0:6}
> + [ "$x" = "Failed" ] && continue
> +
> + # Count the number of commas
> + x=$(echo $line | tr -d -c ',')
> + cnt="${#x}"
> + # echo $line $cnt
> + [ "$cnt" -ne "$exp" ] && {
> + echo "wrong number of fields. expected $exp in $line" 1>&2
> + exit 1;
> + }
> + done
> + return 0
> +}
>
> # Return true if perf_event_paranoid is > $1 and not running as root.
> function ParanoidAndNotRoot()
> @@ -30,7 +51,7 @@ function ParanoidAndNotRoot()
> check_no_args()
> {
> echo -n "Checking CSV output: no args "
> - perf stat -x, true 2>&1 | $PYTHON $pythonchecker --no-args
> + perf stat -x, true 2>&1 | commachecker --no-args
> echo "[Success]"
> }
>
> @@ -42,7 +63,7 @@ check_system_wide()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, -a true 2>&1 | $PYTHON $pythonchecker --system-wide
> + perf stat -x, -a true 2>&1 | commachecker --system-wide
> echo "[Success]"
> }
>
> @@ -55,14 +76,14 @@ check_system_wide_no_aggr()
> return
> fi
> echo -n "Checking CSV output: system wide no aggregation "
> - perf stat -x, -A -a --no-merge true 2>&1 | $PYTHON $pythonchecker --system-wide-no-aggr
> + perf stat -x, -A -a --no-merge true 2>&1 | commachecker --system-wide-no-aggr
> echo "[Success]"
> }
>
> check_interval()
> {
> echo -n "Checking CSV output: interval "
> - perf stat -x, -I 1000 true 2>&1 | $PYTHON $pythonchecker --interval
> + perf stat -x, -I 1000 true 2>&1 | commachecker --interval
> echo "[Success]"
> }
>
> @@ -70,7 +91,7 @@ check_interval()
> check_event()
> {
> echo -n "Checking CSV output: event "
> - perf stat -x, -e cpu-clock true 2>&1 | $PYTHON $pythonchecker --event
> + perf stat -x, -e cpu-clock true 2>&1 | commachecker --event
> echo "[Success]"
> }
>
> @@ -82,7 +103,7 @@ check_per_core()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, --per-core -a true 2>&1 | $PYTHON $pythonchecker --per-core
> + perf stat -x, --per-core -a true 2>&1 | commachecker --per-core
> echo "[Success]"
> }
>
> @@ -94,7 +115,7 @@ check_per_thread()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, --per-thread -a true 2>&1 | $PYTHON $pythonchecker --per-thread
> + perf stat -x, --per-thread -a true 2>&1 | commachecker --per-thread
> echo "[Success]"
> }
>
> @@ -106,7 +127,7 @@ check_per_die()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, --per-die -a true 2>&1 | $PYTHON $pythonchecker --per-die
> + perf stat -x, --per-die -a true 2>&1 | commachecker --per-die
> echo "[Success]"
> }
>
> @@ -118,7 +139,7 @@ check_per_node()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, --per-node -a true 2>&1 | $PYTHON $pythonchecker --per-node
> + perf stat -x, --per-node -a true 2>&1 | commachecker --per-node
> echo "[Success]"
> }
>
> @@ -130,7 +151,7 @@ check_per_socket()
> echo "[Skip] paranoid and not root"
> return
> fi
> - perf stat -x, --per-socket -a true 2>&1 | $PYTHON $pythonchecker --per-socket
> + perf stat -x, --per-socket -a true 2>&1 | commachecker --per-socket
> echo "[Success]"
> }
>
> --
> 2.36.1
>

Next message: Andrii Nakryiko: "Re: [PATCH v2 0/9] bpf: Per-operation map permissions"
Previous message: Andrii Nakryiko: "Re: [PATCH v2 9/9] selftests/bpf: Add map access tests"
In reply to: Thomas Richter: "[PATCH] perf/test: perf test 83 fails on s390"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]