Messages in this thread | | | Date | Wed, 10 Apr 2024 13:47:23 +0300 | Subject | Re: BISECTED: perf test 'Miscellaneous Intel PT' failing on Intel hybrid machines | From | Adrian Hunter <> |
| |
On 9/04/24 22:05, Adrian Hunter wrote: > 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.
Patch here:
https://lore.kernel.org/all/20240410104450.15602-1-adrian.hunter@intel.com/
| |