| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <YQ2AzS3Az9Zfnu4I@kernel.org>
Date: Fri, 6 Aug 2021 15:34:53 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Claire Jensen <cjense@...gle.com>
Cc: linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
peterz@...radead.org, mingo@...hat.com, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...hat.com,
namhyung@...nel.org, irogers@...gle.com, eranian@...gle.com
Subject: Re: [PATCH 1/1] perf test: Add test for CSV output.
Em Thu, Aug 05, 2021 at 05:14:31PM +0000, Claire Jensen escreveu:
> Add field checking tests for perf stat CSV output. Counts expected
> fields for various commands. No testing added for summary mode
> since it is broken.
>
> An example of the summary output is:
>
> summary,263831,,instructions:u,1435072,100.0,0.46,insn per cycle
> ,,,,,1.37,stalled cycles per insn
>
> This should be:
>
> summary,263831,,instructions:u,1435072,100.0,0.46,insn per cycle
> summary,,,,,,1.37,stalled cycles per insn
>
> The output has 7 fields when it should have 8. Additionally, the newline
> spacing is wrong, so it was excluded from testing until a fix is made.
You forgot to add your Signed-off-by: line as described in
Documentation/process/submitting-patches.rst.
Thanks for working on this, the more 'perf test' entries, the better :-)
Some more comments below.
- Arnaldo
> ---
> .../tests/shell/lib/perf_csv_output_lint.py | 55 ++++++++
> tools/perf/tests/shell/stat+csv_output.sh | 117 ++++++++++++++++++
> 2 files changed, 172 insertions(+)
> create mode 100644 tools/perf/tests/shell/lib/perf_csv_output_lint.py
> create mode 100644 tools/perf/tests/shell/stat+csv_output.sh
>
> diff --git a/tools/perf/tests/shell/lib/perf_csv_output_lint.py b/tools/perf/tests/shell/lib/perf_csv_output_lint.py
> new file mode 100644
> index 000000000000..ef8a32c3523c
> --- /dev/null
> +++ b/tools/perf/tests/shell/lib/perf_csv_output_lint.py
> @@ -0,0 +1,55 @@
> +#!/usr/bin/python
> +# SPDX-License-Identifier: GPL-2.0
> +
> +from __future__ import print_function
> +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('--all-cpus-no-aggr', action='store_true')
> +ap.add_argument('--all-cpus', 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', const=',', nargs='?')
> +args = ap.parse_args()
> +
> +Lines = sys.stdin.readlines()
> +ch = args.separator
> +
> +
> +def check_csv_output(exp):
> + for line in Lines:
> + if 'failed' not in line:
> + count = 0
> + count = line.count(args.separator)
> + if count != exp:
> + sys.stdout.write(''.join(Lines))
> + raise RuntimeError('wrong number of fields.'
> + ' expected {0} in {1}\n'.format(exp, line))
> +
> +try:
> + if args.no_args or args.all_cpus or args.event:
> + check_csv_output(6)
> +
> + if args.interval or args.per_thread:
> + check_csv_output(7)
> +
> + if args.per_core or args.per_socket or args.per_node or args.per_die:
> + check_csv_output(8)
> +
> +except:
> + sys.stdout.write('Test failed for input: ' + ''.join(Lines))
> + raise
> +
> +
> +
> +
Please avoid adding empty lines at the end of a file.
> diff --git a/tools/perf/tests/shell/stat+csv_output.sh b/tools/perf/tests/shell/stat+csv_output.sh
> new file mode 100644
> index 000000000000..282f9547e7be
> --- /dev/null
> +++ b/tools/perf/tests/shell/stat+csv_output.sh
> @@ -0,0 +1,117 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# perf stat csv output test
> +# Tests various perf stat CSV output commands for the
> +# correct number of fields and the CSV separator set to ','.
> +
> +set -e
> +
> +pythonchecker=$(dirname $0)/lib/perf_csv_output_lint.py
> +file="/proc/sys/kernel/perf_event_paranoid"
> +paranoia=$(cat "$file" | grep -o -E '[0-9]+')
> +echo $paranoia
> +
> +check_no_args()
> +{
> + perf stat -x, sleep 1 2>&1 | \
> + python $pythonchecker --no-args --separator
> +}
> +
> +if [ $paranoia -gt 0 ];
> +then
Please have the 'then' line at the end of the if one, following the
style of pre-existing shell tests such as
tools/perf/tests/shell/pipe_test.sh.
> + echo check_all_cpus test skipped because of paranoia level.
> +else
> + check_all_cpus()
> + {
> + perf stat -x, -a 2>&1 sleep 1 | \
> + python $pythonchecker --all-cpus --separator
> + }
> + check_all_cpus
> +fi
> +
> +check_interval()
> +{
> + perf stat -x, -I 1000 2>&1 sleep 1 | \
> + python $pythonchecker --interval --separator
> +}
> +
> +check_all_cpus_no_aggr()
> +{
> + perf stat -x, -A -a --no-merge 2>&1 sleep 1 | \
> + python $pythonchecker --all-cpus-no-aggr --separator
> +}
> +
> +check_event()
> +{
> + perf stat -x, -e cpu-clock 2>&1 sleep 1 | \
> + python $pythonchecker --event --separator
> +}
> +
> +if [ $paranoia -gt 0 ];
> +then
> + echo check_all_cpus test skipped because of paranoia level.
> +else
> + check_per_core()
> + {
> + perf stat -x, --per-core -a 2>&1 sleep 1 | \
> + python $pythonchecker --per-core --separator
> + }
> + check_per_core
> +fi
> +
> +if [ $paranoia -gt 0 ];
> +then
> + echo check_all_cpus test skipped because of paranoia level.
> +else
> + check_per_thread()
> + {
> + perf stat -x, --per-thread -a 2>&1 sleep 1 | \
> + python $pythonchecker --per-thread --separator
> + }
> + check_per_thread
> +fi
> +
> +if [ $paranoia -gt 0 ];
> +then
> + echo check_per_die test skipped because of paranoia level.
> +else
> + check_per_die()
> + {
> + perf stat -x, --per-die -a 2>&1 sleep 1 | \
> + python $pythonchecker --per-die --separator
> + }
> + check_per_die
> +fi
> +
> +if [ $paranoia -gt 0 ];
> +then
> + echo check_per_node test skipped because of paranoia level.
> +else
> + check_per_node()
> + {
> + perf stat -x, --per-node -a 2>&1 sleep 1 | \
> + python $pythonchecker --per-node --separator
> + }
> + check_per_node
> +fi
> +
> +if [ $paranoia -gt 0 ];
> +then
> + echo check_per_socket test skipped because of paranoia level.
> +else
> + check_per_socket()
> + {
> + perf stat -x, --per-socket -a 2>&1 sleep 1 | \
> + python $pythonchecker --per-socket --separator
> + }
> + check_per_socket
> +fi
> +
> +check_no_args
> +check_interval
> +check_all_cpus_no_aggr
> +check_event
> +
> +exit 0
> +
Also here, please remove this blank line at the end of the file.
> --
> 2.32.0.605.g8dce9f2422-goog
>
--
- Arnaldo
Powered by blists - more mailing lists