Messages in this thread | | | Date | Tue, 9 Apr 2024 22:05:32 +0300 | Subject | Re: BISECTED: perf test 'Miscellaneous Intel PT' failing on Intel hybrid machines | From | Adrian Hunter <> |
| |
On 9/04/24 18:46, Ian Rogers wrote: > On Tue, Apr 9, 2024 at 8:34 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: >> >> On Tue, Apr 09, 2024 at 12:32:06PM -0300, Arnaldo Carvalho de Melo wrote: >>> root@x1:~# perf test "Miscellaneous Intel PT testing" >>> 112: Miscellaneous Intel PT testing : FAILED! >>> root@x1:~# >>> >>> then I revert: >>> >>> commit 642e1ac96aaa12aeb41402e68eac7faf5917a67a (HEAD -> perf-tools-next) >>> Author: Arnaldo Carvalho de Melo <acme@redhat.com> >>> Date: Tue Apr 9 12:28:49 2024 -0300 >>> >>> Revert "perf pmus: Check if we can encode the PMU number in perf_event_attr.type" >>> >>> This reverts commit 82fe2e45cdb00de4fa648050ae33bdadf9b3294a. >>> ⬢[acme@toolbox perf-tools-next]$ >>> >>> It works now: >>> >>> root@x1:~# perf -v >>> perf version 6.8.g642e1ac96aaa >>> root@x1:~# perf test "Miscellaneous Intel PT testing" >>> 117: Miscellaneous Intel PT testing : Ok >>> root@x1:~# >>> >>> Investigating, if you come up with ideas, lemme know. >> >> Some more context: >> >> When this patch was implemented/tested I had access only to an ARM64 >> hybrid machine, now my notebook is a Rocket Lake lenovo (13th gen), that >> is hybrid and the test is failing with: >> >> root@x1:~# perf test -v "Miscellaneous Intel PT testing" >> 112: Miscellaneous Intel PT testing : >> --- start --- >> test child forked, pid 304355 >> --- Test system-wide sideband --- >> Checking for CPU-wide recording on CPU 0 >> OK >> Checking for CPU-wide recording on CPU 1 >> OK >> Linux >> [ perf record: Woken up 1 times to write data ] >> [ perf record: Captured and wrote 1.934 MB /tmp/perf-test-intel-pt-sh.xACV6V7Hn4/test-perf.data ] >> OK >> --- Test per-thread recording --- >> Workload PIDs are 304377 and 304378 >> perf PID is 304389 >> Waiting for "perf record has started" message >> OK >> pid 0 cpu -1 fd 5 : sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 5 >> pid 0 cpu -1 fd 6 : sys_perf_event_open: pid 0 cpu -1 group_fd -1 flags 0x8 = 6 >> pid 304377 cpu -1 fd 7 : sys_perf_event_open: pid 304377 cpu -1 group_fd -1 flags 0x8 = 7 >> pid 304380 cpu -1 fd 8 : sys_perf_event_open: pid 304380 cpu -1 group_fd -1 flags 0x8 = 8 >> pid 304378 cpu -1 fd 9 : sys_perf_event_open: pid 304378 cpu -1 group_fd -1 flags 0x8 = 9 >> pid 304381 cpu -1 fd 10 : sys_perf_event_open: pid 304381 cpu -1 group_fd -1 flags 0x8 = 10 >> pid 304377 cpu -1 fd 11 : sys_perf_event_open: pid 304377 cpu -1 group_fd -1 flags 0x8 = 11 >> pid 304380 cpu -1 fd 12 : sys_perf_event_open: pid 304380 cpu -1 group_fd -1 flags 0x8 = 12 >> pid 304378 cpu -1 fd 13 : sys_perf_event_open: pid 304378 cpu -1 group_fd -1 flags 0x8 = 13 >> pid 304381 cpu -1 fd 14 : sys_perf_event_open: pid 304381 cpu -1 group_fd -1 flags 0x8 = 14 >> fd 7 : idx 0: mmapping fd 7 >> fd 11 fd_to 7 : idx 0: set output fd 11 -> 7 >> fd 8 : idx 1: mmapping fd 8 >> fd 12 fd_to 8 : idx 1: set output fd 12 -> 8 >> fd 9 : idx 2: mmapping fd 9 >> fd 13 fd_to 9 : idx 2: set output fd 13 -> 9 >> fd 10 : idx 3: mmapping fd 10 >> fd 14 fd_to 10 : idx 3: set output fd 14 -> 10 >> Checking 10 fds >> No mmap for fd 5 > > Thanks Arnaldo, so the reverted change is: > ``` > --- a/tools/perf/util/pmus.c > +++ b/tools/perf/util/pmus.c > @@ -4,6 +4,7 @@ > #include <subcmd/pager.h> > #include <sys/types.h> > #include <dirent.h> > +#include <pthread.h> > #include <string.h> > #include <unistd.h> > #include "debug.h" > @@ -492,9 +493,35 @@ int perf_pmus__num_core_pmus(void) > return count; > } > > +static bool __perf_pmus__supports_extended_type(void) > +{ > + struct perf_pmu *pmu = NULL; > + > + if (perf_pmus__num_core_pmus() <= 1) > + return false; > + > + while ((pmu = perf_pmus__scan_core(pmu)) != NULL) { > + if (!is_event_supported(PERF_TYPE_HARDWARE, > PERF_COUNT_HW_CPU_CYCLES | ((__u64)pmu-> > type << PERF_PMU_TYPE_SHIFT))) > + return false; > + } > + > + return true; > +} > + > +static bool perf_pmus__do_support_extended_type; > + > +static void perf_pmus__init_supports_extended_type(void) > +{ > + perf_pmus__do_support_extended_type = > __perf_pmus__supports_extended_type(); > +} > + > bool perf_pmus__supports_extended_type(void) > { > - return perf_pmus__num_core_pmus() > 1; > + static pthread_once_t extended_type_once = PTHREAD_ONCE_INIT; > + > + pthread_once(&extended_type_once, > perf_pmus__init_supports_extended_type); > + > + return perf_pmus__do_support_extended_type; > } > > struct perf_pmu *evsel__find_pmu(const struct evsel *evsel) > ``` > On your Intel this should have previously returned true as > "perf_pmus__num_core_pmus() > 1", and with the new code presumably > is_event_supported is returning false. Could you dump the PMU's name > at that point? Is cpu_core or cpu_atom looking like it doesn't support > the event? Is the test failing when run as root (ie is > is_event_supported failing to have expected fallback paths)?
Problem is the test scrapes debug output and is_event_supported() prints out debug that confuses the test. Other probe functions like in perf_api_probe.c use sys_perf_event_open() so the issue has not arisen before.
| |