2024-01-24 10:11:34

by James Clark

[permalink] [raw]
Subject: [PATCH v4] perf evlist: Fix evlist__new_default() for > 1 core PMU

The 'Session topology' test currently fails with this message when
evlist__new_default() opens more than one event:

32: Session topology :
--- start ---
templ file: /tmp/perf-test-vv5YzZ
Using CPUID 0x00000000410fd070
Opening: unknown-hardware:HG
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
config 0xb00000000
disabled 1
------------------------------------------------------------
sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 4
Opening: unknown-hardware:HG
------------------------------------------------------------
perf_event_attr:
type 0 (PERF_TYPE_HARDWARE)
config 0xa00000000
disabled 1
------------------------------------------------------------
sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 5
non matching sample_type
FAILED tests/topology.c:73 can't get session
---- end ----
Session topology: FAILED!

This is because when re-opening the file and parsing the header, Perf
expects that any file that has more than one event has the sample ID
flag set. Perf record already sets the flag in a similar way when there
is more than one event, so add the same logic to evlist__new_default().

evlist__new_default() is only currently used in tests, so I don't
expect this change to have any other side effects. The other tests that
use it don't save and re-open the file so don't hit this issue.

The session topology test has been failing on Arm big.LITTLE platforms
since commit 251aa040244a ("perf parse-events: Wildcard most
"numeric" events") when evlist__new_default() started opening multiple
events for 'cycles'.

Fixes: 251aa040244a ("perf parse-events: Wildcard most "numeric" events")
Closes: https://lore.kernel.org/lkml/CAP-5=fWVQ-7ijjK3-w1q+k2WYVNHbAcejb-xY0ptbjRw476VKA@mail.gmail.com/
Tested-by: Ian Rogers <[email protected]>
Reviewed-by: Ian Rogers <[email protected]>
Tested-by: Kan Liang <[email protected]>
Signed-off-by: James Clark <[email protected]>
---
tools/perf/util/evlist.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Changes since v3:

* Return early if parsing fails, don't try to set the sample id.

Changes since v2:

* Undo the fact that v2 was accidentally based on v1 instead of
perf-tools

Changes since v1:

* Reduce scope of evsel variable
* Add argument label
* Change summary to be less specific about the failing test
* Add the closes: tag

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 95f25e9fb994..55a300a0977b 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -103,7 +103,14 @@ struct evlist *evlist__new_default(void)
err = parse_event(evlist, can_profile_kernel ? "cycles:P" : "cycles:Pu");
if (err) {
evlist__delete(evlist);
- evlist = NULL;
+ return NULL;
+ }
+
+ if (evlist->core.nr_entries > 1) {
+ struct evsel *evsel;
+
+ evlist__for_each_entry(evlist, evsel)
+ evsel__set_sample_id(evsel, /*can_sample_identifier=*/false);
}

return evlist;
--
2.34.1



2024-01-25 21:46:20

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH v4] perf evlist: Fix evlist__new_default() for > 1 core PMU

On Wed, 24 Jan 2024 09:43:57 +0000, James Clark wrote:
> The 'Session topology' test currently fails with this message when
> evlist__new_default() opens more than one event:
>
> 32: Session topology :
> --- start ---
> templ file: /tmp/perf-test-vv5YzZ
> Using CPUID 0x00000000410fd070
> Opening: unknown-hardware:HG
> ------------------------------------------------------------
> perf_event_attr:
> type 0 (PERF_TYPE_HARDWARE)
> config 0xb00000000
> disabled 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 4
> Opening: unknown-hardware:HG
> ------------------------------------------------------------
> perf_event_attr:
> type 0 (PERF_TYPE_HARDWARE)
> config 0xa00000000
> disabled 1
> ------------------------------------------------------------
> sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 5
> non matching sample_type
> FAILED tests/topology.c:73 can't get session
> ---- end ----
> Session topology: FAILED!
>
> [...]

Applied to perf-tools-next, thanks!

Best regards,
--
Namhyung Kim <[email protected]>

2024-01-30 14:39:29

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v4] perf evlist: Fix evlist__new_default() for > 1 core PMU

Em Wed, Jan 24, 2024 at 09:43:57AM +0000, James Clark escreveu:
> Fixes: 251aa040244a ("perf parse-events: Wildcard most "numeric" events")
> Closes: https://lore.kernel.org/lkml/CAP-5=fWVQ-7ijjK3-w1q+k2WYVNHbAcejb-xY0ptbjRw476VKA@mail.gmail.com/
> Tested-by: Ian Rogers <[email protected]>
> Reviewed-by: Ian Rogers <[email protected]>
> Tested-by: Kan Liang <[email protected]>
> Signed-off-by: James Clark <[email protected]>

Thanks, applied to perf-tools.

- Arnaldo