Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230623230139.985594-1-namhyung@kernel.org>
In-Reply-To: <20230623230139.985594-1-namhyung@kernel.org>
From:   Ian Rogers <irogers@google.com>
Date:   Fri, 23 Jun 2023 16:22:25 -0700
Message-ID: <CAP-5=fX6Of3pKjZGLEaNL58opn2veXxOhrY_sQqgY6hgUJeaBw@mail.gmail.com>
Subject: Re: [PATCH 1/2] perf test: Reorder event name checks in stat STD
 output linter
To:     Namhyung Kim <namhyung@kernel.org>
Cc:     Arnaldo Carvalho de Melo <acme@kernel.org>,
        Jiri Olsa <jolsa@kernel.org>,
        Adrian Hunter <adrian.hunter@intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-perf-users@vger.kernel.org,
        Kan Liang <kan.liang@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jun 23, 2023 at 4:01=E2=80=AFPM Namhyung Kim <namhyung@kernel.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 sta=
t' output")
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Acked-by: Ian Rogers <irogers@google.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=3D$(mktemp /tmp/__perf_test.stat_output.std.XXXXX)
>
> -event_name=3D(cpu-clock task-clock context-switches cpu-migrations page-=
faults cycles instructions branches branch-misses stalled-cycles-frontend s=
talled-cycles-backend)
> -event_metric=3D("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "GH=
z" "insn per cycle" "/sec" "of all branches" "frontend cycles idle" "backen=
d cycles idle")
> +event_name=3D(cpu-clock task-clock context-switches cpu-migrations page-=
faults stalled-cycles-frontend stalled-cycles-backend cycles instructions b=
ranches branch-misses)
> +event_metric=3D("CPUs utilized" "CPUs utilized" "/sec" "/sec" "/sec" "fr=
ontend cycles idle" "backend cycles idle" "GHz" "insn per cycle" "/sec" "of=
 all branches")
>
>  metricgroup_name=3D(TopdownL1 TopdownL2)
>
> --
> 2.41.0.162.gfafddb0af9-goog
>