linux-kernel - Re: [PATCH 1/2] perf test: Reorder event name checks in stat STD output linter

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 23 Jun 2023 16:22:25 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Jiri Olsa <jolsa@...nel.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-perf-users@...r.kernel.org,
        Kan Liang <kan.liang@...ux.intel.com>
Subject: Re: [PATCH 1/2] perf test: Reorder event name checks in stat STD
 output linter

On Fri, Jun 23, 2023 at 4:01 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On AMD machines, the perf stat STD output test failed like below:
>
>   $ sudo ./perf test -v 98
>    98: perf stat STD output linter                                     :
>   --- start ---
>   test child forked, pid 1841901
>   Checking STD output: no argswrong event metric.
>     expected 'GHz' in 108,121 stalled-cycles-frontend  # 10.88% frontend cycles idle
>   test child finished with -1
>   ---- end ----
>   perf stat STD output linter: FAILED!
>
> This is because there are stalled-cycles-{frontend,backend} events are
> used by default.  The current logic checks the event_name array to find
> which event it's running.  But 'cycles' event comes before those stalled
> cycles event and it matches first.  So it tries to find 'GHz' metric
> in the output (which is for the 'cycles') and fails.
>
> Move the stalled-cycles-{frontend,backend} events before 'cycles' so
> that it can find the stalled cycles events first.
>
> Also add a space after 'no args' test name for consistency.
>
> Fixes: 99a04a48f225 ("perf test: Add test case for the standard 'perf stat' output")
> Cc: Kan Liang <kan.liang@...ux.intel.com>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>

Acked-by: Ian Rogers <irogers@...gle.com>

Thanks,
Ian

> ---
>  tools/perf/tests/shell/lib/stat_output.sh | 2 +-
>  tools/perf/tests/shell/stat+std_output.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/shell/lib/stat_output.sh b/tools/perf/tests/shell/lib/stat_output.sh
> index 363979b1123d..698343f0ecf9 100644
> --- a/tools/perf/tests/shell/lib/stat_output.sh
> +++ b/tools/perf/tests/shell/lib/stat_output.sh
> @@ -9,7 +9,7 @@ function ParanoidAndNotRoot()
>  # $1 name $2 extra_opt
>  check_no_args()
>  {
> -        echo -n "Checking $1 output: no args"
> +        echo -n "Checking $1 output: no args "
>          perf stat $2 true
>          commachecker --no-args
>          echo "[Success]"
> diff --git a/tools/perf/tests/shell/stat+std_output.sh b/tools/perf/tests/shell/stat+std_output.sh
> index 98cc3356a04a..1f70aab45184 100755
> --- a/tools/perf/tests/shell/stat+std_output.sh
> +++ b/tools/perf/tests/shell/stat+std_output.sh
> @@ -10,8 +10,8 @@ set -e
>
>  stat_output=$(mktemp /tmp/__perf_test.stat_output.std.XXXXX)
>
> -event_name=(cpu-clock task-clock context-switches cpu-migrations page-faults cycles instructions branches branch-misses stalled-cycles-frontend stalled-cycles-backend)
> -event_metric=("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "GHz" "insn per cycle" "/sec" "of all branches" "frontend cycles idle" "backend cycles idle")
> +event_name=(cpu-clock task-clock context-switches cpu-migrations page-faults stalled-cycles-frontend stalled-cycles-backend cycles instructions branches branch-misses)
> +event_metric=("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "frontend cycles idle" "backend cycles idle" "GHz" "insn per cycle" "/sec" "of all branches")
>
>  metricgroup_name=(TopdownL1 TopdownL2)
>
> --
> 2.41.0.162.gfafddb0af9-goog
>