[PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test

Ian Rogers posted 3 patches 1 month, 3 weeks ago
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%)
[PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Ian Rogers 1 month, 3 weeks ago
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.

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
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Namhyung Kim 1 month, 2 weeks ago
On Tue, 01 Oct 2024 10:19:47 -0700, Ian Rogers 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.
> 
> 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
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
Namhyung
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Namhyung Kim 1 month, 2 weeks ago
On 2024-10-10 15:50 -0700, Namhyung Kim wrote:
> On Tue, 01 Oct 2024 10:19:47 -0700, Ian Rogers 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.
> > 
> > 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
> > 
> > [...]
> 
> Applied to perf-tools-next, thanks!

Dropped from perf-tools-next due to build failures on PPC.

https://lore.kernel.org/r/20241011102330.02bece12@canb.auug.org.au

Thanks,
Namhyung
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Athira Rajeev 1 month, 2 weeks ago

> On 11 Oct 2024, at 12:18 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> On 2024-10-10 15:50 -0700, Namhyung Kim wrote:
>> On Tue, 01 Oct 2024 10:19:47 -0700, Ian Rogers 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.
>>> 
>>> 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
>>> 
>>> [...]
>> 
>> Applied to perf-tools-next, thanks!
> 
> Dropped from perf-tools-next due to build failures on PPC.
> 
> https://lore.kernel.org/r/20241011102330.02bece12@canb.auug.org.au

Hi Namhyung

I am checking this on powerpc. Will respond with the findings

Thanks
Athira
> 
> Thanks,
> Namhyung
> 
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Athira Rajeev 1 month, 2 weeks ago

> On 13 Oct 2024, at 11:08 AM, Athira Rajeev <atrajeev@linux.vnet.ibm.com> wrote:
> 
> 
> 
>> On 11 Oct 2024, at 12:18 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>> 
>> On 2024-10-10 15:50 -0700, Namhyung Kim wrote:
>>> On Tue, 01 Oct 2024 10:19:47 -0700, Ian Rogers 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.
>>>> 
>>>> 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
>>>> 
>>>> [...]
>>> 
>>> Applied to perf-tools-next, thanks!
>> 
>> Dropped from perf-tools-next due to build failures on PPC.
>> 
>> https://lore.kernel.org/r/20241011102330.02bece12@canb.auug.org.au
> 
> Hi Namhyung
> 
> I am checking this on powerpc. Will respond with the findings
> 
> Thanks
> Athira

I tried on tmp.perf-tools-next branch from git://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git

But hit a patch apply issue in second patch

error: patch failed: tools/perf/tests/tests.h:83
error: tools/perf/tests/tests.h: patch does not apply

Applied this change separately and hit the compilation error reported by Stephen

As suggested by Ian , tried below change and it did solve the compilation issue

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 37c338b0f8b2..cc5c919dcd0f 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -6,6 +6,11 @@
  * copyright notes.
  */
  +/* Powerpc needs __SANE_USERSPACE_TYPES__ before <linux/types.h> to select
+ * 'int-ll64.h' and avoid compile warnings when printing __u64 with %llu.
+ */
+#define __SANE_USERSPACE_TYPES__
+
 #include <byteswap.h>
 #include <errno.h>
 #include <inttypes.h>

I could also verify running the test as well

# ./perf test attribute
 65: Perf attribute expectations test                                : Ok


With the change above,

Tested-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com <mailto:atrajeev@linux.vnet.ibm.com>>

Thanks
Athira


>> 
>> Thanks,
>> Namhyung
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Ian Rogers 1 month, 2 weeks ago
On Tue, Oct 1, 2024 at 10:19 AM Ian Rogers <irogers@google.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/

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
>
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Namhyung Kim 1 month, 2 weeks ago
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@google.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?

Thanks,
Namhyung

> 
> > 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
> >
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Ian Rogers 1 month, 2 weeks ago
On Tue, Oct 8, 2024 at 10:38 PM Namhyung Kim <namhyung@kernel.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@google.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
> > >
Re: [PATCH v1 0/3] Make a "Setup struct perf_event_attr" a shell test
Posted by Namhyung Kim 1 month, 2 weeks ago
On Tue, Oct 08, 2024 at 10:59:59PM -0700, Ian Rogers wrote:
> On Tue, Oct 8, 2024 at 10:38 PM Namhyung Kim <namhyung@kernel.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@google.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
> ```

Ok, thanks for the explanation!
Namhyung

> 
> > >
> > > > 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
> > > >