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] [day] [month] [year] [list]
Message-ID: <CAP-5=fUY8d4DN+ekpj5B58kvoksrPcWpZU=iZhs+=bFfpakK3Q@mail.gmail.com>
Date: Tue, 8 Oct 2024 22:59:59 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: 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>, 
	Adrian Hunter <adrian.hunter@...el.com>, Kan Liang <kan.liang@...ux.intel.com>, 
	James Clark <james.clark@...aro.org>, Athira Jajeev <atrajeev@...ux.vnet.ibm.com>, 
	zhaimingbing <zhaimingbing@...s.chinamobile.com>, Thomas Richter <tmricht@...ux.ibm.com>, 
	Veronika Molnarova <vmolnaro@...hat.com>, Leo Yan <leo.yan@...ux.dev>, 
	Howard Chu <howardchu95@...il.com>, Ze Gao <zegao2021@...il.com>, 
	Weilin Wang <weilin.wang@...el.com>, linux-kernel@...r.kernel.org, 
	linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test

On Tue, Oct 8, 2024 at 10:38 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> On Tue, Oct 08, 2024 at 11:55:29AM -0700, Ian Rogers wrote:
> > On Tue, Oct 1, 2024 at 10:19 AM Ian Rogers <irogers@...gle.com> wrote:
> > >
> > > The path detection for "Setup struct perf_event_attr" test is brittle
> > > and leads to the test frequently not running. Running shell tests is
> > > reasonably robust, so make the test a shell test. Move the test files
> > > to reflect this.
> >
> > Ping.
> >
> > I think this is worthwhile cleanup for the attributes test. It should
> > avoid problems like:
> > https://lore.kernel.org/lkml/ZroNTkdA8XDFaDks@x1/
>
> Sorry, it's not clear to me what was the problem.  Can you please say it
> again briefly?

If you build perf like:
make -C tools/perf O=/tmp/perf

Then run the built perf test for the "Setup struct perf_event_attr" it
skips (causing the tests to bitrot and fixes to be sent by Veronika):
```
$ sudo /tmp/perf/perf test -vv perf_event_attr
capget syscall failed (No such file or directory - 2) fall back on root check
17: Setup struct perf_event_attr:
17: Setup struct perf_event_attr:
--- start ---
test child forked, pid 806601
Using CPUID GenuineIntel-6-8D-1
---- end(-2) ----
17: Setup struct perf_event_attr                                    : Skip
```

The issue is around the path set up, the test has a few path
expectations but they are brittle as shown above. While we could
endeavour to set up the path in C code, it makes sense to migrate the
test to a shell test due to the tests smaller size, ease of
environment variable manipulation, existing perf test support for
better path setup, etc. Ie let's not reinvent the shell test
infrastructure that handles python tests for the sake of one C test.
After this change:
```
$ sudo /tmp/perf/perf test attribute
76: Perf attribute expectations test                                : Ok
```

Thanks,
Ian

