linux-kernel - Re: [PATCH 2/2] perf test: Add basic stat and topdown group test

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:   Thu, 5 May 2022 11:35:03 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     Caleb Biggers <caleb.biggers@...el.com>,
        Perry Taylor <perry.taylor@...el.com>,
        Kshipra Bopardikar <kshipra.bopardikar@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Ravi Bangoria <ravi.bangoria@....com>,
        Andi Kleen <ak@...ux.intel.com>,
        Haowen Bai <baihaowen@...zu.com>,
        Riccardo Mancini <rickyman7@...il.com>,
        Kim Phillips <kim.phillips@....com>,
        Madhavan Srinivasan <maddy@...ux.ibm.com>,
        Shunsuke Nakamura <nakamura.shun@...itsu.com>,
        Florian Fischer <florian.fischer@...q.space>,
        linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        Zhengjun Xing <zhengjun.xing@...ux.intel.com>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 2/2] perf test: Add basic stat and topdown group test

On Thu, May 5, 2022 at 11:19 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
>
> On 5/5/2022 11:22 AM, Ian Rogers wrote:
> > On Thu, May 5, 2022 at 5:12 AM Liang, Kan <kan.liang@...ux.intel.com> wrote:
> >>
> >> On 5/5/2022 12:38 AM, Ian Rogers wrote:
> >>> Add a basic stat test.
> >>> Add two tests of grouping behavior for topdown events. Topdown events
> >>> are special as they must be grouped with the slots event first.
> >>>
> >>> Signed-off-by: Ian Rogers <irogers@...gle.com>
> >>> ---
> >>>    tools/perf/tests/shell/stat.sh | 65 ++++++++++++++++++++++++++++++++++
> >>>    1 file changed, 65 insertions(+)
> >>>    create mode 100755 tools/perf/tests/shell/stat.sh
> >>>
> >>> diff --git a/tools/perf/tests/shell/stat.sh b/tools/perf/tests/shell/stat.sh
> >>> new file mode 100755
> >>> index 000000000000..80869ea6debc
> >>> --- /dev/null
> >>> +++ b/tools/perf/tests/shell/stat.sh
> >>> @@ -0,0 +1,65 @@
> >>> +#!/bin/sh
> >>> +# perf stat tests
> >>> +# SPDX-License-Identifier: GPL-2.0
> >>> +
> >>> +set -e
> >>> +
> >>> +err=0
> >>> +test_default_stat() {
> >>> +  echo "Basic stat command test"
> >>> +  if ! perf stat true 2>&1 | egrep -q "Performance counter stats for 'true':"
> >>> +  then
> >>> +    echo "Basic stat command test [Failed]"
> >>> +    err=1
> >>> +    return
> >>> +  fi
> >>> +  echo "Basic stat command test [Success]"
> >>> +}
> >>> +
> >>> +test_topdown_groups() {
> >>> +  # Topdown events must be grouped with the slots event first. Test that
> >>> +  # parse-events reorders this.
> >>> +  echo "Topdown event group test"
> >>> +  if ! perf stat -e '{slots,topdown-retiring}' true > /dev/null 2>&1
> >>> +  then
> >>> +    echo "Topdown event group test [Skipped event parsing failed]"
> >>> +    return
> >>> +  fi
> >>> +  if perf stat -e '{slots,topdown-retiring}' true 2>&1 | egrep -q "<not supported>"
> >>> +  then
> >>> +    echo "Topdown event group test [Failed events not supported]"
> >>> +    err=1
> >>> +    return
> >>> +  fi
> >>> +  if perf stat -e '{topdown-retiring,slots}' true 2>&1 | egrep -q "<not supported>"
> >>> +  then
> >>> +    echo "Topdown event group test [Failed slots not reordered first]"
> >>> +    err=1
> >>> +    return
> >>> +  fi
> >>> +  echo "Topdown event group test [Success]"
> >>> +}
> >>> +
> >>> +test_topdown_weak_groups() {
> >>> +  # Weak groups break if the perf_event_open of multiple grouped events
> >>> +  # fails. Breaking a topdown group causes the events to fail. Test a very large
> >>> +  # grouping to see that the topdown events aren't broken out.
> >>> +  echo "Topdown weak groups test"
> >>> +  if ! perf stat -e '{slots,topdown-bad-spec,topdown-be-bound,topdown-fe-bound,topdown-retiring},branch-instructions,branch-misses,bus-cycles,cache-misses,cache-references,cpu-cycles,instructions,mem-loads,mem-stores,ref-cycles,baclears.any,ARITH.DIVIDER_ACTIVE' true > /dev/null 2>&1
> >>> +  then
> >>> +    echo "Topdown weak groups test [Skipped event parsing failed]"
> >>> +    return
> >>> +  fi
> >>> +  if perf stat -e '{slots,topdown-bad-spec,topdown-be-bound,topdown-fe-bound,topdown-retiring,branch-instructions,branch-misses,bus-cycles,cache-misses,cache-references,cpu-cycles,instructions,mem-loads,mem-stores,ref-cycles,baclears.any,ARITH.DIVIDER_ACTIVE}:W' true 2>&1 | egrep -q "<not supported>"
> >>> +  then
> >>> +    echo "Topdown weak groups test [Failed events not supported]"
> >>> +    err=1
> >>> +    return
> >>> +  fi
> >>> +  echo "Topdown weak groups test [Success]"
> >>> +}
> >>> +
> >>
> >> Should we check the existence of the slots event before the test?
> >> The perf metrics feature only be available on the new platform after
> >> ICL. It doesn't work on Atom.
> >>
> >> Also, I think the test may fails on the hybrid platform, since big core
> >> and small core have different formula for the topdown. I think we should
> >> avoid the test for the hybrid platform for now.
> >> +Zhengjun, who is fixing the topdown gap for the hybrid platform. I
> >> think he may take care of the hybrid support later.
> >
> > Thanks Kan, the test filters out systems that don't support the events
> > and silently skips the test. The main purpose of the test is to make
> > sure the somewhat complicated grouping operations for Icelake have
> > some coverage. Adding more coverage for hybrid would be great, but not
> > something I think gates this change.
> >
>
> Sure, we can add the coverage for hybrid later. But please make sure the
> test can filter out both the systems which doesn't support perf metircs
> and the hybrid system.
>
> Thanks,
> Kan

If the test fails on hybrid then that feels like value add :-) We
genuinely have broken grouping functions. We could just add to the
test a skip if /sys/devices/cpu_core and /sys/devices/cpu_atom
directories exist (making assumptions on where sysfs is mounted). I'm
not yet able to test on Alderlake hence not wanting to have a lot of
untested code.

Thanks,
Ian

> > Thanks,
> > Ian
> >
> >> Thanks,
> >> Kan
> >>> +test_default_stat
> >>> +test_topdown_groups
> >>> +test_topdown_weak_groups
> >>> +exit $err