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 <[email protected]>
> 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
On Tue, Apr 9, 2024 at 8:34 AM Arnaldo Carvalho de Melo <[email protected]> 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 <[email protected]>
> > 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)?
Thanks,
Ian
On 9/04/24 18:46, Ian Rogers wrote:
> On Tue, Apr 9, 2024 at 8:34 AM Arnaldo Carvalho de Melo <[email protected]> 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 <[email protected]>
>>> 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.
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 <[email protected]> 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 <[email protected]>
>>>> 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/[email protected]/