> >
> > > Ian Rogers (3):
> > >   perf test: Add a shell wrapper for "Setup struct perf_event_attr"
> > >   perf test: Remove C test wrapper for attr.py
> > >   perf test: Move attr files into shell directory where they are used
> > >
> > >  tools/perf/Makefile.perf                      |   5 +-
> > >  tools/perf/perf.c                             |   2 -
> > >  tools/perf/tests/Build                        |   1 -
> > >  tools/perf/tests/attr.c                       | 218 ------------------
> > >  tools/perf/tests/builtin-test.c               |   1 -
> > >  tools/perf/tests/shell/attr.sh                |  22 ++
> > >  tools/perf/tests/{ => shell}/attr/README      |   0
> > >  tools/perf/tests/{ => shell}/attr/base-record |   0
> > >  .../tests/{ => shell}/attr/base-record-spe    |   0
> > >  tools/perf/tests/{ => shell}/attr/base-stat   |   0
> > >  .../tests/{ => shell}/attr/system-wide-dummy  |   0
> > >  .../tests/{ => shell}/attr/test-record-C0     |   0
> > >  .../tests/{ => shell}/attr/test-record-basic  |   0
> > >  .../{ => shell}/attr/test-record-branch-any   |   0
> > >  .../attr/test-record-branch-filter-any        |   0
> > >  .../attr/test-record-branch-filter-any_call   |   0
> > >  .../attr/test-record-branch-filter-any_ret    |   0
> > >  .../attr/test-record-branch-filter-hv         |   0
> > >  .../attr/test-record-branch-filter-ind_call   |   0
> > >  .../attr/test-record-branch-filter-k          |   0
> > >  .../attr/test-record-branch-filter-u          |   0
> > >  .../tests/{ => shell}/attr/test-record-count  |   0
> > >  .../tests/{ => shell}/attr/test-record-data   |   0
> > >  .../{ => shell}/attr/test-record-dummy-C0     |   0
> > >  .../tests/{ => shell}/attr/test-record-freq   |   0
> > >  .../attr/test-record-graph-default            |   0
> > >  .../attr/test-record-graph-default-aarch64    |   0
> > >  .../{ => shell}/attr/test-record-graph-dwarf  |   0
> > >  .../{ => shell}/attr/test-record-graph-fp     |   0
> > >  .../attr/test-record-graph-fp-aarch64         |   0
> > >  .../attr/test-record-group-sampling           |   0
> > >  .../tests/{ => shell}/attr/test-record-group1 |   0
> > >  .../tests/{ => shell}/attr/test-record-group2 |   0
> > >  .../{ => shell}/attr/test-record-no-buffering |   0
> > >  .../{ => shell}/attr/test-record-no-inherit   |   0
> > >  .../{ => shell}/attr/test-record-no-samples   |   0
> > >  .../tests/{ => shell}/attr/test-record-period |   0
> > >  .../{ => shell}/attr/test-record-pfm-period   |   0
> > >  .../tests/{ => shell}/attr/test-record-raw    |   0
> > >  .../{ => shell}/attr/test-record-spe-period   |   0
> > >  .../attr/test-record-spe-period-term          |   0
> > >  .../attr/test-record-spe-physical-address     |   0
> > >  .../attr/test-record-user-regs-no-sve-aarch64 |   0
> > >  .../test-record-user-regs-old-sve-aarch64     |   0
> > >  .../attr/test-record-user-regs-sve-aarch64    |   0
> > >  .../perf/tests/{ => shell}/attr/test-stat-C0  |   0
> > >  .../tests/{ => shell}/attr/test-stat-basic    |   0
> > >  .../tests/{ => shell}/attr/test-stat-default  |   0
> > >  .../{ => shell}/attr/test-stat-detailed-1     |   0
> > >  .../{ => shell}/attr/test-stat-detailed-2     |   0
> > >  .../{ => shell}/attr/test-stat-detailed-3     |   0
> > >  .../tests/{ => shell}/attr/test-stat-group1   |   0
> > >  .../{ => shell}/attr/test-stat-no-inherit     |   0
> > >  tools/perf/tests/{ => shell/lib}/attr.py      |   0
> > >  tools/perf/tests/tests.h                      |   1 -
> > >  tools/perf/util/evsel.c                       | 122 +++++++++-
> > >  tools/perf/util/util.h                        |   7 -
> > >  57 files changed, 142 insertions(+), 237 deletions(-)
> > >  delete mode 100644 tools/perf/tests/attr.c
> > >  create mode 100755 tools/perf/tests/shell/attr.sh
> > >  rename tools/perf/tests/{ => shell}/attr/README (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/base-record (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/base-record-spe (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/base-stat (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/system-wide-dummy (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-C0 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-basic (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-any (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-any (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-any_call (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-any_ret (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-hv (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-ind_call (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-k (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-branch-filter-u (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-count (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-data (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-dummy-C0 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-freq (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-graph-default (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-graph-default-aarch64 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-graph-dwarf (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-graph-fp (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-graph-fp-aarch64 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-group-sampling (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-group1 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-group2 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-no-buffering (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-no-inherit (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-no-samples (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-period (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-pfm-period (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-raw (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-spe-period (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-spe-period-term (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-spe-physical-address (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-user-regs-no-sve-aarch64 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-user-regs-old-sve-aarch64 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-record-user-regs-sve-aarch64 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-C0 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-basic (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-default (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-detailed-1 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-detailed-2 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-detailed-3 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-group1 (100%)
> > >  rename tools/perf/tests/{ => shell}/attr/test-stat-no-inherit (100%)
> > >  rename tools/perf/tests/{ => shell/lib}/attr.py (100%)
> > >
> > > --
> > > 2.46.1.824.gd892dcdcdd-goog
> > >

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