perf test attr: Report the failures again and cope with more than one core PMU
Subsystems (2)
Arnaldo Carvalho de Melo
maintainerv4 cover perf test attr: Report the failures again and cope with more than one core PMU
From: Arnaldo Carvalho de Melo <[email protected]>
Hello all,
perf test 82 stopped reporting failures when it got its shell wrapper (8519e4f44c2af722), so expectations that no longer match perf went unverified. The prerequisite fixes are upstream now, so they are not repeated here.
This series updates those expectations and makes the test cope with machines with no core PMU, or with more than one. Asking for one event on the latter gives one event per core PMU. The expectations ignore what that adds; the commit messages say what and why.
The tests that map ring buffers take turns on the per user mlock budget (kernel.perf_event_mlock_kb) through a robust, process-shared mutex. The sysctl is left alone. A test killed while holding the budget does not leave the next one waiting.
Tested: 'perf test -vv attribute' Ok on an Amlogic A311D (aarch64, as root), an Intel i7-14700K (hybrid, two core PMUs), x86_64 (AMD Ryzen 9 9950X3D, kernel 7.2.10-200.fc44) and riscv64 (Orange Pi RV). On riscv64 the stat tests run and the record tests report unsupp, as sampling is not available there (no Sscofpmf).
Best regards,
- Arnaldo
What changed from v3:
- sashiko-bot's v3 review: the python binding no longer leaks the parse error entries it prints when parsing fails.
- sashiko-bot's v3 review: attr.py no longer compares an event against itself unrecorded, and masks both sides with both events' types.
- sashiko-bot's v3 review: evsel__open_cpu() no longer leaks the fds of an attempt abandoned to disable missing features.
- sashiko-bot's v3 review: <stdio.h> included where FILE and snprintf() are used, in builtin-test.c and tests-scripts.c.
What changed from v2:
- sashiko-bot's v2 review: a one-component kernel_since or kernel_until (f.i. kernel_until = 6) no longer crashes attr.py.
- sashiko-bot's v2 review: mutex_trylock() and cond_wait() now take over a robust mutex left behind by a dead owner, as mutex_lock().
What changed from v1:
- Namhyung's python fix and Michael Petlan's two fixes are upstream now, so they are dropped from the series.
- Namhyung's v1 report (test-record-branch-filter-any_call): probe events are no longer dumped, and the core PMU fallout is masked.
- New, not from review: the 'requires = core-pmu' skip, the skip reasons and the mlock budget part.
- New, not from review: the events of the other core PMUs are left out, when the expectations name a core PMU.
tools/build/Makefile.feature | 1 + tools/build/feature/Makefile | 4 + tools/build/feature/test-all.c | 5 + .../build/feature/test-pthread-mutex-robust.c | 18 + tools/perf/Documentation/perf-test.txt | 36 +- tools/perf/Makefile.config | 4 + tools/perf/tests/backward-ring-buffer.c | 2 +- tools/perf/tests/builtin-test.c | 174 ++++++- tools/perf/tests/code-reading.c | 2 +- tools/perf/tests/keep-tracking.c | 2 +- tools/perf/tests/mmap-basic.c | 1 + tools/perf/tests/openat-syscall-tp-fields.c | 1 + tools/perf/tests/perf-record.c | 1 + tools/perf/tests/perf-time-to-tsc.c | 1 + tools/perf/tests/shell/attr/README | 10 +- tools/perf/tests/shell/attr/base-stat-group | 45 ++ .../tests/shell/attr/test-record-graph-dwarf | 2 +- .../shell/attr/test-record-group-sampling | 5 +- .../shell/attr/test-record-group-sampling1 | 5 +- .../shell/attr/test-record-group-sampling2 | 5 +- .../perf/tests/shell/attr/test-record-group1 | 3 +- .../perf/tests/shell/attr/test-record-group2 | 3 +- .../perf/tests/shell/attr/test-record-group3 | 3 +- tools/perf/tests/shell/attr/test-stat-C0 | 1 + tools/perf/tests/shell/attr/test-stat-basic | 1 + tools/perf/tests/shell/attr/test-stat-default | 91 ++-- .../tests/shell/attr/test-stat-detailed-1 | 99 ++-- .../tests/shell/attr/test-stat-detailed-2 | 111 +++-- .../tests/shell/attr/test-stat-detailed-3 | 115 +++-- tools/perf/tests/shell/attr/test-stat-group1 | 3 +- .../tests/shell/attr/test-stat-no-inherit | 1 + tools/perf/tests/shell/drm_pmu.sh | 2 +- tools/perf/tests/shell/ftrace.sh | 2 +- tools/perf/tests/shell/kallsyms.sh | 2 +- tools/perf/tests/shell/lib/attr.py | 461 ++++++++++++++++-- tools/perf/tests/shell/list.sh | 2 +- tools/perf/tests/shell/perf_sched_stats.sh | 2 +- tools/perf/tests/shell/probe_vfs_getname.sh | 2 +- tools/perf/tests/shell/python-use.sh | 2 +- tools/perf/tests/shell/schedstat_snapshots.sh | 2 +- tools/perf/tests/shell/stat+csv_output.sh | 2 +- tools/perf/tests/shell/stat+csv_summary.sh | 2 +- .../tests/shell/stat+event_uniquifying.sh | 2 +- tools/perf/tests/shell/stat+json_output.sh | 2 +- tools/perf/tests/shell/stat+shadow_stat.sh | 2 +- tools/perf/tests/shell/stat+std_output.sh | 2 +- tools/perf/tests/shell/stat.sh | 2 +- .../perf/tests/shell/stat_all_metricgroups.sh | 2 +- tools/perf/tests/shell/stat_all_metrics.sh | 2 +- tools/perf/tests/shell/stat_all_pfm.sh | 2 +- .../tests/shell/stat_bpf_counters_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_values.sh | 2 +- tools/perf/tests/shell/test_ilist_python.sh | 2 +- .../perf/tests/shell/test_stat_cpi_python.sh | 2 +- .../tests/shell/test_test_junit_output.sh | 5 +- .../shell/test_uprobe_from_different_cu.sh | 2 +- tools/perf/tests/sw-clock.c | 2 +- tools/perf/tests/task-exit.c | 1 + tools/perf/tests/tests-scripts.c | 85 +++- tools/perf/tests/tests.h | 17 + tools/perf/util/evsel.c | 42 +- tools/perf/util/evsel.h | 1 + tools/perf/util/mutex.c | 101 +++- tools/perf/util/mutex.h | 6 + tools/perf/util/pfm.c | 1 + tools/perf/util/print-events.c | 1 + tools/perf/util/python.c | 2 + 68 files changed, 1256 insertions(+), 272 deletions(-)
base-commit: cb9dba6f4764fc68ab75c36e332887332339f9c3
v1-head: b1300831ecc4503d
v2-head: da166ef7a002
v3-head: c5f0897aceb01743
Arnaldo Carvalho de Melo
maintainerPATCH v4 1/21 perf test attr: Fix record dwarf sample_type expectation
From: Arnaldo Carvalho de Melo <[email protected]>
Since commit bdd051e24914 ("perf record: Split --data-mmap option"), record_opts__parse_callchain() sets record_data_mmap instead of record->sample_address for the DWARF unwind mode, so -d/--data and its implied PERF_SAMPLE_DATA_SRC are not enabled implicitly anymore.
As a result, --call-graph dwarf no longer sets PERF_SAMPLE_ADDR and PERF_SAMPLE_DATA_SRC, making the test fail:
expected sample_type=45359, got 12583
where 45359 (0xb12f) is:
IP|TID|TIME|ADDR|CALLCHAIN|PERIOD|REGS_USER|STACK_USER|DATA_SRC
and 12583 (0x3127) is:
IP|TID|TIME|CALLCHAIN|PERIOD|REGS_USER|STACK_USER
i.e. the values differ exactly by PERF_SAMPLE_ADDR (0x8) and PERF_SAMPLE_DATA_SRC (0x8000). The mmap_data=1 expectation is still correct as DWARF now enables record_data_mmap.
This went unnoticed until tests/attr.sh stopped ignoring attr.py's return value.
--- tools/perf/tests/shell/attr/test-record-graph-dwarf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/tests/shell/attr/test-record-graph-dwarf +1 -1
@@ -4,7 +4,7 @@ args = --no-bpf-event --call-graph dwarf -- kill >/dev/null 2>&1 ret = 1 [event:base-record] -sample_type=45359 +sample_type=12583 exclude_callchain_user=1 sample_stack_user=8192 # TODO different for each arch, no support for that now
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 01/21] perf test attr: Fix record dwarf sample_type expectation
Sashiko has reviewed this patch and found no issues.
Quoted 24 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Since commit
bdd051e24914("perf record: Split --data-mmap option"),record_opts__parse_callchain()setsrecord_data_mmapinstead ofrecord->sample_addressfor the DWARF unwind mode, so -d/--data and its impliedPERF_SAMPLE_DATA_SRCare not enabled implicitly anymore.As a result, --call-graph dwarf no longer sets
PERF_SAMPLE_ADDRandPERF_SAMPLE_DATA_SRC, making the test fail:expected sample_type=45359, got 12583
where 45359 (
0xb12f) is:IP|TID|TIME|ADDR|CALLCHAIN|PERIOD|REGS_USER|STACK_USER|DATA_SRC
and 12583 (
0x3127) is:IP|TID|TIME|CALLCHAIN|PERIOD|REGS_USER|STACK_USER
i.e. the values differ exactly by
PERF_SAMPLE_ADDR(0x8) andPERF_SAMPLE_DATA_SRC(0x8000). The mmap_data=1 expectation is still correct as DWARF now enablesrecord_data_mmap.
Quoted 6 lines
This went unnoticed until
tests/attr.shstopped ignoring attr.py's return value.Fixes bdd051e249141c79 ("perf record: Split --data-mmap option")Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Arnaldo Carvalho de Melo
maintainerPATCH v4 2/21 perf python: Free the parse error entries when parsing fails
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
pyrf__parse_events() prints the entries parse_events_error__print() receives with the details of a failed parse, but returns without freeing them, leaking whatever parse_events_error__handle() allocated, so call parse_events_error__exit() on both paths, as the other parse_events() callers do.
The perf test attr core PMU probe takes this failure path on machines without a core PMU, where the leak then happens on every test run.
--- tools/perf/util/python.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/python.c +2 -0
@@ -3824,10 +3824,12 @@ static PyObject *pyrf__parse_events(PyObject *self, PyObject *args)perf_evlist__set_maps (evlist__core (evlist), cpus, threads);if (parse_events (evlist, input, &err)) {parse_events_error__print (&err, input); +parse_events_error__exit (&err);PyErr_SetFromErrno (PyExc_OSError);evlist__put (evlist);return NULL ; } +parse_events_error__exit (&err); result =pyrf_evlist__from_evlist (evlist);evlist__put (evlist);return result;
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 02/21] perf python: Free the parse error entries when parsing fails
Sashiko has reviewed this patch and found no issues.
Quoted 11 lines
From: Arnaldo Carvalho de Melo <[email protected]>
pyrf__parse_events()prints the entriesparse_events_error__print()receives with the details of a failed parse, but returns without freeing them, leaking whateverparse_events_error__handle()allocated, so callparse_events_error__exit()on both paths, as the otherparse_events()callers do.The perf test attr core PMU probe takes this failure path on machines without a core PMU, where the leak then happens on every test run.
Quoted 6 lines
Fixes f081defccd93 ("perf python: Add parse_events function")Reported-by Sashiko <[email protected]>Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
Arnaldo Carvalho de Melo
maintainerPATCH v4 3/21 perf test attr: Allow tests to require a core PMU
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
On machines without a usable core PMU, like an aarch64 box with no DT node for the ARM PMU, the expectations that pin the legacy hardware encoding can't be checked: without a core PMU at all 'cycles' doesn't even resolve, and with one that can't be opened 'perf record' takes the software fallback and records attributes they can never match:
expected type=0, got 1
expected config=0, got 1
FAILED '.../attr/test-record-group-sampling1' - match failure
And since attr.py stops at the first failure, every expectation that would come after that one ends up not being run at all.
Add a 'requires' limitation, modelled after the existing 'arch', 'auxv' and 'kernel_since/until' ones, with 'core-pmu' as its first value, that is checked by asking perf itself to open 'cycles' via its python
binding: a parse failure or an ENOENT means there is no PMU that can count it, hence nothing to check and the test gets skipped, while any other error means we can't tell, so the test keeps running as it does today, where there is no binding to ask either.
When there is no core PMU, asking the binding fails and prints a parse error, once for every test that asks, so the answer is kept: it is asked once per run. A value other than 'core-pmu' is warned about, not silently ignored, so a typo can't turn the skip into a no-op.
An old distro python package may have a 'perf' module without parse_events(), i.e. there is nothing to ask with, so that AttributeError gets the same treatment as the ImportError: let the expectations speak.
The expectations that have a strict 'type=0', plus the group tests, where the fallback drops a member and leaves nothing to compare the group leader attributes with, now state that they require a core PMU. They are marked here, before the commit that has the group tests ask for the legacy encodings, so the requisite comes with that dependency.
--- .../shell/attr/test-record-group-sampling | 1 + .../shell/attr/test-record-group-sampling1 | 1 + .../shell/attr/test-record-group-sampling2 | 1 + .../perf/tests/shell/attr/test-record-group1 | 1 + .../perf/tests/shell/attr/test-record-group2 | 1 + .../perf/tests/shell/attr/test-record-group3 | 1 + tools/perf/tests/shell/attr/test-stat-C0 | 1 + tools/perf/tests/shell/attr/test-stat-basic | 1 + tools/perf/tests/shell/attr/test-stat-default | 1 + .../tests/shell/attr/test-stat-detailed-1 | 1 + .../tests/shell/attr/test-stat-detailed-2 | 1 + .../tests/shell/attr/test-stat-detailed-3 | 1 + tools/perf/tests/shell/attr/test-stat-group1 | 1 + .../tests/shell/attr/test-stat-no-inherit | 1 + tools/perf/tests/shell/lib/attr.py | 63 +++++++++++++++++++ 15 files changed, 77 insertions(+)
tools/perf/tests/shell/attr/test-record-group-sampling +1 -0
@@ -3,6 +3,7 @@ command = record args = --no-bpf-event -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 ret = 1 kernel_until = 6.12 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-record-group-sampling1 +1 -0
@@ -3,6 +3,7 @@ command = record args = --no-bpf-event -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-record-group-sampling2 +1 -0
@@ -3,6 +3,7 @@ command = record args = --no-bpf-event -c 10000 -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-record-group1 +1 -0
@@ -2,6 +2,7 @@ command = record args = --no-bpf-event -e '{cycles,instructions}' kill >/dev/null 2>&1 ret = 1 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-record-group2 +1 -0
@@ -3,6 +3,7 @@ command = record args = --no-bpf-event -e '{cycles/period=1234000/,instructions/period=6789000/}:S' kill >/dev/null 2>&1 ret = 1 kernel_until = 6.12 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-record-group3 +1 -0
@@ -3,6 +3,7 @@ command = record args = --no-bpf-event -e '{cycles/period=1234000/,instructions/period=6789000/}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 +requires = core-pmu [event-1:base-record] fd=1
tools/perf/tests/shell/attr/test-stat-C0 +1 -0
@@ -2,6 +2,7 @@ command = stat args = -e cycles -C 0 kill >/dev/null 2>&1 ret = 1 +requires = core-pmu [event:base-stat] # events are disabled by default when attached to cpu
tools/perf/tests/shell/attr/test-stat-basic +1 -0
@@ -2,6 +2,7 @@ command = stat args = -e cycles kill >/dev/null 2>&1 ret = 1 +requires = core-pmu [event:base-stat] optional=1
tools/perf/tests/shell/attr/test-stat-default +1 -0
@@ -2,6 +2,7 @@ command = stat args = kill >/dev/null 2>&1 ret = 1 +requires = core-pmu # PERF_TYPE_SOFTWARE / PERF_COUNT_SW_TASK_CLOCK [event1:base-stat]
tools/perf/tests/shell/attr/test-stat-detailed-1 +1 -0
@@ -2,6 +2,7 @@ command = stat args = -d kill >/dev/null 2>&1 ret = 1 +requires = core-pmu # PERF_TYPE_SOFTWARE / PERF_COUNT_SW_TASK_CLOCK
tools/perf/tests/shell/attr/test-stat-detailed-2 +1 -0
@@ -2,6 +2,7 @@ command = stat args = -dd kill >/dev/null 2>&1 ret = 1 +requires = core-pmu # PERF_TYPE_SOFTWARE / PERF_COUNT_SW_TASK_CLOCK
tools/perf/tests/shell/attr/test-stat-detailed-3 +1 -0
@@ -2,6 +2,7 @@ command = stat args = -ddd kill >/dev/null 2>&1 ret = 1 +requires = core-pmu # PERF_TYPE_SOFTWARE / PERF_COUNT_SW_TASK_CLOCK
tools/perf/tests/shell/attr/test-stat-group1 +1 -0
@@ -2,6 +2,7 @@ command = stat args = -e '{cycles,instructions}' kill >/dev/null 2>&1 ret = 1 +requires = core-pmu [event-1:base-stat] fd=1
tools/perf/tests/shell/attr/test-stat-no-inherit +1 -0
@@ -2,6 +2,7 @@ command = stat args = -i -e cycles kill >/dev/null 2>&1 ret = 1 +requires = core-pmu [event:base-stat] inherit=0
tools/perf/tests/shell/lib/attr.py +63 -0
99 lines not on this page · raw message
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 03/21] perf test attr: Allow tests to require a core PMU
Sashiko has reviewed this patch and found no issues.
Quoted 15 lines
From: Arnaldo Carvalho de Melo <[email protected]>
On machines without a usable core PMU, like an aarch64 box with no DT node for the ARM PMU, the expectations that pin the legacy hardware encoding can't be checked: without a core PMU at all 'cycles' doesn't even resolve, and with one that can't be opened 'perf record' takes the software fallback and records attributes they can never match:
expected type=0, got 1
expected config=0, got 1
FAILED '.../attr/test-record-group-sampling1' - match failureAnd since
attr.pystops at the first failure, every expectation that would come after that one ends up not being run at all.
Quoted 6 lines
[ ... ]
They are marked here, before the commit that has the group tests ask for the legacy encodings, so the requisite comes with that dependency.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Arnaldo Carvalho de Melo
maintainerPATCH v4 4/21 perf test attr: Fix legacy event encodings in group tests
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Since commit b1c5efbfd92e ("perf parse-events: Remove hard coded legacy hardware and cache parsing"), event names are resolved with sysfs/JSON priority over the legacy encodings, so bare cache-misses and instructions now become the core PMU sysfs/JSON events (on x86 type 4, configs 0x412e and 0xc0) rather than the legacy PERF_TYPE_HARDWARE/config=3 and config=1 events the tests expect, e.g.:
expected config=3, got 0
expected disabled=0, got 1
expected mmap=0, got 1
expected comm=0, got 1
expected freq=0, got 1
expected enable_on_exec=0, got 1
expected task=0, got 1
FAILED '.../test-record-group-sampling1' - match failure
The shown diff is against the cycles group leader, as no result event matches the expected cache-misses member anymore.
Request the legacy encodings explicitly with the legacy-hardware-config term, added in commit 0012e0fa221bf9cc ("perf jevents: Add legacy-hardware and legacy-cache json"), keeping the original attribute expectations unchanged, in:
test-record-group1
test-record-group2
test-record-group3
test-record-group-sampling
test-record-group-sampling1
test-record-group-sampling2
test-stat-group1
This went unnoticed until tests/attr.sh stopped ignoring attr.py's return value.
--- tools/perf/tests/shell/attr/README | 10 +++++----- tools/perf/tests/shell/attr/test-record-group-sampling | 4 ++-- .../perf/tests/shell/attr/test-record-group-sampling1 | 4 ++-- .../perf/tests/shell/attr/test-record-group-sampling2 | 4 ++-- tools/perf/tests/shell/attr/test-record-group1 | 2 +- tools/perf/tests/shell/attr/test-record-group2 | 2 +- tools/perf/tests/shell/attr/test-record-group3 | 2 +- tools/perf/tests/shell/attr/test-stat-group1 | 2 +- 8 files changed, 15 insertions(+), 15 deletions(-)
tools/perf/tests/shell/attr/README +5 -5
@@ -49,10 +49,10 @@ Following tests are defined (with perf commands): perf record --call-graph dwarf kill (test-record-graph-dwarf) perf record --call-graph fp kill (test-record-graph-fp) perf record --call-graph fp kill (test-record-graph-fp-aarch64) - perf record -e '{cycles,instructions}' kill (test-record-group1) - perf record -e '{cycles/period=1/,instructions/period=2/}:S' kill (test-record-group2) - perf record -e '{cycles,cache-misses}:S' kill (test-record-group-sampling1) - perf record -c 10000 -e '{cycles,cache-misses}:S' kill (test-record-group-sampling2) + perf record -e '{cycles,default_core/legacy-hardware-config=1/}' kill (test-record-group1) + perf record -e '{cycles/period=1234000/,default_core/legacy-hardware-config=1,period=6789000/}:S' kill (test-record-group2) + perf record -e '{cycles,default_core/legacy-hardware-config=3/}:S' kill (test-record-group-sampling1) + perf record -c 10000 -e '{cycles,default_core/legacy-hardware-config=3/}:S' kill (test-record-group-sampling2) perf record -D kill (test-record-no-delay) perf record -i kill (test-record-no-inherit) perf record -n kill (test-record-no-samples)
@@ -67,5 +67,5 @@ Following tests are defined (with perf commands): perf stat -d kill (test-stat-detailed-1) perf stat -dd kill (test-stat-detailed-2) perf stat -ddd kill (test-stat-detailed-3) - perf stat -e '{cycles,instructions}' kill (test-stat-group1) + perf stat -e '{cycles,default_core/legacy-hardware-config=1/}' kill (test-stat-group1) perf stat -i -e cycles kill (test-stat-no-inherit)
tools/perf/tests/shell/attr/test-record-group-sampling +2 -2
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 +args = --no-bpf-event -e '{cycles,default_core/legacy-hardware-config=3/}:S' kill >/dev/null 2>&1 ret = 1 kernel_until = 6.12 requires = core-pmu
@@ -16,7 +16,7 @@ inherit=0 fd=2 group_fd=1 -# cache-misses +# cache-misses, explicitly requested with the legacy hardware encoding type=0 config=3
tools/perf/tests/shell/attr/test-record-group-sampling1 +2 -2
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 +args = --no-bpf-event -e '{cycles,default_core/legacy-hardware-config=3/}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 requires = core-pmu
@@ -31,7 +31,7 @@ inherit=1 fd=2 group_fd=1 -# cache-misses +# cache-misses, explicitly requested with the legacy hardware encoding type=0 config=3
tools/perf/tests/shell/attr/test-record-group-sampling2 +2 -2
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -c 10000 -e '{cycles,cache-misses}:S' kill >/dev/null 2>&1 +args = --no-bpf-event -c 10000 -e '{cycles,default_core/legacy-hardware-config=3/}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 requires = core-pmu
@@ -37,7 +37,7 @@ write_backward=0 fd=2 group_fd=1 -# cache-misses +# cache-misses, explicitly requested with the legacy hardware encoding type=0 config=3
tools/perf/tests/shell/attr/test-record-group1 +1 -1
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -e '{cycles,instructions}' kill >/dev/null 2>&1 +args = --no-bpf-event -e '{cycles,default_core/legacy-hardware-config=1/}' kill >/dev/null 2>&1 ret = 1 requires = core-pmu
tools/perf/tests/shell/attr/test-record-group2 +1 -1
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -e '{cycles/period=1234000/,instructions/period=6789000/}:S' kill >/dev/null 2>&1 +args = --no-bpf-event -e '{cycles/period=1234000/,default_core/legacy-hardware-config=1,period=6789000/}:S' kill >/dev/null 2>&1 ret = 1 kernel_until = 6.12 requires = core-pmu
tools/perf/tests/shell/attr/test-record-group3 +1 -1
@@ -1,6 +1,6 @@ [config] command = record -args = --no-bpf-event -e '{cycles/period=1234000/,instructions/period=6789000/}:S' kill >/dev/null 2>&1 +args = --no-bpf-event -e '{cycles/period=1234000/,default_core/legacy-hardware-config=1,period=6789000/}:S' kill >/dev/null 2>&1 ret = 1 kernel_since = 6.12 requires = core-pmu
tools/perf/tests/shell/attr/test-stat-group1 +1 -1
@@ -1,6 +1,6 @@ [config] command = stat -args = -e '{cycles,instructions}' kill >/dev/null 2>&1 +args = -e '{cycles,default_core/legacy-hardware-config=1/}' kill >/dev/null 2>&1 ret = 1 requires = core-pmu
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 04/21] perf test attr: Fix legacy event encodings in group tests
Sashiko has reviewed this patch and found no issues.
Quoted 18 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Since commit
b1c5efbfd92e("perf parse-events: Remove hard coded legacy hardware and cache parsing"), event names are resolved with sysfs/JSON priority over the legacy encodings, so bare cache-misses and instructions now become the core PMU sysfs/JSON events (on x86 type 4, configs0x412eand0xc0) rather than the legacy PERF_TYPE_HARDWARE/config=3 and config=1 events the tests expect, e.g.:expected config=3, got 0
expected disabled=0, got 1
expected mmap=0, got 1
expected comm=0, got 1
expected freq=0, got 1
expected enable_on_exec=0, got 1
[ ... ]
return value.
Quoted 3 lines
Fixes b1c5efbfd92eb84d ("perf parse-events: Remove hard coded legacy hardware and cache parsing")Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Arnaldo Carvalho de Melo
maintainerPATCH v4 5/21 perf test attr: Fix default stat metrics expectations
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Since commit c7adeb0974f1 ("perf jevents: Add set of common metrics based on default ones") and commit 2dfc0cab3da4 ("perf stat: Add detail -d,-dd,-ddd metrics") the perf stat default metrics open their events in groups, so group leaders and members have read_format=15 and members are enabled without enable_on_exec, while standalone events keep read_format=3 and are disabled until exec.
Also, since commit b1c5efbfd92e ("perf parse-events: Remove hard coded legacy hardware and cache parsing") bare hardware events resolve to the core PMU sysfs/JSON events (type 4), so add the raw encodings the Intel and AMD core PMUs use for instructions, branch-misses, cpu-cycles and stalled-cycles-frontend/backend to test-stat-default and test-stat-detailed-1/2/3.
Add base-stat-group, that accepts both the standalone and the group variants of read_format, disabled and enable_on_exec, and point the four default/detailed expectations at it. test-stat-basic, test-stat-C0 and test-stat-no-inherit keep base-stat, whose events aren't opened in a
group: for those, a change that groups or enables them differently is still something to catch. test-stat-group1 does open its events in a group, but pins read_format, disabled and enable_on_exec itself, the very terms base-stat-group loosens, so it doesn't need it either.
--- tools/perf/tests/shell/attr/base-stat-group | 45 +++++++ tools/perf/tests/shell/attr/test-stat-default | 90 +++++++++----- .../tests/shell/attr/test-stat-detailed-1 | 98 +++++++++------ .../tests/shell/attr/test-stat-detailed-2 | 110 ++++++++++------- .../tests/shell/attr/test-stat-detailed-3 | 114 +++++++++++------- 5 files changed, 307 insertions(+), 150 deletions(-) create mode 100644 tools/perf/tests/shell/attr/base-stat-group
tools/perf/tests/shell/attr/base-stat-group new file +45 -0
@@ -0,0 +1,45 @@ +[event] +fd=1 +group_fd=-1 +# 0 or PERF_FLAG_FD_CLOEXEC flag +flags=0|8 +cpu=* +type=0 +size=176 +config=0 +sample_period=0 +sample_type=65536 +# The default and -d/-dd/-ddd metrics open part of their events in groups: +# the members get read_format=15 and are enabled right away, the standalone +# ones keep read_format=3 and wait for the exec, and the leaders get 15 when +# their group has members, 3 when alone, as create_perf_stat_counter() goes. +read_format=3|15 +disabled=0|1 +inherit=1 +pinned=0 +exclusive=0 +exclude_user=0 +exclude_kernel=0|1 +exclude_hv=0|1 +exclude_idle=0 +mmap=0 +comm=0 +freq=0 +inherit_stat=0 +enable_on_exec=0|1 +task=0 +watermark=0 +precise_ip=0 +mmap_data=0 +sample_id_all=0 +exclude_host=0|1 +exclude_guest=0|1 +exclude_callchain_kernel=0 +exclude_callchain_user=0 +wakeup_events=0 +bp_type=0 +config1=0 +config2=0 +branch_sample_type=0 +sample_regs_user=0 +sample_stack_user=0
tools/perf/tests/shell/attr/test-stat-default +59 -31
259 lines not on this page · raw message
tools/perf/tests/shell/attr/test-stat-detailed-1 +63 -35
292 lines not on this page · raw message
tools/perf/tests/shell/attr/test-stat-detailed-2 +69 -41
340 lines not on this page · raw message
tools/perf/tests/shell/attr/test-stat-detailed-3 +71 -43
356 lines not on this page · raw message
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 05/21] perf test attr: Fix default stat metrics expectations
Sashiko has reviewed this patch and found no issues.
Quoted 25 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Since commit
c7adeb0974f1("perf jevents: Add set of common metrics based on default ones") and commit2dfc0cab3da4("perf stat: Add detail -d,-dd,-ddd metrics") the perf stat default metrics open their events in groups, so group leaders and members have read_format=15 and members are enabled withoutenable_on_exec, while standalone events keep read_format=3 and are disabled until exec.Also, since commit
b1c5efbfd92e("perf parse-events: Remove hard coded legacy hardware and cache parsing") bare hardware events resolve to the core PMU sysfs/JSON events (type 4), so add the raw encodings the Intel and AMD core PMUs use for instructions, branch-misses, cpu-cycles and stalled-cycles-frontend/backend to test-stat-default and test-stat-detailed-1/2/3.Add base-stat-group, that accepts both the standalone and the group variants of
read_format, disabled andenable_on_exec, and point the four default/detailed expectations at it. test-stat-basic, test-stat-C0 and test-stat-no-inherit keep base-stat, whose events aren't opened in a
group: for those, a change that groups or enables them differently is still something to catch. test-stat-group1 does open its events in a group, but pinsread_format, disabled andenable_on_execitself, the very terms base-stat-group loosens, so it doesn't need it either.
Quoted 5 lines
Fixes b1c5efbfd92eb84d ("perf parse-events: Remove hard coded legacy hardware and cache parsing")Fixes c7adeb0974f18da4 ("perf jevents: Add set of common metrics based on default ones")Fixes 2dfc0cab3da4a205 ("perf stat: Add detail -d,-dd,-ddd metrics")Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Arnaldo Carvalho de Melo
maintainerPATCH v4 6/21 perf test attr: Relax group checking for ungrouped expectations
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
When matching the recorded events back against the expectations, attr.py requires every event opened in a group to match an expectation that declares the same group. The perf stat default metrics open the events of each metric in groups, but the test expectations only verify the event attributes, so test-stat-default fails with:
FAILED '.../test-stat-default' - group failure
Only enforce the group of a recorded event when the expectation it matches declares one. Expectations that ask for a group are still verified in both directions.
--- tools/perf/tests/shell/lib/attr.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
tools/perf/tests/shell/lib/attr.py +7 -4
@@ -372,7 +372,7 @@ class Test(object): else: raise Unsup(self) - def compare(self, expect, result): + def compare(self, expect, result, strict_groups=True): match = {} log.debug(" compare")
@@ -406,8 +406,9 @@ class Test(object): match[exp_name] = exp_list - # For each defined group in the expected events - # check we match the same group in the result. + # Check that each expected group is matched in the result; an event + # opened in a group may match an expectation that doesn't declare + # one, as those only verify the event attributes. for exp_name, exp_event in expect.items(): group = exp_event.group
@@ -416,6 +417,8 @@ class Test(object): for res_name in match[exp_name]: res_group = result[res_name].group + if res_group == '' and not strict_groups: + continue if res_group not in match[group]: raise Fail(self, 'group failure')
@@ -454,7 +457,7 @@ class Test(object): # do the expectation - results matching - both ways self.compare(self.expect, self.result) - self.compare(self.result, self.expect) + self.compare(self.result, self.expect, strict_groups=False) finally: # cleanup
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 06/21] perf test attr: Relax group checking for ungrouped expectations
Sashiko has reviewed this patch and found no issues.
Quoted 10 lines
From: Arnaldo Carvalho de Melo <[email protected]>
When matching the recorded events back against the expectations,
attr.pyrequires every event opened in a group to match an expectation that declares the same group. The perf stat default metrics open the events of each metric in groups, but the test expectations only verify the event attributes, so test-stat-default fails with:FAILED '.../test-stat-default' - group failure
Quoted 8 lines
Only enforce the group of a recorded event when the expectation it matches declares one. Expectations that ask for a group are still verified in both directions.
Fixes c7adeb0974f18da4 ("perf jevents: Add set of common metrics based on default ones")Fixes 2dfc0cab3da4a205 ("perf stat: Add detail -d,-dd,-ddd metrics")Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
Arnaldo Carvalho de Melo
maintainerPATCH v4 7/21 perf evsel: Mark the event as supported when retrying an open
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Opening a group with perf record on a machine with perf_event_paranoid > 1 and no CAP_PERFMON fails the first perf_event_open() with EACCES, after which evsel__open_cpu() clears evsel->supported; evsel__fallback() then adds exclude_kernel and record__open() retries, and the retry succeeds with supported left false. get_group_fd() treats unsupported events as ungroupable, so the retry opens the group members without their leader:
sys_perf_event_open: pid 12345 cpu 0 group_fd -1 FAILED '.../test-record-group-sampling1' - group failure
which fails the group subtests of "perf test 82", the Perf attribute expectations test, when it is run as a regular user, while it passes as root or with CAP_PERFMON, where the first open doesn't fail.
commit 2cc7aa995ce9 ("perf stat: Refactor retry/skip/fatal error handling") dropped the pos->supported = true that record__open() did after a successful open and moved the resets into the stat code only, while saying "When an event is tried the supported is set to true again". Do that in evsel__open_cpu(), which every open entry point funnels through, so that a retry describes its own try instead of the previous failure.
--- tools/perf/util/evsel.c | 6 ++++++ 1 file changed, 6 insertions(+)
tools/perf/util/evsel.c +6 -0
@@ -3014,6 +3014,12 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,enum rlimit_action set_rlimit =NO_CHANGE ;struct perf_cpu cpu; +/* +* A failed attempt being retried after evsel__fallback() would +* otherwise keep group members out of their group in get_group_fd(). +*/ + evsel->supported = true; +if (evsel__is_retire_lat (evsel)) { err =evsel__tpebs_open (evsel); goto out;
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 07/21] perf evsel: Mark the event as supported when retrying an open
Sashiko has reviewed this patch and found no issues.
Quoted 23 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Opening a group with perf record on a machine with
perf_event_paranoid> 1 and noCAP_PERFMONfails the firstperf_event_open()with EACCES, after whichevsel__open_cpu()clearsevsel->supported;evsel__fallback()then addsexclude_kernelandrecord__open()retries, and the retry succeeds with supported left false.get_group_fd()treats unsupported events as ungroupable, so the retry opens the group members without their leader:
sys_perf_event_open: pid 12345 cpu 0group_fd-1 FAILED '.../test-record-group-sampling1' - group failurewhich fails the group subtests of "perf test 82", the Perf attribute expectations test, when it is run as a regular user, while it passes as root or with
CAP_PERFMON, where the first open doesn't fail.commit
2cc7aa995ce9("perf stat: Refactor retry/skip/fatal error handling") dropped thepos->supported= true thatrecord__open()did after a successful open and moved the resets into the stat code only, while saying "When an event is tried the supported is set to true again". Do that inevsel__open_cpu(), which every open entry point funnels through, so that a retry describes its own try instead of the previous failure.
Quoted 4 lines
Fixes 2cc7aa995ce9ecd8 ("perf stat: Refactor retry/skip/fatal error handling")Cc Ian Rogers <[email protected]>Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
Arnaldo Carvalho de Melo
maintainerPATCH v4 8/21 perf test attr: Ignore the core PMU type perf puts in 'config'
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Since
commit 55bcf6ef314a ("perf: Extend PERF_TYPE_HARDWARE and PERF_TYPE_HW_CACHE") in v5.13, and arm_pmu opted in with
commit 5c816728651a ("arm_pmu: Add PERF_PMU_CAP_EXTENDED_HW_TYPE capability") in v6.6, the kernel can
be told which core PMU a legacy type event is meant for, with its type in the top 32 bits of 'config', see perf_pmus__supports_extended_type(), which __parse_events_add_numeric() does for HARDWARE and HW_CACHE events, but only when there is more than one core PMU, i.e. on heterogeneous machines, where asking for one event ends up being one event per core PMU:
$ perf stat -e cycles -a -- sleep 1
20,520,998 armv8_cortex_a53/cycles/
13,029,350 armv8_cortex_a73/cycles/and what lands in PERF_TEST_ATTR is the extended encoding:
type=0
config=38654705664 # core PMU type 9, PERF_COUNT_HW_CPU_CYCLES
so the expectations written for the single core PMU case fail with:
expected config=0|1, got 38654705664
expected config=0, got 38654705664 # 'perf stat -d' L1-dcache events
which is what now happens on an Amlogic A311D (2 Cortex-A53 + 4 Cortex-A73), a machine that just got the CPU PMU nodes it was missing.
Take those bits out when comparing the 'config' of legacy type events, keeping what these expectations mean on machines with a single core PMU:
which of the PMUs is counting is not what we're asking here.
--- tools/perf/tests/shell/lib/attr.py | 53 ++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-)
tools/perf/tests/shell/lib/attr.py +51 -2
@@ -73,6 +73,32 @@ def core_pmu_probe(): return True +PERF_TYPE_HARDWARE = 0 +PERF_TYPE_HW_CACHE = 3 + +# The legacy event types, where perf may qualify 'config' with the core PMU +# type in its top 32 bits when there is more than one core PMU. +LEGACY_TYPES = (str(PERF_TYPE_HARDWARE), str(PERF_TYPE_HW_CACHE)) + +def hide_core_pmu_type(value): + """ + The kernel can be told which core PMU a legacy type event is meant for by + putting its type in the top 32 bits of 'config', which perf only does when + there is more than one core PMU, so take those bits out to keep the + expectations written for the single core PMU case matching on + heterogeneous machines. + """ + hidden = [] + + for alternative in value.split('|'): + try: + alternative = str(int(alternative, 0) & 0xffffffff) + except ValueError: + pass + hidden.append(alternative) + + return '|'.join(hidden) + class Fail(Exception): def __init__(self, test, msg): self.msg = msg
@@ -146,12 +172,33 @@ class Event(dict): self.add(base) self.add(data) + def hide_core_pmu_type_terms(self, other): + """ + Is this a legacy type event? Then the core PMU type that perf may have + put in the top 32 bits of 'config' to tell the kernel which core PMU it + wants isn't part of what we asked for, see hide_core_pmu_type(). + """ + for event in (self, other): + for type in event.get('type', '').split('|'): + if type in LEGACY_TYPES: + return True + + return False + + def hide_core_pmu_type(self, term, value, other): + if term == 'config' and self.hide_core_pmu_type_terms(other): + return hide_core_pmu_type(value) + + return value + def equal(self, other): for t in Event.terms: log.debug(" [%s] %s %s", t, self[t], other[t]) if t not in self or t not in other: return False - if not data_equal(self[t], other[t]): + exp = self.hide_core_pmu_type(t, self[t], other) + got = self.hide_core_pmu_type(t, other[t], other) + if not data_equal(exp, got): return False return True
@@ -164,7 +211,9 @@ class Event(dict): for t in Event.terms: if t not in self or t not in other: continue - if not data_equal(self[t], other[t]): + exp = self.hide_core_pmu_type(t, self[t], other) + got = self.hide_core_pmu_type(t, other[t], other) + if not data_equal(exp, got): log.warning("expected %s=%s, got %s", t, self[t], other[t]) def parse_version(version):
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 08/21] perf test attr: Ignore the core PMU type perf puts in 'config'
Sashiko has reviewed this patch and found no issues.
Quoted 14 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Since
commit55bcf6ef314a("perf: ExtendPERF_TYPE_HARDWAREandPERF_TYPE_HW_CACHE") in v5.13, andarm_pmuopted in with
commit5c816728651a("arm_pmu: AddPERF_PMU_CAP_EXTENDED_HW_TYPEcapability") in v6.6, the kernel can
be told which core PMU a legacy type event is meant for, with its type in the top 32 bits of 'config', seeperf_pmus__supports_extended_type(), which__parse_events_add_numeric()does for HARDWARE andHW_CACHEevents, but only when there is more than one core PMU, i.e. on heterogeneous machines, where asking for one event ends up being one event per core PMU:
Quoted 7 lines
$ perf stat -e cycles -a -- sleep 1 [ ... ] keeping what these expectations mean on machines with a single core PMU: which of the PMUs is counting is not what we're asking here.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
Arnaldo Carvalho de Melo
maintainerPATCH v4 9/21 perf evsel: Don't dump probe events to PERF_TEST_ATTR
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Events opened just to ask if they are supported, i.e. the ones in is_event_supported() and is_libpfm_event_supported(), go through the regular evsel__open() path and thus end up in the event files written to $PERF_TEST_ATTR, with everything but type, config and disabled zeroed:
[event-0-38654705664-3]
fd=3
cpu=-1
pid=0
type=0
size=0
config=38654705664
sample_type=0
...
which 'perf test attr' then takes as one of the events that were asked for, failing with:
expected cpu=0, got -1
expected size=176, got 0
expected sample_type=65927, got 0
expected freq=1, got 0
This started showing up when there is more than one core PMU, as perf_pmus__supports_extended_type() probes one such event per core PMU, but these records were always bogus: nothing asked for them.
Flag those evsels as support probes, keeping the name clear of the kprobe/uprobe meaning probe already has in evsel, and leave them out of the dump.
--- tools/perf/util/evsel.c | 6 +++++- tools/perf/util/evsel.h | 1 + tools/perf/util/pfm.c | 1 + tools/perf/util/print-events.c | 1 + 4 files changed, 8 insertions(+), 1 deletion(-)
tools/perf/util/evsel.c +5 -1
@@ -3104,7 +3104,11 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,bpf_counter__install_pe (evsel, idx, fd); -if (unlikely (test_attr__enabled ())) { +/* +* Support probes aren't what was asked for, leave them +* out of the PERF_TEST_ATTR dump. +*/ +if (unlikely (test_attr__enabled () && !evsel->support_probe)) {test_attr__open (&evsel->core.attr, pid, cpu, fd, group_fd, evsel->open_flags); }
tools/perf/util/evsel.h +1 -0
@@ -120,6 +120,7 @@ struct evsel {u64 db_id;bool uniquified_name;bool supported; +bool support_probe;/* Opened just to check for support. */ bool needs_swap;bool disabled;bool no_aux_samples;
tools/perf/util/pfm.c +1 -0
@@ -141,6 +141,7 @@ static bool is_libpfm_event_supported(const char *name, struct perf_cpu_map *cpureturn false; evsel->is_libpfm_event = true; + evsel->support_probe = true; ret =evsel__open (evsel, cpus, threads);if (ret == -EACCES ) {
tools/perf/util/print-events.c +1 -0
@@ -147,6 +147,7 @@ bool is_event_supported(u8 type, u64 config) evsel =evsel__new (&attr);if (evsel) { + evsel->support_probe = true; ret =evsel__open (evsel,NULL , tmap) >=0 ;if (!ret) {
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 09/21] perf evsel: Don't dump probe events to PERF_TEST_ATTR
Sashiko has reviewed this patch and found no issues.
Quoted 17 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Events opened just to ask if they are supported, i.e. the ones in
is_event_supported()andis_libpfm_event_supported(), go through the regularevsel__open()path and thus end up in the event files written to $PERF_TEST_ATTR, with everything but type, config and disabled zeroed:[event-0-38654705664-3]
fd=3
cpu=-1
pid=0
type=0
size=0
config=38654705664
sample_type=0
[ ... ]
Quoted 4 lines
Reported-by Namhyung Kim <[email protected]>Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
Arnaldo Carvalho de Melo
maintainerPATCH v4 10/21 perf evsel: Close the fds opened so far when disabling missing features
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
evsel__open_cpu() opens the events one by one and, when an open fails with EINVAL and evsel__detect_missing_features() has a feature to leave out, goes back to fallback_missing_features to open them all again from start_cpu_map_idx, without closing what the abandoned attempt opened:
the fd of every CPU or thread that was opened before the failing one is overwritten in the FD() array, leaking it.
Close them first, the same way out_close does when returning an error, leaving the entries at -1 for the retry.
--- tools/perf/util/evsel.c | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-)
tools/perf/util/evsel.c +21 -9
@@ -3005,6 +3005,19 @@ static bool evsel__detect_missing_features(struct evsel *evsel, struct perf_cpureturn false; } +static void evsel__close_fds (struct evsel *evsel,int start_cpu_map_idx, +int cpu_map_idx,int thread,int nthreads) +{ +do { +while (--thread >=0 ) { +if (FD (evsel, cpu_map_idx, thread) >=0 ) +close (FD (evsel, cpu_map_idx, thread)); +FD (evsel, cpu_map_idx, thread) =-1 ; + } + thread = nthreads; + }while (--cpu_map_idx >= start_cpu_map_idx); +} +static int evsel__open_cpu (struct evsel *evsel,struct perf_cpu_map *cpus,struct perf_thread_map *threads,int start_cpu_map_idx,int end_cpu_map_idx)
@@ -3150,8 +3163,14 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus,if (err == -EMFILE &&rlimit__increase_nofile (&set_rlimit)) goto retry_open; -if (err == -EINVAL &&evsel__detect_missing_features (evsel, cpu)) +if (err == -EINVAL &&evsel__detect_missing_features (evsel, cpu)) { +/* +* The restart opens the events again, so close what this +* attempt opened before its entries are overwritten. +*/ +evsel__close_fds (evsel, start_cpu_map_idx, idx, thread, nthreads); goto fallback_missing_features; + }if (evsel__precise_ip_fallback (evsel)) goto retry_open;
@@ -3161,14 +3180,7 @@ static int evsel__open_cpu(struct evsel *evsel, struct perf_cpu_map *cpus, threads->err_thread = thread; old_errno = errno; -do { -while (--thread >=0 ) { -if (FD (evsel, idx, thread) >=0 ) -close (FD (evsel, idx, thread)); -FD (evsel, idx, thread) =-1 ; - } - thread = nthreads; - }while (--idx >= start_cpu_map_idx); +evsel__close_fds (evsel, start_cpu_map_idx, idx, thread, nthreads); errno = old_errno; out:if (err)
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 10/21] perf evsel: Close the fds opened so far when disabling missing features
Sashiko has reviewed this patch and found no issues.
Quoted 12 lines
From: Arnaldo Carvalho de Melo <[email protected]>
evsel__open_cpu()opens the events one by one and, when an open fails with EINVAL andevsel__detect_missing_features()has a feature to leave out, goes back tofallback_missing_featuresto open them all again fromstart_cpu_map_idx, without closing what the abandoned attempt opened:
the fd of every CPU or thread that was opened before the failing one is overwritten in theFD()array, leaking it.Close them first, the same way
out_closedoes when returning an error, leaving the entries at -1 for the retry.
Quoted 6 lines
Fixes af954f76eea5 ("perf tools: Check fallback error and order")Reported-by Sashiko <[email protected]>Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10
Arnaldo Carvalho de Melo
maintainerPATCH v4 11/21 perf test attr: Introduce mask_bits(), generalizing hide_core_pmu_type()
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
What hide_core_pmu_type() does is take a set of bits out of each of the '|'-separated alternatives in a value, wildcards included, keeping the rest of the expression: extract that as mask_bits() and implement hide_core_pmu_type() in terms of it, a prep for using the same thing to ignore, when comparing expectations, the sample id bit perf ends up asking for and the read_format group bit that a PMU count can remove.
No change in behaviour.
--- tools/perf/tests/shell/lib/attr.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-)
tools/perf/tests/shell/lib/attr.py +17 -10
@@ -80,24 +80,31 @@ PERF_TYPE_HW_CACHE = 3 # type in its top 32 bits when there is more than one core PMU. LEGACY_TYPES = (str(PERF_TYPE_HARDWARE), str(PERF_TYPE_HW_CACHE)) -def hide_core_pmu_type(value): +def mask_bits(value, bits): """ - The kernel can be told which core PMU a legacy type event is meant for by - putting its type in the top 32 bits of 'config', which perf only does when - there is more than one core PMU, so take those bits out to keep the - expectations written for the single core PMU case matching on - heterogeneous machines. + Take the bits in 'bits' out of each of the '|'-separated alternatives in + 'value', keeping the rest of the expression, wildcards included. """ - hidden = [] + masked = [] for alternative in value.split('|'): try: - alternative = str(int(alternative, 0) & 0xffffffff) + alternative = str(int(alternative, 0) & ~bits) except ValueError: pass - hidden.append(alternative) + masked.append(alternative) - return '|'.join(hidden) + return '|'.join(masked) + +def hide_core_pmu_type(value): + """ + The kernel can be told which core PMU a legacy type event is meant for by + putting its type in the top 32 bits of 'config', which perf only does when + there is more than one core PMU, so take those bits out to keep the + expectations written for the single core PMU case matching on + heterogeneous machines. + """ + return mask_bits(value, 0xffffffff00000000) class Fail(Exception): def __init__(self, test, msg):
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 11/21] perf test attr: Introduce mask_bits(), generalizing hide_core_pmu_type()
Sashiko has reviewed this patch and found no issues.
Quoted 9 lines
From: Arnaldo Carvalho de Melo <[email protected]>
What
hide_core_pmu_type()does is take a set of bits out of each of the '|'-separated alternatives in a value, wildcards included, keeping the rest of the expression: extract that asmask_bits()and implementhide_core_pmu_type()in terms of it, a prep for using the same thing to ignore, when comparing expectations, the sample id bit perf ends up asking for and theread_formatgroup bit that a PMU count can remove.
Quoted 4 lines
No change in behaviour.
Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11
Arnaldo Carvalho de Melo
maintainerPATCH v4 12/21 perf test attr: Accept either sample id bit perf asks for
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
evlist__config() asks for a way to tell the samples apart, PERF_SAMPLE_IDENTIFIER where available, PERF_SAMPLE_ID otherwise:
bool sample_id = opts->sample_id;
...
if (opts->full_auxtrace || opts->sample_identifier) {
...
use_sample_identifier = perf_can_sample_identifier();
sample_id = true;
} else if (evlist__nr_entries(evlist) > 1) {
...
use_sample_identifier = perf_can_sample_identifier();
sample_id = true;
}so the id is asked for whenever the evlist has more than one event, and also when asked to outright, with --sample-identifier or by an auxtrace session, or by record__config() for a pipe run, through opts->sample_id, none of which depends on the sample types: 'perf record --sample-identifier -e cycles' and the arm SPE expectations in this very directory ask for it with a single event.
Which one it asks for depends on the composition of the evlist: with more than one core PMU a request for 'cycles' becomes one event per core PMU, the extra dummy tracking events then make the sample types differ and perf goes for IDENTIFIER, while the expectations were written on machines with a single core PMU, where perf asks for ID:
expected sample_type=263, got 65799 # 263 | (1 << 16)
expected sample_type=343, got 65815 # (343 & ~(1 << 6)) | (1 << 16), group-sampling
Both bits mean the same thing here, how the samples get associated with the event that produced them, so when the expectation asks for one of them, ask for "an id of some sort" on both sides, and when the expectation doesn't ask for any, take the bits out of what got recorded: whether they are there and which one it is are consequences of what was asked for and of how the request got expanded, none of that is what these expectations are about.
Since Test.compare() looks at both directions, expectations matching the recorded events and recorded events matching the expectations, keep track of which side is the one perf recorded to take the bits out of the right side.
The masking is only for the comparison: the warnings keep showing the values as they are, nothing to un-mask by hand to see what is being complained about.
--- tools/perf/tests/shell/lib/attr.py | 88 ++++++++++++++++++++++++++++-- 1 file changed, 83 insertions(+), 5 deletions(-)
tools/perf/tests/shell/lib/attr.py +83 -5
@@ -75,6 +75,8 @@ def core_pmu_probe(): PERF_TYPE_HARDWARE = 0 PERF_TYPE_HW_CACHE = 3 +PERF_SAMPLE_ID = 1 << 6 +PERF_SAMPLE_IDENTIFIER = 1 << 16 # The legacy event types, where perf may qualify 'config' with the core PMU # type in its top 32 bits when there is more than one core PMU.
@@ -106,6 +108,49 @@ def hide_core_pmu_type(value): """ return mask_bits(value, 0xffffffff00000000) +# The two sample id bits mean the same thing here: how the samples of the +# events in the evlist get associated with the event that produced them. +SAMPLE_ID_BITS = PERF_SAMPLE_ID | PERF_SAMPLE_IDENTIFIER + +# No real sample_type bit is this high, so it can stand for "either of the +# sample id bits" when comparing the two sides. +SAMPLE_ID_EITHER = 1 << 40 + +def hide_sample_id(expected, recorded): + """ + perf asks for a way to tell the samples apart, PERF_SAMPLE_IDENTIFIER where + available, PERF_SAMPLE_ID otherwise, see evlist__config(): whenever the + evlist has more than one event, and when asked to outright, with + --sample-identifier, by an auxtrace session, or by record__config() for a + pipe run, none of which depends on the sample types, as those only choose + between the two bits: + with more than one core PMU the dummy tracking events make the sample types + differ, so IDENTIFIER is asked for even for events whose expectations ask + for ID, written on a machine with a single core PMU. + + So treat the two as the same thing: when the expectation asks for either + of them, ask for "an id of some sort" on both sides, and when the + expectation doesn't ask for any, take the bits out of what got recorded. + """ + try: + asked = int(expected, 0) & SAMPLE_ID_BITS + except ValueError: + # The expectation is a wildcard or a set of alternatives. + return expected, recorded + + try: + recorded_id_bits = int(recorded, 0) & SAMPLE_ID_BITS + except ValueError: + # The recorded side is a wildcard or a set of alternatives. + return expected, mask_bits(recorded, SAMPLE_ID_BITS) + + if not asked: + return expected, mask_bits(recorded, SAMPLE_ID_BITS) + + return (str(int(expected, 0) & ~SAMPLE_ID_BITS | SAMPLE_ID_EITHER), + str(int(recorded, 0) & ~SAMPLE_ID_BITS | + (SAMPLE_ID_EITHER if recorded_id_bits else 0))) + class Fail(Exception): def __init__(self, test, msg): self.msg = msg
@@ -198,13 +243,36 @@ class Event(dict): return value + def hide_event_count_dependent_terms(self, term, expected, recorded): + if term == 'sample_type': + return hide_sample_id(expected, recorded) + + return expected, recorded + + def recorded_side(self, other): + """ + Which one is what perf recorded? Both directions get compared, see + Test.compare(), so this can't be taken from the arguments' order. + """ + if getattr(other, 'recorded', False): + return other + + return self if getattr(self, 'recorded', False) else None + def equal(self, other): for t in Event.terms: log.debug(" [%s] %s %s", t, self[t], other[t]) if t not in self or t not in other: return False - exp = self.hide_core_pmu_type(t, self[t], other) - got = self.hide_core_pmu_type(t, other[t], other) + recorded = self.recorded_side(other) + if recorded is self: + expected, got_event = other, self + else: + expected, got_event = self, other + exp = self.hide_core_pmu_type(t, expected[t], other) + got = self.hide_core_pmu_type(t, got_event[t], other) + if recorded is not None: + exp, got = self.hide_event_count_dependent_terms(t, exp, got) if not data_equal(exp, got): return False return True
@@ -218,10 +286,18 @@ class Event(dict): for t in Event.terms: if t not in self or t not in other: continue - exp = self.hide_core_pmu_type(t, self[t], other) - got = self.hide_core_pmu_type(t, other[t], other) + recorded = self.recorded_side(other) + if recorded is self: + expected, got_event = other, self + else: + expected, got_event = self, other + exp = self.hide_core_pmu_type(t, expected[t], other) + got = self.hide_core_pmu_type(t, got_event[t], other) + if recorded is not None: + exp, got = self.hide_event_count_dependent_terms(t, exp, got) if not data_equal(exp, got): - log.warning("expected %s=%s, got %s", t, self[t], other[t]) + log.warning("expected %s=%s, got %s", t, expected[t], + got_event[t]) def parse_version(version): if not version:
@@ -506,6 +582,8 @@ class Test(object): log.debug(" loading result events") for f in glob.glob(tempdir + '/event*'): self.load_events(f, self.result) + for event in self.result.values(): + event.recorded = True # resolve group_fd to event names self.resolve_groups(self.expect)
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 12/21] perf test attr: Accept either sample id bit perf asks for
Sashiko has reviewed this patch and found no issues.
Quoted 15 lines
From: Arnaldo Carvalho de Melo <[email protected]>
evlist__config()asks for a way to tell the samples apart,PERF_SAMPLE_IDENTIFIERwhere available,PERF_SAMPLE_IDotherwise:bool
sample_id=opts->sample_id;
...if (opts->full_auxtrace || opts->sample_identifier) { ... use_sample_identifier = perf_can_sample_identifier(); sample_id = true; } else if (evlist__nr_entries(evlist) > 1) { ... use_sample_identifier = perf_can_sample_identifier(); sample_id = true;
Quoted 6 lines
[ ... ]
values as they are, nothing to un-mask by hand to see what is being complained about.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12
Arnaldo Carvalho de Melo
maintainerPATCH v4 13/21 perf test attr: Ignore the dummy tracking event and the terms it carries
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
With more than one core PMU the requested events can't cover all the CPUs, each cluster has its own PMU, so perf opens the software dummy event to carry the tracking terms (mmap, comm, task, mmap_data), leaving the requested events without them:
expected mmap=1, got 0
expected comm=1, got 0
expected task=1, got 0
That dummy event is not part of what the expectations are about, so leave it out of the comparison, unless the expectations ask for one, like system-wide-dummy and test-record-dummy-C0 do.
And when perf used it to carry the tracking terms, those terms can't be matched on the requested events, leave them out of the match as well.
The terms are left out only where the dropped dummy actually carried them, from the sample_type it was opened with, a dummy that didn't is no reason to stop verifying them.
--- tools/perf/tests/shell/lib/attr.py | 61 ++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+)
tools/perf/tests/shell/lib/attr.py +61 -0
@@ -77,6 +77,21 @@ PERF_TYPE_HARDWARE = 0 PERF_TYPE_HW_CACHE = 3 PERF_SAMPLE_ID = 1 << 6 PERF_SAMPLE_IDENTIFIER = 1 << 16 +PERF_TYPE_SOFTWARE = 1 +PERF_COUNT_SW_DUMMY = 9 + +def is_dummy_tracking_event(event): + """ + Is this the dummy event perf opens to carry the tracking events (mmap, + comm, task) when the requested ones can't, like when there is more than one + core PMU and each core PMU's events run on only part of the CPUs? + """ + return (event.get('type') == str(PERF_TYPE_SOFTWARE) and + event.get('config') == str(PERF_COUNT_SW_DUMMY)) + +# With more than one core PMU the tracking terms go to the dummy tracking event, +# leaving the requested events without them, so they can't be matched there. +TRACKING_TERMS = ['mmap', 'comm', 'task', 'mmap_data'] # The legacy event types, where perf may qualify 'config' with the core PMU # type in its top 32 bits when there is more than one core PMU.
@@ -259,11 +274,29 @@ class Event(dict): return self if getattr(self, 'recorded', False) else None + def hide_tracking_via_dummy(self, term, other): + """ + The tracking terms are set on the tracking event, which is the dummy + event when there is more than one core PMU, leaving the requested + events without them, nothing to match on then. + """ + if term not in TRACKING_TERMS: + return False + + for event in (self, other): + if term in getattr(event, 'tracking_moved_to_dummy', ()): + return True + + return False + def equal(self, other): for t in Event.terms: log.debug(" [%s] %s %s", t, self[t], other[t]) if t not in self or t not in other: return False + if self.hide_tracking_via_dummy(t, other): + continue + recorded = self.recorded_side(other) if recorded is self: expected, got_event = other, self
@@ -286,6 +319,9 @@ class Event(dict): for t in Event.terms: if t not in self or t not in other: continue + if self.hide_tracking_via_dummy(t, other): + continue + recorded = self.recorded_side(other) if recorded is self: expected, got_event = other, self
@@ -504,6 +540,29 @@ class Test(object): else: raise Unsup(self) + def drop_dummy_tracking_events(self): + """ + Leave out the dummy event perf itself opened, see + is_dummy_tracking_event(), unless the expectations ask for one. + """ + if any(is_dummy_tracking_event(e) for e in self.expect.values()): + return + + if not any(is_dummy_tracking_event(e) for e in self.result.values()): + return + + moved = set() + for name, event in list(self.result.items()): + if is_dummy_tracking_event(event): + log.debug(" dropping dummy tracking event [%s]", name) + moved.update(t for t in TRACKING_TERMS if event.get(t) == '1') + del self.result[name] + + # The tracking terms the dropped dummies carried, flag the remaining + # events so that the match leaves only those out. + for event in self.result.values(): + event.tracking_moved_to_dummy = moved + def compare(self, expect, result, strict_groups=True): match = {}
@@ -585,6 +644,8 @@ class Test(object): for event in self.result.values(): event.recorded = True + self.drop_dummy_tracking_events() + # resolve group_fd to event names self.resolve_groups(self.expect) self.resolve_groups(self.result)
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 13/21] perf test attr: Ignore the dummy tracking event and the terms it carries
Sashiko has reviewed this patch and found no issues.
Quoted 18 lines
From: Arnaldo Carvalho de Melo <[email protected]>
With more than one core PMU the requested events can't cover all the CPUs, each cluster has its own PMU, so perf opens the software dummy event to carry the tracking terms (mmap, comm, task,
mmap_data), leaving the requested events without them:expected mmap=1, got 0
expected comm=1, got 0
expected task=1, got 0That dummy event is not part of what the expectations are about, so leave it out of the comparison, unless the expectations ask for one, like system-wide-dummy and test-record-dummy-C0 do.
And when perf used it to carry the tracking terms, those terms can't be matched on the requested events, leave them out of the match as well.
Quoted 6 lines
The terms are left out only where the dropped dummy actually carried them, from the
sample_typeit was opened with, a dummy that didn't is no reason to stop verifying them.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13
Arnaldo Carvalho de Melo
maintainerPATCH v4 14/21 perf test attr: Leave out the events of the other core PMUs
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
On a hybrid machine the default stat metrics are opened on each core PMU, and the events of the other one have their own encodings: on an i7-14700K cpu_atom (type 10) opens UOPS_RETIRED.ALL, config 194, which the expectations, written for cpu_core (type 4), don't name:
expected type=4, got 10
expected config=209|391, got 194
The test already masks the core PMU type out of 'config' for the legacy events, but raw events keep their type and encoding, so leave out the events of the core PMUs no expectation names, like the dummy tracking event. Core PMUs are found as perf finds them: 'cpu' or 'cpum_cf', or a PMU with a 'cpus' file in sysfs. Nothing is left out unless an expectation names a core PMU, so single core PMU machines and the tests with only legacy expectations are not affected.
--- tools/perf/tests/shell/lib/attr.py | 39 ++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
tools/perf/tests/shell/lib/attr.py +39 -0
@@ -563,6 +563,44 @@ class Test(object): for event in self.result.values(): event.tracking_moved_to_dummy = moved + def drop_other_core_pmu_events(self, devices='/sys/bus/event_source/devices'): + """ + Leave out the events of the other core PMUs, which have their own + encodings, when the expectations name one of the core PMUs. + """ + asked = set() + for event in self.expect.values(): + types = event.get('type', '').split('|') + if '*' in types: + return + asked.update(types) + + try: + names = os.listdir(devices) + except OSError: + return + + core = set() + for name in names: + path = os.path.join(devices, name) + has_cpus = os.path.exists(os.path.join(path, 'cpus')) + if name not in ('cpu', 'cpum_cf') and not has_cpus: + continue + try: + with open(os.path.join(path, 'type'), encoding='utf-8') as f: + core.add(f.read().strip()) + except OSError: + pass + + if not core & asked: + return + + others = core - asked + for name, event in list(self.result.items()): + if event.get('type') in others: + log.debug(" dropping other core PMU event [%s]", name) + del self.result[name] + def compare(self, expect, result, strict_groups=True): match = {}
@@ -645,6 +683,7 @@ class Test(object): event.recorded = True self.drop_dummy_tracking_events() + self.drop_other_core_pmu_events() # resolve group_fd to event names self.resolve_groups(self.expect)
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 14/21] perf test attr: Leave out the events of the other core PMUs
Sashiko has reviewed this patch and found no issues.
Quoted 18 lines
From: Arnaldo Carvalho de Melo <[email protected]>
On a hybrid machine the default stat metrics are opened on each core PMU, and the events of the other one have their own encodings: on an i7-14700K
cpu_atom(type 10) opens UOPS_RETIRED.ALL, config 194, which the expectations, written forcpu_core(type 4), don't name:expected type=4, got 10
expected config=209|391, got 194The test already masks the core PMU type out of 'config' for the legacy events, but raw events keep their type and encoding, so leave out the events of the core PMUs no expectation names, like the dummy tracking event. Core PMUs are found as perf finds them: 'cpu' or '
cpum_cf', or a PMU with a 'cpus' file in sysfs. Nothing is left out unless an expectation names a core PMU, so single core PMU machines and the tests with only legacy expectations are not affected.
Quoted 2 lines
Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
Arnaldo Carvalho de Melo
maintainerPATCH v4 15/21 perf test attr: Ignore the group read_format bit the PMU count can remove
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
read_format's PERF_FORMAT_GROUP bit asks for the group members to be read along with the leader, which aside from this bit is verified with the fd/group_fd based group checking in Test.compare(), and with more than one core PMU a group whose members are pinned to one of them, say 'default_core', leaves the clones for the other core PMUs without any members:
$ perf stat -e '{cycles,default_core/legacy-hardware-config=3/}:S'
... the A53 leader keeps its cohort, both A73 cycles clones are alone,
read_format 20 instead of 28so that bit isn't there: whether it is there is a consequence of how the evlist got expanded to more than one core PMU, not of the groupness the expectations say about, which is verified structurally, so take it out on both sides.
That leaves the A73 leader clones, now with:
expected read_format=28|31, got 20
matching the base event expectations again.
With a single core PMU there is nothing to expand, so there is nothing to explain away: count the core PMUs when the script starts, from what perf makes of 'cycles', see parse_events_multi_pmu_add(), and take the bit out only when there is more than one, leaving the machines with a single core PMU verifying what the expectations say about it. That count is 2 on the Amlogic A311D, where its A53 and A73 PMUs both answer for 'cycles', and 1 on a machine with a single core PMU. Where there is no binding to ask, as when attr.py runs without setup_python.sh, the count isn't available and the bit is taken out as well.
core_pmu_count() takes the count from the same probe core_pmu_available() uses, so the binding is asked once, and only when a comparison needs the answer, not at import time.
The sample id bits hide_sample_id() takes out when the expectations ask for none are another consequence of having more than one core PMU, so the same single core PMU gate covers them; equating the two id bits, needed when the evlist has more than one event even with a single core PMU, keeps applying.
--- tools/perf/tests/shell/lib/attr.py | 75 +++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 12 deletions(-)
tools/perf/tests/shell/lib/attr.py +63 -12
@@ -30,6 +30,7 @@ def data_equal(a, b): return False CORE_PMU_AVAILABLE = None +CORE_PMU_COUNT = None def core_pmu_available(): """
@@ -38,40 +39,47 @@ def core_pmu_available(): check the expectations that pin their legacy encodings. The answer can't change while the tests run, so probe the binding only once. """ - global CORE_PMU_AVAILABLE - - if CORE_PMU_AVAILABLE is None: - CORE_PMU_AVAILABLE = core_pmu_probe() - + core_pmu_probe() return CORE_PMU_AVAILABLE def core_pmu_probe(): """ - Ask perf itself, through its python binding, to parse and open 'cycles': - a parse failure or an ENOENT means no PMU can count it, any other error - means we can't tell whether there is one. + Ask perf itself, through its python binding, to parse and open 'cycles' + once: a parse failure or an ENOENT means no PMU can count it, any other + error means we can't tell whether there is one, and the number of events + it expanded to is the number of core PMUs, see + parse_events_multi_pmu_add(), which is what core_pmu_count() asks for. """ + global CORE_PMU_AVAILABLE, CORE_PMU_COUNT + + if CORE_PMU_AVAILABLE is not None: + return + try: import perf parse_events = perf.parse_events except (ImportError, AttributeError): # Nothing to ask about it, let the expectations speak. - return True + CORE_PMU_AVAILABLE = True + return try: evlist = parse_events('cycles') except OSError: # No PMU that can count it, the same as the ENOENT below. - return False + CORE_PMU_AVAILABLE = False + return + CORE_PMU_COUNT = len(evlist) try: evlist.open() except OSError as err: # Anything else is a permission problem or alike, not our business here. - return err.errno != errno.ENOENT + CORE_PMU_AVAILABLE = err.errno != errno.ENOENT + return evlist.close() - return True + CORE_PMU_AVAILABLE = True PERF_TYPE_HARDWARE = 0 PERF_TYPE_HW_CACHE = 3
@@ -131,6 +139,45 @@ SAMPLE_ID_BITS = PERF_SAMPLE_ID | PERF_SAMPLE_IDENTIFIER # sample id bits" when comparing the two sides. SAMPLE_ID_EITHER = 1 << 40 +PERF_FORMAT_GROUP = 1 << 3 + +def core_pmu_count(): + """ + How many core PMUs does perf see? An event with no PMU named in it + becomes one event per core PMU, see parse_events_multi_pmu_add(), and + that is the expansion that leaves the clones for the other core PMUs + without any group members and that makes perf qualify 'config' with the + type of the PMU it is for, so this is the number of events perf makes of + 'cycles', from the probe core_pmu_available() shares. + """ + core_pmu_probe() + return CORE_PMU_COUNT + +def single_core_pmu(): + """ + Does perf see only one core PMU? Asked for by the comparisons that need + it, not at import time, as a run may never get to one. + """ + return core_pmu_count() == 1 + +def hide_group_format(expected, recorded): + """ + read_format's PERF_FORMAT_GROUP bit asks for the group members to be read + along with the leader, which aside from this bit is verified with the + fd/group_fd based group checking, and with more than one core PMU a group + whose members are pinned to one of them, say 'default_core', leaves the + clones for the other core PMUs without any members, so that bit isn't + there: whether it is there is a consequence of how the evlist got + expanded to more than one core PMU, not of what the expectations are + about, so take it out on both sides. On a machine with a single core PMU + there is nothing to expand, so the expectations keep verifying it. + """ + if single_core_pmu(): + return expected, recorded + + return (mask_bits(expected, PERF_FORMAT_GROUP), + mask_bits(recorded, PERF_FORMAT_GROUP)) + def hide_sample_id(expected, recorded): """ perf asks for a way to tell the samples apart, PERF_SAMPLE_IDENTIFIER where
@@ -160,6 +207,8 @@ def hide_sample_id(expected, recorded): return expected, mask_bits(recorded, SAMPLE_ID_BITS) if not asked: + if single_core_pmu(): + return expected, recorded return expected, mask_bits(recorded, SAMPLE_ID_BITS) return (str(int(expected, 0) & ~SAMPLE_ID_BITS | SAMPLE_ID_EITHER),
@@ -261,6 +310,8 @@ class Event(dict): def hide_event_count_dependent_terms(self, term, expected, recorded): if term == 'sample_type': return hide_sample_id(expected, recorded) + if term == 'read_format': + return hide_group_format(expected, recorded) return expected, recorded
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 15/21] perf test attr: Ignore the group read_format bit the PMU count can remove
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
From: Arnaldo Carvalho de Melo <[email protected]>
read_format's
PERF_FORMAT_GROUPbit asks for the group members to be read along with the leader, which aside from this bit is verified with the fd/group_fd based group checking inTest.compare(), and with more than one core PMU a group whose members are pinned to one of them, say 'default_core', leaves the clones for the other core PMUs without any members:$ perf stat -e '{cycles,default_core/legacy-hardware-config=3/}:S' ... the A53 leader keeps its cohort, both A73 cycles clones are alone, read_format 20 instead of 28
Quoted 8 lines
so that bit isn't there: whether it is there is a consequence of how the evlist got expanded to more than one core PMU, not of the groupness the [ ... ]
when the evlist has more than one event even with a single core PMU, keeps applying.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
Arnaldo Carvalho de Melo
maintainerPATCH v4 16/21 perf test attr: State, spelled out in a line, why a test is skipped
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Per skip the default output shows a self contained one liner:
unsupp 'test-record-branch-filter-any' exit 255, expected 1, no attrs recorded to verify skipped [kernel newer than 6.12] 'test-record-group-sampling'
skipped [arch = !aarch64, machine = aarch64] 'test-record-graph-default'
The extra notes explaining the reason, with a copy pasteable command to observe the failure, come in the very same stream when the verbosity is bumped, at -vvv, nothing is written to a separate place for later consultation, it is just the logging ladder:
detail: '/root/bin/perf record -o /dev/null --no-bpf-event -j any kill' exited with 255, the expectations want the workload's 1 exit, so there are no attrs recorded to verify; this is normally a capability problem, run '/root/bin/perf record -o /dev/null --no-bpf-event -j any kill' to see why
Note that 'perf test -v/-vv/-vvv attribute' appends a single -v to the shell test entry (shell_test__run()), so to see the notes one runs the python module directly, f.i.:
python3 tests/shell/lib/attr.py -vvv -d tests/shell/attr -p ./perf
The skip functions now return the reason fragment when they want to skip and None when the test can go on.
The command suggested to observe the failure writes to /dev/null, so it doesn't leave a perf.data in the directory it is run from.
--- tools/perf/tests/shell/lib/attr.py | 106 +++++++++++++++++++---------- 1 file changed, 70 insertions(+), 36 deletions(-)
tools/perf/tests/shell/lib/attr.py +70 -36
@@ -223,17 +223,20 @@ class Fail(Exception): return '\'%s\' - %s' % (self.test.path, self.msg) class Notest(Exception): - def __init__(self, test, arch): - self.arch = arch + def __init__(self, test, reason): + self.reason = reason self.test = test def getMsg(self): - return '[%s] \'%s\'' % (self.arch, self.test.path) + return '[%s] \'%s\'' % (self.reason, self.test.path) class Unsup(Exception): - def __init__(self, test): + def __init__(self, test, ret, expected): self.test = test + self.ret = ret + self.expected = expected def getMsg(self): - return '\'%s\'' % self.test.path + return '\'%s\' exit %d, expected %s, no attrs recorded to verify' % ( + self.test.path, self.ret, self.expected) class Event(dict): terms = [
@@ -391,6 +394,9 @@ def parse_version(version): return None return [int(v) for v in version.split(".")[0:2]] +def format_version(version): + return ".".join(str(v) for v in version) + # Test file description needs to have following sections: # [config] # - just single instance in file
@@ -456,13 +462,17 @@ class Test(object): def skip_test_kernel_since(self): if not self.kernel_since: - return False - return not self.kernel_since <= parse_version(platform.release()) + return None + if self.kernel_since <= parse_version(platform.release()): + return None + return "kernel older than %s" % format_version(self.kernel_since) def skip_test_kernel_until(self): if not self.kernel_until: - return False - return not parse_version(platform.release()) < self.kernel_until + return None + if parse_version(platform.release()) < self.kernel_until: + return None + return "kernel newer than %s" % format_version(self.kernel_until) def skip_test_auxv(self): def new_auxv(a, pattern):
@@ -478,18 +488,20 @@ class Test(object): return (items[0], value) if not self.auxv: - return False + return None auxv = subprocess.check_output("LD_SHOW_AUXV=1 sleep 0", shell=True) \ .decode(sys.stdout.encoding) pattern = re.compile(r"[: ]+") auxv = dict([new_auxv(a, pattern) for a in auxv.splitlines()]) - return not eval(self.auxv) + if eval(self.auxv): + return None + + return "the auxv condition doesn't hold here" def skip_test_arch(self, myarch): # If architecture not set always run test if self.arch == '': - # log.warning("test for arch %s is ok" % myarch) - return False + return None # Allow multiple values in assignment separated by ',' arch_list = self.arch.split(',')
@@ -499,27 +511,26 @@ class Test(object): arch_list[0] = arch_list[0][1:] log.warning("excluded architecture list %s", arch_list) for arch_item in arch_list: - # log.warning("test for %s arch is %s" % (arch_item, myarch)) if arch_item == myarch: - return True - return False + return "arch = %s, machine = %s" % (self.arch, myarch) + return None for arch_item in arch_list: - # log.warning("test for architecture '%s' current '%s'" % (arch_item, myarch)) if arch_item == myarch: - return False - return True + return None + + return "arch = %s, machine = %s" % (self.arch, myarch) def skip_test_requires(self): for requirement in filter(None, map(str.strip, self.requires.split(','))): if requirement == 'core-pmu': if not core_pmu_available(): log.warning("test limitation 'requires %s'", requirement) - return True + return "needs a core PMU, none here" else: log.warning("unknown requirement '%s', ignored", requirement) - return False + return None def restore_sample_rate(self, value=10000): try:
@@ -563,20 +574,36 @@ class Test(object): def run_cmd(self, tempdir): _junk1, _junk2, _junk3, _junk4, myarch = (os.uname()) - if self.skip_test_arch(myarch): - raise Notest(self, myarch) - - if self.skip_test_auxv(): - raise Notest(self, "auxv skip") - - if self.skip_test_kernel_since(): - raise Notest(self, "old kernel skip") - - if self.skip_test_kernel_until(): - raise Notest(self, "new kernel skip") - - if self.skip_test_requires(): - raise Notest(self, "requirements skip") + arch = self.skip_test_arch(myarch) + if arch: + log.debug(" detail: the test file has arch = %s, the machine reports %s", + self.arch, myarch) + raise Notest(self, arch) + + auxv = self.skip_test_auxv() + if auxv: + log.debug(" detail: the auxv condition %s doesn't hold, the bits come " + "from the LD_SHOW_AUXV=1 output", self.auxv) + raise Notest(self, auxv) + + kernel = self.skip_test_kernel_since() + if kernel: + log.debug(" detail: the test file's kernel_since = %s, this kernel is %s", + format_version(self.kernel_since), platform.release()) + raise Notest(self, kernel) + + kernel = self.skip_test_kernel_until() + if kernel: + log.debug(" detail: the test file's kernel_until = %s, this kernel is %s", + format_version(self.kernel_until), platform.release()) + raise Notest(self, kernel) + + requirement = self.skip_test_requires() + if requirement: + log.debug(" detail: requires core-pmu, nothing countable here, check " + "kernel.perf_event_paranoid and the PMU nodes under " + "/sys/bus/event_source/") + raise Notest(self, requirement) self.restore_sample_rate() cmd = "PERF_TEST_ATTR=%s %s %s -o %s/perf.data %s" % (tempdir,
@@ -586,10 +613,17 @@ class Test(object): log.info(" '%s' ret '%s', expected '%s'", cmd, str(ret), str(self.ret)) if not data_equal(str(ret), str(self.ret)): + plain_cmd = "%s %s -o /dev/null %s" % ( + self.perf, self.command, + self.args.split('>/dev/null')[0].rstrip()) if self.test_ret: raise Fail(self, "Perf exit code failure") else: - raise Unsup(self) + log.debug(" detail: '%s' exited with %d, the expectations want the " + "workload's %s exit, so there are no attrs recorded to " + "verify; this is normally a capability problem, run '%s' " + "to see why", plain_cmd, ret, self.ret, plain_cmd) + raise Unsup(self, ret, str(self.ret)) def drop_dummy_tracking_events(self): """
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 16/21] perf test attr: State, spelled out in a line, why a test is skipped
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Per skip the default output shows a self contained one liner:
unsupp 'test-record-branch-filter-any' exit 255, expected 1, no attrs recorded to verify skipped [kernel newer than 6.12] 'test-record-group-sampling'
skipped [arch = !aarch64, machine = aarch64] 'test-record-graph-default'The extra notes explaining the reason, with a copy pasteable command to observe the failure, come in the very same stream when the verbosity is bumped, at -vvv, nothing is written to a separate place for later consultation, it is just the logging ladder:
Quoted 8 lines
detail: '
/root/bin/perfrecord -o/dev/null--no-bpf-event -j any kill' exited with 255, the expectations want the workload's 1 exit, so there
[ ... ]
The command suggested to observe the failure writes to/dev/null, so it doesn't leave a perf.data in the directory it is run from.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
Arnaldo Carvalho de Melo
maintainerPATCH v4 17/21 perf tests: Introduce a 'needs_mlock' suite flag
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Each perf instance maps one ring buffer per CPU, all charged to the same per user budget set in kernel.perf_event_mlock_kb, so suites that map them need to be told apart from the ones that don't, to later have them take turns using that budget while the other tests keep running in parallel.
Add the flag to 'struct test_suite' and a DEFINE_SUITE_NEEDS_MLOCK() variant for the suites using the DEFINE_SUITE() form.
Shell tests run perf, that maps ring buffers, so they are taken to need room in that budget by default; the requisites, a comma separated list in the parens ending their description line, name what a test needs and take a "no_" prefix to waive it, e.g.:
# perf stat tests (no_mlock)
"mlock" is room in that budget, so stating it only spells the default out, "no_mlock" waives it, and "exclusive" is the catch-all that keeps a test from running alongside any other, so it runs alone, in sequence, and implies every other requisite, which is why it can't be waived either. More requisites are to come, each stating what a test needs, till the catch-all isn't needed anymore.
'perf test requisites' lists what each test needs, reusing the listing routine and taking the same test name fragments and numbers as 'list'.
--- tools/perf/Documentation/perf-test.txt | 35 ++++++++++- tools/perf/tests/builtin-test.c | 45 ++++++++++++-- tools/perf/tests/tests-scripts.c | 85 ++++++++++++++++++++++++-- tools/perf/tests/tests.h | 17 ++++++ 4 files changed, 168 insertions(+), 14 deletions(-)
tools/perf/Documentation/perf-test.txt +32 -3
@@ -8,7 +8,7 @@ perf-test - Runs sanity tests. SYNOPSIS -------- [verse] -'perf test [<options>] [{list <test-name-fragment>|[<test-name-fragments>|<test-numbers>]}]' +'perf test [<options>] [{list|requisites} [<test-name-fragment>]|[<test-name-fragments>|<test-numbers>]]' DESCRIPTION -----------
@@ -18,9 +18,38 @@ also will look for a directory with more tests in the form of scripts. To get a list of available tests use 'perf test list', specifying a test name fragment will show all tests that have it. +To get a list of what the tests need, i.e. of their requisites (see below), use +'perf test requisites', that takes the same test name fragments and numbers as +'list'. + To run just specific tests, inform test name fragments or the numbers obtained from 'perf test list'. +REQUISITES +---------- +Shell tests run perf, that maps ring buffers, so they are taken to need room +in the per user budget set in kernel.perf_event_mlock_kb, shared by the tests +running at the same time. Tests state what they need, or waive, in a comma +separated list in the parens ending their description line, e.g.: + + # perf stat tests (no_mlock) + +Each requisite is named in that list, "mlock" being room in the budget set in +kernel.perf_event_mlock_kb, and takes a "no_" prefix to waive it: "(mlock)" +states the default, "(no_mlock)" waives it, for tests that don't map ring +buffers. Leaving a requisite out states nothing about it, so what happens +then is up to each kind of test: shell ones run perf, that maps ring buffers, +so they are taken to need room in the budget unless they waive it, while C +suites say which ones map, see DEFINE_SUITE_NEEDS_MLOCK() in tests.h. +"exclusive" is a catch-all: the test can't run alongside any other one, so +it runs alone, in sequence, and implies every other requisite; it can't be +waived. More requisites are to come, each stating what a test needs, till +the catch-all isn't needed anymore. + +Use 'perf test requisites' to list what each test needs. That listing states +the needs, not the requisites form used in the scripts: a test listed as +needing "mlock" is one whose script doesn't say "(no_mlock)". + OPTIONS ------- -s::
@@ -36,8 +65,8 @@ OPTIONS -S:: --sequential:: - Run all tests one after the other. By default "exclusive" - tests are run sequentially, but other tests are run in + Run all tests one after the other. By default the tests asking to be + "exclusive" (see REQUISITES) run sequentially, but other tests run in parallel to speed execution. -r::
tools/perf/tests/builtin-test.c +40 -5
96 lines not on this page · raw message
tools/perf/tests/tests-scripts.c +79 -6
115 lines not on this page · raw message
tools/perf/tests/tests.h +17 -0
@@ -46,6 +46,12 @@ struct test_suite {struct test_case *test_cases;void *priv;int (*setup)(struct test_suite *suite); +/* +* Set for the suites mapping perf ring buffers, that need room in the +* per user budget set in kernel.perf_event_mlock_kb, see +* shell_test__requisites() for the shell suites. +*/ +bool needs_mlock; };#define DECLARE_SUITE (name) \
@@ -103,6 +109,17 @@ struct test_suite { .test_cases = tests__##_name, \ } +#define DEFINE_SUITE_NEEDS_MLOCK (description, _name) \ + struct test_case tests__##_name[] = { \ + TEST_CASE(description, _name), \ + { .name = NULL, } \ + }; \ + struct test_suite suite__##_name = { \ + .desc = description, \ + .test_cases = tests__##_name, \ + .needs_mlock = true, \ + } +/* Tests */ DECLARE_SUITE (vmlinux_matches_kallsyms);DECLARE_SUITE (openat_syscall_event);
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 17/21] perf tests: Introduce a 'needs_mlock' suite flag
Sashiko has reviewed this patch and found no issues.
Quoted 25 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Each perf instance maps one ring buffer per CPU, all charged to the same per user budget set in kernel.perf_event_mlock_kb, so suites that map them need to be told apart from the ones that don't, to later have them take turns using that budget while the other tests keep running in parallel.
Add the flag to 'struct
test_suite' and aDEFINE_SUITE_NEEDS_MLOCK()variant for the suites using theDEFINE_SUITE()form.Shell tests run perf, that maps ring buffers, so they are taken to need room in that budget by default; the requisites, a comma separated list in the parens ending their description line, name what a test needs and take a "
no_" prefix to waive it, e.g.:# perf stat tests (no_mlock)"mlock" is room in that budget, so stating it only spells the default out, "
no_mlock" waives it, and "exclusive" is the catch-all that keeps a test from running alongside any other, so it runs alone, in sequence, and implies every other requisite, which is why it can't be waived either. More requisites are to come, each stating what a test needs, till the catch-all isn't needed anymore.
Quoted 5 lines
'perf test requisites' lists what each test needs, reusing the listing routine and taking the same test name fragments and numbers as 'list'.
Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=17
Arnaldo Carvalho de Melo
maintainerPATCH v4 18/21 perf tests: Mark the C tests mapping perf ring buffers
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
The C tests that run 'perf record' in the test process itself map ring buffers and so need room in the per user budget set in kernel.perf_event_mlock_kb: mark them with the new 'needs_mlock' suite flag, read by the turnstile added later in the series, so that they take turns using that budget while the tests that don't map them keep running in parallel.
--- tools/perf/tests/backward-ring-buffer.c | 2 +- tools/perf/tests/code-reading.c | 2 +- tools/perf/tests/keep-tracking.c | 2 +- tools/perf/tests/mmap-basic.c | 1 + tools/perf/tests/openat-syscall-tp-fields.c | 1 + tools/perf/tests/perf-record.c | 1 + tools/perf/tests/perf-time-to-tsc.c | 1 + tools/perf/tests/sw-clock.c | 2 +- tools/perf/tests/task-exit.c | 1 + 9 files changed, 9 insertions(+), 4 deletions(-)
tools/perf/tests/backward-ring-buffer.c +1 -1
@@ -167,4 +167,4 @@ static int test__backward_ring_buffer(struct test_suite *test __maybe_unused, inreturn ret; } -DEFINE_SUITE ("Read backward ring buffer" , backward_ring_buffer); +DEFINE_SUITE_NEEDS_MLOCK ("Read backward ring buffer" , backward_ring_buffer);
tools/perf/tests/code-reading.c +1 -1
@@ -885,4 +885,4 @@ static int test__code_reading(struct test_suite *test __maybe_unused, int subtes }; } -DEFINE_SUITE ("Object code reading" , code_reading); +DEFINE_SUITE_NEEDS_MLOCK ("Object code reading" , code_reading);
tools/perf/tests/keep-tracking.c +1 -1
@@ -161,4 +161,4 @@ static int test__keep_tracking(struct test_suite *test __maybe_unused, int subtereturn err; } -DEFINE_SUITE ("Use a dummy software event to keep tracking" , keep_tracking); +DEFINE_SUITE_NEEDS_MLOCK ("Use a dummy software event to keep tracking" , keep_tracking);
tools/perf/tests/mmap-basic.c +1 -0
@@ -448,4 +448,5 @@ static struct test_case tests__basic_mmap[] = {struct test_suite suite__basic_mmap = { .desc ="mmap interface tests" , .test_cases = tests__basic_mmap, + .needs_mlock = true, };
tools/perf/tests/openat-syscall-tp-fields.c +1 -0
@@ -160,4 +160,5 @@ static struct test_case tests__syscall_openat_tp_fields[] = {struct test_suite suite__syscall_openat_tp_fields = { .desc ="syscalls:sys_enter_openat event fields" , .test_cases = tests__syscall_openat_tp_fields, + .needs_mlock = true, };
tools/perf/tests/perf-record.c +1 -0
@@ -372,4 +372,5 @@ static struct test_case tests__PERF_RECORD[] = {struct test_suite suite__PERF_RECORD = { .desc ="PERF_RECORD_* events & perf_sample fields" , .test_cases = tests__PERF_RECORD, + .needs_mlock = true, };
tools/perf/tests/perf-time-to-tsc.c +1 -0
@@ -218,4 +218,5 @@ static struct test_case time_to_tsc_tests[] = {struct test_suite suite__perf_time_to_tsc = { .desc ="Convert perf time to TSC" , .test_cases = time_to_tsc_tests, + .needs_mlock = true, };
tools/perf/tests/sw-clock.c +1 -1
@@ -149,4 +149,4 @@ static int test__sw_clock_freq(struct test_suite *test __maybe_unused, int subtereturn ret; } -DEFINE_SUITE ("Software clock events period values" , sw_clock_freq); +DEFINE_SUITE_NEEDS_MLOCK ("Software clock events period values" , sw_clock_freq);
tools/perf/tests/task-exit.c +1 -0
@@ -158,4 +158,5 @@ struct test_case tests__task_exit[] = {struct test_suite suite__task_exit = { .desc ="Number of exit events of a simple workload" , .test_cases = tests__task_exit, + .needs_mlock = true, };
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 18/21] perf tests: Mark the C tests mapping perf ring buffers
Sashiko has reviewed this patch and found no issues.
Quoted 11 lines
From: Arnaldo Carvalho de Melo <[email protected]>
The C tests that run 'perf record' in the test process itself map ring buffers and so need room in the per user budget set in kernel.perf_event_mlock_kb: mark them with the new '
needs_mlock' suite flag, read by the turnstile added later in the series, so that they take turns using that budget while the tests that don't map them keep running in parallel.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=18
Arnaldo Carvalho de Melo
maintainerPATCH v4 19/21 perf tests: Mark the shell tests not mapping perf ring buffers
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
Shell tests run perf, that maps ring buffers, so they take a turn using the per user budget in kernel.perf_event_mlock_kb by default; the ones that don't map them, like the ones running just 'perf stat', say " (no_mlock)" in their description line and thus keep running in parallel with the ones taking turns.
The 'perf sched stats' tests are among them, they synthesize the samples from /proc/schedstat without mapping a ring buffer, and the tests already marked " (exclusive)" don't need it, exclusive implies every other requisite and keeps them running alone.
--- tools/perf/tests/shell/drm_pmu.sh | 2 +- tools/perf/tests/shell/ftrace.sh | 2 +- tools/perf/tests/shell/kallsyms.sh | 2 +- tools/perf/tests/shell/list.sh | 2 +- tools/perf/tests/shell/perf_sched_stats.sh | 2 +- tools/perf/tests/shell/probe_vfs_getname.sh | 2 +- tools/perf/tests/shell/python-use.sh | 2 +- tools/perf/tests/shell/schedstat_snapshots.sh | 2 +- tools/perf/tests/shell/stat+csv_output.sh | 2 +- tools/perf/tests/shell/stat+csv_summary.sh | 2 +- tools/perf/tests/shell/stat+event_uniquifying.sh | 2 +- tools/perf/tests/shell/stat+json_output.sh | 2 +- tools/perf/tests/shell/stat+shadow_stat.sh | 2 +- tools/perf/tests/shell/stat+std_output.sh | 2 +- tools/perf/tests/shell/stat.sh | 2 +- tools/perf/tests/shell/stat_all_metricgroups.sh | 2 +- tools/perf/tests/shell/stat_all_metrics.sh | 2 +- tools/perf/tests/shell/stat_all_pfm.sh | 2 +- tools/perf/tests/shell/stat_bpf_counters_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_values.sh | 2 +- tools/perf/tests/shell/test_ilist_python.sh | 2 +- tools/perf/tests/shell/test_stat_cpi_python.sh | 2 +- tools/perf/tests/shell/test_test_junit_output.sh | 5 ++++- tools/perf/tests/shell/test_uprobe_from_different_cu.sh | 2 +- 25 files changed, 28 insertions(+), 25 deletions(-)
tools/perf/tests/shell/drm_pmu.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# DRM PMU +# DRM PMU (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/ftrace.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf ftrace tests +# perf ftrace tests (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/kallsyms.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf kallsyms tests +# perf kallsyms tests (no_mlock) # SPDX-License-Identifier: GPL-2.0 err=0
tools/perf/tests/shell/list.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf list tests +# perf list tests (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/perf_sched_stats.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/sh -# perf sched stats tests +# perf sched stats tests (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/probe_vfs_getname.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# Add vfs_getname probe to get syscall args filenames +# Add vfs_getname probe to get syscall args filenames (no_mlock) # SPDX-License-Identifier: GPL-2.0 # Arnaldo Carvalho de Melo <[email protected]>, 2017
tools/perf/tests/shell/python-use.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# 'import perf' in python +# 'import perf' in python (no_mlock) # SPDX-License-Identifier: GPL-2.0 # Just test if we can load the python binding. set -e
tools/perf/tests/shell/schedstat_snapshots.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/sh -# Validate CPU and domain pairing in perf sched stats snapshots +# Validate CPU and domain pairing in perf sched stats snapshots (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat+csv_output.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat CSV output linter +# perf stat CSV output linter (no_mlock) # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) # Tests various perf stat CSV output commands for the # correct number of fields and the CSV separator set to ','.
tools/perf/tests/shell/stat+csv_summary.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat csv summary test +# perf stat csv summary test (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat+event_uniquifying.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat events uniquifying +# perf stat events uniquifying (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat+json_output.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat JSON output linter +# perf stat JSON output linter (no_mlock) # SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause) # Checks various perf stat JSON output commands for the # correct number of fields.
tools/perf/tests/shell/stat+shadow_stat.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat metrics (shadow stat) test +# perf stat metrics (shadow stat) test (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat+std_output.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat STD output linter +# perf stat STD output linter (no_mlock) # SPDX-License-Identifier: GPL-2.0 # Tests various perf stat STD output commands for # default event and metricgroup
tools/perf/tests/shell/stat.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat tests +# perf stat tests (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat_all_metricgroups.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf all metricgroups test +# perf all metricgroups test (no_mlock) # SPDX-License-Identifier: GPL-2.0 ParanoidAndNotRoot()
tools/perf/tests/shell/stat_all_metrics.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf all metrics test +# perf all metrics test (no_mlock) # SPDX-License-Identifier: GPL-2.0 ParanoidAndNotRoot()
tools/perf/tests/shell/stat_all_pfm.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf all libpfm4 events test +# perf all libpfm4 events test (no_mlock) # SPDX-License-Identifier: GPL-2.0 if perf version --build-options | grep HAVE_LIBPFM | grep -q OFF
tools/perf/tests/shell/stat_bpf_counters_cgrp.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat --bpf-counters --for-each-cgroup test +# perf stat --bpf-counters --for-each-cgroup test (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat_metrics_cgrp.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf stat metrics --for-each-cgroup test +# perf stat metrics --for-each-cgroup test (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/stat_metrics_values.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf metrics value validation +# perf metrics value validation (no_mlock) # SPDX-License-Identifier: GPL-2.0 shelldir=$(dirname "$0")
tools/perf/tests/shell/test_ilist_python.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# perf script ilist and perf list --tui test +# perf script ilist and perf list --tui test (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
tools/perf/tests/shell/test_stat_cpi_python.sh +1 -1
@@ -1,6 +1,6 @@ #!/bin/bash # SPDX-License-Identifier: GPL-2.0 -# stat-cpi python test +# stat-cpi python test (no_mlock) set -e
tools/perf/tests/shell/test_test_junit_output.sh +4 -1
@@ -1,6 +1,9 @@ #!/bin/bash # SPDX-License-Identifier: GPL-2.0 -# perf test junit XML output validation +# perf test junit XML output validation (no_mlock) +# +# Waiving the mlock requisite this way holds only while the 'perf test' +# runs below keep picking tests that don't map ring buffers. set -e
tools/perf/tests/shell/test_uprobe_from_different_cu.sh +1 -1
@@ -1,5 +1,5 @@ #!/bin/bash -# test perf probe of function from different CU +# test perf probe of function from different CU (no_mlock) # SPDX-License-Identifier: GPL-2.0 set -e
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 19/21] perf tests: Mark the shell tests not mapping perf ring buffers
Sashiko has reviewed this patch and found no issues.
Quoted 8 lines
From: Arnaldo Carvalho de Melo <[email protected]>
Shell tests run perf, that maps ring buffers, so they take a turn using the per user budget in kernel.perf_event_mlock_kb by default; the ones that don't map them, like the ones running just 'perf stat', say " (
no_mlock)" in their description line and thus keep running in parallel with the ones taking turns.
Quoted 7 lines
The 'perf sched stats' tests are among them, they synthesize the samples from
/proc/schedstatwithout mapping a ring buffer, and the tests already marked " (exclusive)" don't need it, exclusive implies every other requisite and keeps them running alone.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19
Arnaldo Carvalho de Melo
maintainerPATCH v4 20/21 perf mutex: Add a robust, process-shared mutex initializer
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
A mutex held by a process that can die before unlocking it, like the tests forked by 'perf test' that will take turns using the per user budget set in kernel.perf_event_mlock_kb, has to be taken over by the next locker instead of staying locked forever.
Robust mutexes, on top of the kernel's robust futexes, do that: when an owner dies while holding it the kernel marks the futex word owner-died and the next pthread_mutex_lock() returns EOWNERDEAD, then mutex_lock() makes it consistent and takes it over; mutex_trylock() and cond_wait() come back with EOWNERDEAD too, so they take it over the same way, with mutex_trylock() giving the mutex back when it can't be taken over: it is held all the same, and a failed trylock must not leak it.
Add mutex_init_pshared_robust(), returning false when robust mutexes aren't available: from libcs without the API, gated by a feature check, to musl and glibc refusing them at runtime, when get_robust_list(2) or set_robust_list(2) aren't usable. glibc only sets a flag in pthread_mutexattr_setrobust(), refusing the mutex itself in pthread_mutex_init() with ENOTSUP instead, other errors from it still being reported. Robust mutexes being unavailable isn't an error, just a debug message for whoever then falls back.
--- tools/build/Makefile.feature | 1 + tools/build/feature/Makefile | 4 + tools/build/feature/test-all.c | 5 + .../build/feature/test-pthread-mutex-robust.c | 18 ++++ tools/perf/Makefile.config | 4 + tools/perf/util/mutex.c | 101 +++++++++++++++--- tools/perf/util/mutex.h | 6 ++ 7 files changed, 125 insertions(+), 14 deletions(-) create mode 100644 tools/build/feature/test-pthread-mutex-robust.c
tools/build/Makefile.feature +1 -0
@@ -85,6 +85,7 @@ FEATURE_TESTS_BASIC := \ libcpupower \ pthread-attr-setaffinity-np \ pthread-barrier \ + pthread-mutex-robust \ reallocarray \ stackprotector-all \ timerfd \
tools/build/feature/Makefile +4 -0
@@ -41,6 +41,7 @@ FILES= \ $(foreach arch,$(LIBUNWIND_ARCHS),test-libunwind-$(arch).bin test-libunwind-debug-frame-$(arch).bin) \ test-pthread-attr-setaffinity-np.bin \ test-pthread-barrier.bin \ + test-pthread-mutex-robust.bin \ test-stackprotector-all.bin \ test-timerfd.bin \ test-babeltrace2-ctf-writer.bin \
@@ -128,6 +129,9 @@ $(OUTPUT)test-pthread-attr-setaffinity-np.bin: $(OUTPUT)test-pthread-barrier.bin: $(BUILD) -lpthread +$(OUTPUT)test-pthread-mutex-robust.bin: + $(BUILD) -lpthread + $(OUTPUT)test-stackprotector-all.bin: $(BUILD) -fstack-protector-all
tools/build/feature/test-all.c +5 -0
@@ -90,6 +90,10 @@# include "test-pthread-barrier.c" #undef main +#define main main_test_pthread_mutex_robust +# include "test-pthread-mutex-robust.c" +#undef main +#define main main_test_scandirat# include "test-scandirat.c" #undef main
@@ -167,6 +171,7 @@ int main(int argc, char *argv[])main_test_zlib ();main_test_pthread_attr_setaffinity_np ();main_test_pthread_barrier (); +main_test_pthread_mutex_robust ();main_test_lzma ();main_test_bpf ();main_test_scandirat ();
tools/build/feature/test-pthread-mutex-robust.c new file +18 -0
@@ -0,0 +1,18 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <pthread.h> + +int main (void ) +{ +pthread_mutexattr_t attr; +pthread_mutex_t mtx; + +pthread_mutexattr_init (&attr); +pthread_mutexattr_setpshared (&attr,PTHREAD_PROCESS_SHARED ); +pthread_mutexattr_setrobust (&attr,PTHREAD_MUTEX_ROBUST ); +pthread_mutex_init (&mtx, &attr); +pthread_mutex_consistent (&mtx); +pthread_mutex_destroy (&mtx); +pthread_mutexattr_destroy (&attr); + +return 0 ; +}
tools/perf/Makefile.config +4 -0
@@ -417,6 +417,10 @@ ifeq ($(feature-pthread-barrier), 1) CFLAGS += -DHAVE_PTHREAD_BARRIER endif +ifeq ($(feature-pthread-mutex-robust), 1) + CFLAGS += -DHAVE_PTHREAD_MUTEX_ROBUST +endif + ifndef NO_BIONIC $(call feature_check,bionic) ifeq ($(feature-bionic), 1)
tools/perf/util/mutex.c +87 -14
145 lines not on this page · raw message
tools/perf/util/mutex.h +6 -0
@@ -142,6 +142,12 @@ void mutex_init(struct mutex *mtx);void mutex_init_pshared (struct mutex *mtx);/* Initializes a mutex that may be recursively held on the same thread. */ void mutex_init_recursive (struct mutex *mtx); +/* +* As mutex_init_pshared(), plus the robust attribute: a process dying while +* holding it doesn't leave it locked, but the takeover isn't reported and the +* data it protects may be mid update. Returns false when not available. +*/ +bool mutex_init_pshared_robust (struct mutex *mtx);void mutex_destroy (struct mutex *mtx); void mutex_lock(struct mutex *mtx) EXCLUSIVE_LOCK_FUNCTION(*mtx);
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 20/21] perf mutex: Add a robust, process-shared mutex initializer
Sashiko has reviewed this patch and found no issues.
Quoted 24 lines
From: Arnaldo Carvalho de Melo <[email protected]>
A mutex held by a process that can die before unlocking it, like the tests forked by 'perf test' that will take turns using the per user budget set in kernel.perf_event_mlock_kb, has to be taken over by the next locker instead of staying locked forever.
Robust mutexes, on top of the kernel's robust futexes, do that: when an owner dies while holding it the kernel marks the futex word owner-died and the next
pthread_mutex_lock()returns EOWNERDEAD, thenmutex_lock()makes it consistent and takes it over;mutex_trylock()andcond_wait()come back with EOWNERDEAD too, so they take it over the same way, withmutex_trylock()giving the mutex back when it can't be taken over: it is held all the same, and a failed trylock must not leak it.Add
mutex_init_pshared_robust(), returning false when robust mutexes aren't available: from libcs without the API, gated by a feature check, to musl and glibc refusing them at runtime, whenget_robust_list(2)orset_robust_list(2)aren't usable. glibc only sets a flag inpthread_mutexattr_setrobust(), refusing the mutex itself inpthread_mutex_init()with ENOTSUP instead, other errors from it still being reported. Robust mutexes being unavailable isn't an error, just a debug message for whoever then falls back.
Quoted 2 lines
Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=20
Arnaldo Carvalho de Melo
maintainerPATCH v4 21/21 perf test: Take turns using the perf mlock budget in the mmaping tests
In reply to Arnaldo Carvalho de MeloFrom: Arnaldo Carvalho de Melo <[email protected]>
The mmap size used when none is asked for is derived from kernel.perf_event_mlock_kb itself (see perf_event_mlock_kb_in_pages()), that after rounding down to a power of two number of pages ends up being about all of a single CPU's share of that per user budget, so with 'perf test' starting all the selected tests at once, any further mapping done while another test still has its ring buffers mapped fails with:
Permission error mapping pages.
Consider increasing /proc/sys/kernel/perf_event_mlock_kb, or try again with a smaller value of -m/--mmap_pages.
that then shows up as a failure or, worse, as an "unsupp" skip in tests like the attr ones, i.e. as a false pass.
Instead of making those tests exclusive, which would serialize them against every other test, keep them running in parallel with the tests that don't map ring buffers and have them take turns using the budget, via a robust, process-shared mutex (see mutex_init_pshared_robust()) in a MAP_SHARED|MAP_ANONYMOUS area that is inherited by the forked test
processes: a test killed by a signal doesn't leave the budget locked, the kernel marks the mutex owner-died and the next locker takes it over; a signal that is caught rather than kills ends up the same way, with the longjmp out of child_test_sig_handler() leaving it held until the child exits and the kernel hands it over.
Perf's struct mutex is used, so the lock discipline is checked in clang builds via -Wthread-safety; as the thread safety analysis can't verify conditionally held locks, the tests that map ring buffers run through a helper taking the mutex unconditionally around the test function call.
Robust mutexes aren't always available, at build time or, as glibc and musl can do when get_robust_list(2)/set_robust_list(2) aren't usable, at run time: in that case the budget is not shared and the tests run as before, failing or skipping on starvation instead of hanging on a lock left behind by a test killed by a signal.
Taking turns is unnecessary when nothing can exhaust the budget: once it is spent, whatever maps on top of it is charged against RLIMIT_MEMLOCK and the mapping is refused with EPERM when that runs out too, but only for the processes the kernel is paranoid about and that have no CAP_IPC_LOCK to lift the check, see perf_mmap_calc_limits(). Root, then, has no turns to
take: it never has a mapping refused, so the turnstile only buys it the wall clock cost measured below, for a budget nothing is contending for.
The area is shared by the tests of one 'perf test' invocation only: a second invocation running at the same time maps its own, so the two don't take turns with each other.
The sysctl is left alone. Growing it, to leave room for whatever else the user has running, would need the original value shared between the invocations, so that the last one out puts it back, and it only helps a user who can write the sysctl but lacks CAP_IPC_LOCK, the one capability that lifts the limit.
Raising the soft RLIMIT_MEMLOCK to the hard one when a mapping is refused is left for a follow-up: it would buy back the wall clock above without taking turns, where the hard limit allows the raise.
None of this is done when just one of the selected tests maps ring buffers, as there is nothing to share the budget with in that case.
Taking turns isn't free: a test that hangs while holding the budget now holds with it every other test mapping ring buffers, where before it would fail, or skip, by itself. And there is a cost even when nothing goes wrong, this being the whole suite on a Ryzen 9 9950X3D, kernel 7.2.7, 32 CPUs, with the default 516 KiB budget, before and after taking turns:
budget not shared: 42 s wall clock, 72 tests Ok, 28 failed
tests take turns: 133 s wall clock, 101 tests Ok, 10 failed
Both are non-root runs of the whole series applied, the shell test markings included, and none of the ten failures is about the budget: eight of them pass as root, where it can't be exhausted, and the two that fail there too, the addr2line inline unwinding one, with no branch stack sampling to ask this machine for, and the BPF metadata collection one, whose output isn't what the test expects, are not budget related either.
--- tools/perf/Documentation/perf-test.txt | 5 +- tools/perf/tests/builtin-test.c | 129 ++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 3 deletions(-)
tools/perf/Documentation/perf-test.txt +3 -2
@@ -29,8 +29,9 @@ REQUISITES ---------- Shell tests run perf, that maps ring buffers, so they are taken to need room in the per user budget set in kernel.perf_event_mlock_kb, shared by the tests -running at the same time. Tests state what they need, or waive, in a comma -separated list in the parens ending their description line, e.g.: +running at the same time, that take turns using it. Tests state what they +need, or waive, in a comma separated list in the parens ending their +description line, e.g.: # perf stat tests (no_mlock)
tools/perf/tests/builtin-test.c +128 -1
178 lines not on this page · raw message
Signature · 1 line
2.55.0
[email protected]
AI reviewRe: [PATCH v4 21/21] perf test: Take turns using the perf mlock budget in the mmaping tests
Sashiko has reviewed this patch and found no issues.
Quoted 13 lines
From: Arnaldo Carvalho de Melo <[email protected]>
The mmap size used when none is asked for is derived from kernel.perf_event_mlock_kb itself (see
perf_event_mlock_kb_in_pages()), that after rounding down to a power of two number of pages ends up being about all of a single CPU's share of that per user budget, so with 'perf test' starting all the selected tests at once, any further mapping done while another test still has its ring buffers mapped fails with:Permission error mapping pages.
Consider increasing/proc/sys/kernel/perf_event_mlock_kb, or try again with a smaller value of -m/--mmap_pages.
Quoted 8 lines
that then shows up as a failure or, worse, as an "unsupp" skip in tests like the attr ones, i.e. as a false pass.
[ ... ]
this machine for, and the BPF metadata collection one, whose output isn't what the test expects, are not budget related either.Assisted-by LLMSigned-off-by Arnaldo Carvalho de Melo <[email protected]>
Sashiko has reviewed this patch and found no issues.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=21
Quoted 20 lines · Arnaldo Carvalho de Melo
From: Arnaldo Carvalho de Melo <[email protected]>
Hello all,
perf test 82 stopped reporting failures when it got its shell wrapper (
8519e4f44c2af722), so expectations that no longer match perf went unverified. The prerequisite fixes are upstream now, so they are not repeated here.This series updates those expectations and makes the test cope with machines with no core PMU, or with more than one. Asking for one event on the latter gives one event per core PMU. The expectations ignore what that adds; the commit messages say what and why.
The tests that map ring buffers take turns on the per user mlock budget (kernel.perf_event_mlock_kb) through a robust, process-shared mutex. The sysctl is left alone. A test killed while holding the budget does not leave the next one waiting.
Quoted 5 lines
Tested: 'perf test -vv attribute' Ok on an Amlogic A311D (aarch64, as root), an Intel i7-14700K (hybrid, two core PMUs),
x86_64(AMD Ryzen 9 9950X3D, kernel 7.2.10-200.fc44) and riscv64 (Orange Pi RV). On riscv64 the stat tests run and the record tests report unsupp, as sampling is not available there (no Sscofpmf).
On a hybrid Alderlake:
Thanks,
Ian
Quoted 107 lines
Best regards,
- Arnaldo
What changed from v3:
- sashiko-bot's v3 review: the python binding no longer leaks the parse error entries it prints when parsing fails.
- sashiko-bot's v3 review:attr.pyno longer compares an event against itself unrecorded, and masks both sides with both events' types.
- sashiko-bot's v3 review:evsel__open_cpu()no longer leaks the fds of an attempt abandoned to disable missing features.
- sashiko-bot's v3 review: <stdio.h> included where FILE andsnprintf()are used, inbuiltin-test.candtests-scripts.c.What changed from v2:
- sashiko-bot's v2 review: a one-component
kernel_sinceorkernel_until(f.i.kernel_until= 6) no longer crashesattr.py.
- sashiko-bot's v2 review:mutex_trylock()andcond_wait()now take over a robust mutex left behind by a dead owner, asmutex_lock().What changed from v1:
- Namhyung's python fix and Michael Petlan's two fixes are upstream now, so they are dropped from the series.
- Namhyung's v1 report (test-record-branch-filter-any_call): probe events are no longer dumped, and the core PMU fallout is masked.
- New, not from review: the 'requires = core-pmu' skip, the skip reasons and the mlock budget part.
- New, not from review: the events of the other core PMUs are left out, when the expectations name a core PMU.tools/build/Makefile.feature | 1 + tools/build/feature/Makefile | 4 + tools/build/feature/test-all.c | 5 + .../build/feature/test-pthread-mutex-robust.c | 18 + tools/perf/Documentation/perf-test.txt | 36 +- tools/perf/Makefile.config | 4 + tools/perf/tests/backward-ring-buffer.c | 2 +- tools/perf/tests/builtin-test.c | 174 ++++++- tools/perf/tests/code-reading.c | 2 +- tools/perf/tests/keep-tracking.c | 2 +- tools/perf/tests/mmap-basic.c | 1 + tools/perf/tests/openat-syscall-tp-fields.c | 1 + tools/perf/tests/perf-record.c | 1 + tools/perf/tests/perf-time-to-tsc.c | 1 + tools/perf/tests/shell/attr/README | 10 +- tools/perf/tests/shell/attr/base-stat-group | 45 ++ .../tests/shell/attr/test-record-graph-dwarf | 2 +- .../shell/attr/test-record-group-sampling | 5 +- .../shell/attr/test-record-group-sampling1 | 5 +- .../shell/attr/test-record-group-sampling2 | 5 +- .../perf/tests/shell/attr/test-record-group1 | 3 +- .../perf/tests/shell/attr/test-record-group2 | 3 +- .../perf/tests/shell/attr/test-record-group3 | 3 +- tools/perf/tests/shell/attr/test-stat-C0 | 1 + tools/perf/tests/shell/attr/test-stat-basic | 1 + tools/perf/tests/shell/attr/test-stat-default | 91 ++-- .../tests/shell/attr/test-stat-detailed-1 | 99 ++-- .../tests/shell/attr/test-stat-detailed-2 | 111 +++-- .../tests/shell/attr/test-stat-detailed-3 | 115 +++-- tools/perf/tests/shell/attr/test-stat-group1 | 3 +- .../tests/shell/attr/test-stat-no-inherit | 1 + tools/perf/tests/shell/drm_pmu.sh | 2 +- tools/perf/tests/shell/ftrace.sh | 2 +- tools/perf/tests/shell/kallsyms.sh | 2 +- tools/perf/tests/shell/lib/attr.py | 461 ++++++++++++++++-- tools/perf/tests/shell/list.sh | 2 +- tools/perf/tests/shell/perf_sched_stats.sh | 2 +- tools/perf/tests/shell/probe_vfs_getname.sh | 2 +- tools/perf/tests/shell/python-use.sh | 2 +- tools/perf/tests/shell/schedstat_snapshots.sh | 2 +- tools/perf/tests/shell/stat+csv_output.sh | 2 +- tools/perf/tests/shell/stat+csv_summary.sh | 2 +- .../tests/shell/stat+event_uniquifying.sh | 2 +- tools/perf/tests/shell/stat+json_output.sh | 2 +- tools/perf/tests/shell/stat+shadow_stat.sh | 2 +- tools/perf/tests/shell/stat+std_output.sh | 2 +- tools/perf/tests/shell/stat.sh | 2 +- .../perf/tests/shell/stat_all_metricgroups.sh | 2 +- tools/perf/tests/shell/stat_all_metrics.sh | 2 +- tools/perf/tests/shell/stat_all_pfm.sh | 2 +- .../tests/shell/stat_bpf_counters_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_cgrp.sh | 2 +- tools/perf/tests/shell/stat_metrics_values.sh | 2 +- tools/perf/tests/shell/test_ilist_python.sh | 2 +- .../perf/tests/shell/test_stat_cpi_python.sh | 2 +- .../tests/shell/test_test_junit_output.sh | 5 +- .../shell/test_uprobe_from_different_cu.sh | 2 +- tools/perf/tests/sw-clock.c | 2 +- tools/perf/tests/task-exit.c | 1 + tools/perf/tests/tests-scripts.c | 85 +++- tools/perf/tests/tests.h | 17 + tools/perf/util/evsel.c | 42 +- tools/perf/util/evsel.h | 1 + tools/perf/util/mutex.c | 101 +++- tools/perf/util/mutex.h | 6 + tools/perf/util/pfm.c | 1 + tools/perf/util/print-events.c | 1 + tools/perf/util/python.c | 2 + 68 files changed, 1256 insertions(+), 272 deletions(-)base-commit:
cb9dba6f4764fc68ab75c36e332887332339f9c3
v1-head:b1300831ecc4503d
v2-head:da166ef7a002
v3-head:c5f0897aceb01743