Skip to contentSkip to conversation

e2641db83f: perf-sanity-tests.perf_all_PMU_test.fail

linux-next:master kernel test robot 3 participants 9 messages Jul 10, 2024 – Jul 16, 2024

Subsystems (2)
lore
Share & tools
lore

kernel test robot

author build bot
#0 lore raw

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240710/[email protected]

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
#1 lore raw
Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 8 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

Can you please try the -vvv option, which should tell the failed event? perf test -vvv "perf all PMU test"

Thanks,
Kan

Quoted 8 lines

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240710/[email protected]

#2 lore raw

hi, Kan,

Quoted 60 lines · Kan
Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 8 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

Quoted 2 lines

Can you please try the -vvv option, which should tell the failed event? perf test -vvv "perf all PMU test"

after add -vvv:

2024-07-11 07:45:20 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test -v 108

108: perf all PMU test:
--- start ---
test child forked, pid 18989
Testing (null)
---- end(-1) ----
108: perf all PMU test : FAILED!

Quoted 12 lines

Thanks,
Kan

Quoted 8 lines

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240710/[email protected]

#3 lore raw
Quoted 66 lines · Oliver Sang

hi, Kan,

Quoted 60 lines · Kan
Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 8 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

Quoted 2 lines

Can you please try the -vvv option, which should tell the failed event? perf test -vvv "perf all PMU test"

Quoted 7 lines

after add -vvv:

2024-07-11 07:45:20 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test -v 108

108: perf all PMU test:
--- start ---
test child forked, pid 18989
Testing (null)

The test cases use the "perf list --raw-dump pmu" to get a complete event list. It seems the perf list command got nothing. That's weird.

The main change of e2641db83f18 is to add the "Counter" field and a few events. It should not impact the perf list command.

What's the output of "perf list --raw-dump pmu" on your machine?

Thanks,
Kan

Quoted 17 lines

---- end(-1) ----
108: perf all PMU test : FAILED!

Quoted 12 lines

Thanks,
Kan

Quoted 8 lines

The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240710/[email protected]

#4 lore raw
In reply to kernel test robot

Hi Ian,

Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 9 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The failure is caused by the below change in the e2641db83f18.

+ {
+ "BriefDescription": "This 48-bit fixed counter counts the UCLK
cycles",

+        "Counter": "FIXED",
+        "EventCode": "0xff",
+        "EventName": "UNC_CLOCK.SOCKET",
+        "PerPkg": "1",
+        "PublicDescription": "This 48-bit fixed counter counts the UCLK
cycles.",
+        "Unit": "cbox_0"
     }

The other cbox events have the unit name "CBOX", while the fixed counter has a unit name "cbox_0". So the events_table will maintain separate entries for cbox and cbox_0.

The perf_pmus__print_pmu_events() calculate the total number of events, allocate an aliases buffer, store all the events into the buffer, sort, and print all the aliases one by one.

The problem is that the calculated total number of events doesn't match the stored events on the SKL machine.

The perf_pmu__num_events() is used to calculate the number of events. It invokes the pmu_events_table__num_events() to go through the entire events_table to find all events. Because of the pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So the events for cbox and cbox_0 are all counted.

When storing events into the aliases buffer, the perf_pmu__for_each_event() only process the events for cbox.

Since a bigger buffer was allocated, the last entry are all 0. When printing all the aliases, null will be outputed.

$ perf list pmu

List of pre-defined events (to be used in -e or -M):

  (null)                                             [Kernel PMU event]
  branch-instructions OR cpu/branch-instructions/    [Kernel PMU event]
  branch-misses OR cpu/branch-misses/                [Kernel PMU event]

I'm thinking of two ways to address it.
One is to only print all the stored events. The below patch can fix it.

tools/perf/util/pmus.c +0 -0
index 3fcabfd8fca1..2b2f5117ff84 100644
@@ -485,6 +485,7 @@ void perf_pmus__print_pmu_events(const struct

print_callbacks *print_cb, void *p
perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
perf_pmus__print_pmu_events__callback);
}
+ len = state.index;
qsort(aliases, len, sizeof(struct sevent), cmp_sevent); for (int j = 0; j < len; j++) {
/* Skip duplicates */

The only drawback is that perf list will not show the new cbox_0 event. (But the event name still works. Users can still apply perf stat -e unc_clock.socket.)

Since the cbox_0 event is only available on old machines (SKL and earlier), people should already use the equivalent kernel event. It doesn't sounds a big issue for me. I prefer this simple fix.

I think the other way would be to modify the perf_pmu__for_each_event() to go through all the possible PMUs.
It seems complicated and may impact others ARCHs (e.g., S390). I haven't tried it yet.

What do you think?
Do you see any other ways to address the issue?

Thanks,
Kan

reviewer
#5 lore raw
Quoted 133 lines · Kan

Hi Ian,

Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 9 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The failure is caused by the below change in the e2641db83f18.

(quoted diff) +2 -0
+    {
+        "BriefDescription": "This 48-bit fixed counter counts the UCLK

cycles",

(quoted diff) +5 -0
+        "Counter": "FIXED",
+        "EventCode": "0xff",
+        "EventName": "UNC_CLOCK.SOCKET",
+        "PerPkg": "1",
+        "PublicDescription": "This 48-bit fixed counter counts the UCLK

cycles.",
+ "Unit": "cbox_0"

     }

The other cbox events have the unit name "CBOX", while the fixed counter has a unit name "cbox_0". So the events_table will maintain separate entries for cbox and cbox_0.

The perf_pmus__print_pmu_events() calculate the total number of events, allocate an aliases buffer, store all the events into the buffer, sort, and print all the aliases one by one.

The problem is that the calculated total number of events doesn't match the stored events on the SKL machine.

The perf_pmu__num_events() is used to calculate the number of events. It invokes the pmu_events_table__num_events() to go through the entire events_table to find all events. Because of the pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So the events for cbox and cbox_0 are all counted.

When storing events into the aliases buffer, the perf_pmu__for_each_event() only process the events for cbox.

Since a bigger buffer was allocated, the last entry are all 0. When printing all the aliases, null will be outputed.

$ perf list pmu

List of pre-defined events (to be used in -e or -M):

  (null)                                             [Kernel PMU event]
  branch-instructions OR cpu/branch-instructions/    [Kernel PMU event]
  branch-misses OR cpu/branch-misses/                [Kernel PMU event]

I'm thinking of two ways to address it.
One is to only print all the stored events. The below patch can fix it.

tools/perf/util/pmus.c +0 -0
index 3fcabfd8fca1..2b2f5117ff84 100644
@@ -485,6 +485,7 @@ void perf_pmus__print_pmu_events(const struct

print_callbacks *print_cb, void *p

                perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
                                        perf_pmus__print_pmu_events__callback);
        }
+       len = state.index;
        qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
        for (int j = 0; j < len; j++) {
                /* Skip duplicates */

The only drawback is that perf list will not show the new cbox_0 event. (But the event name still works. Users can still apply perf stat -e unc_clock.socket.)

Since the cbox_0 event is only available on old machines (SKL and earlier), people should already use the equivalent kernel event. It doesn't sounds a big issue for me. I prefer this simple fix.

Quoted 7 lines

I think the other way would be to modify the perf_pmu__for_each_event() to go through all the possible PMUs.
It seems complicated and may impact others ARCHs (e.g., S390). I haven't tried it yet.

What do you think?
Do you see any other ways to address the issue?

Ugh. It seems the sizing and then iterating approach is just prone to keep breaking. Perhaps we can switch to realloc-ed arrays to avoid the need for perf_pmu__num_events, which seems to be the source of the problems.

Thanks,
Ian

#6 lore raw
Quoted 139 lines · Ian Rogers
Quoted 138 lines · Kan

Hi Ian,

Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 9 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The failure is caused by the below change in the e2641db83f18.

(quoted diff) +2 -0
+    {
+        "BriefDescription": "This 48-bit fixed counter counts the UCLK

cycles",

(quoted diff) +5 -0
+        "Counter": "FIXED",
+        "EventCode": "0xff",
+        "EventName": "UNC_CLOCK.SOCKET",
+        "PerPkg": "1",
+        "PublicDescription": "This 48-bit fixed counter counts the UCLK

cycles.",
+ "Unit": "cbox_0"

     }

The other cbox events have the unit name "CBOX", while the fixed counter has a unit name "cbox_0". So the events_table will maintain separate entries for cbox and cbox_0.

The perf_pmus__print_pmu_events() calculate the total number of events, allocate an aliases buffer, store all the events into the buffer, sort, and print all the aliases one by one.

The problem is that the calculated total number of events doesn't match the stored events on the SKL machine.

The perf_pmu__num_events() is used to calculate the number of events. It invokes the pmu_events_table__num_events() to go through the entire events_table to find all events. Because of the pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So the events for cbox and cbox_0 are all counted.

When storing events into the aliases buffer, the perf_pmu__for_each_event() only process the events for cbox.

Since a bigger buffer was allocated, the last entry are all 0. When printing all the aliases, null will be outputed.

$ perf list pmu

List of pre-defined events (to be used in -e or -M):

  (null)                                             [Kernel PMU event]
  branch-instructions OR cpu/branch-instructions/    [Kernel PMU event]
  branch-misses OR cpu/branch-misses/                [Kernel PMU event]

I'm thinking of two ways to address it.
One is to only print all the stored events. The below patch can fix it.

tools/perf/util/pmus.c +0 -0
index 3fcabfd8fca1..2b2f5117ff84 100644
@@ -485,6 +485,7 @@ void perf_pmus__print_pmu_events(const struct

print_callbacks *print_cb, void *p

                perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
                                        perf_pmus__print_pmu_events__callback);
        }
+       len = state.index;
        qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
        for (int j = 0; j < len; j++) {
                /* Skip duplicates */

The only drawback is that perf list will not show the new cbox_0 event. (But the event name still works. Users can still apply perf stat -e unc_clock.socket.)

Since the cbox_0 event is only available on old machines (SKL and earlier), people should already use the equivalent kernel event. It doesn't sounds a big issue for me. I prefer this simple fix.

I think the other way would be to modify the perf_pmu__for_each_event() to go through all the possible PMUs.
It seems complicated and may impact others ARCHs (e.g., S390). I haven't tried it yet.

Quoted 8 lines
Quoted 2 lines

What do you think?
Do you see any other ways to address the issue?

Ugh. It seems the sizing and then iterating approach is just prone to keep breaking. Perhaps we can switch to realloc-ed arrays to avoid the need for perf_pmu__num_events, which seems to be the source of the problems.

I think a realloc-ed array should have the same drawback as the first way, but bad performance.
Because the pmu_add_cpu_aliases() in the perf_pmu__for_each_event() only add the events from the first matched PMU. If we don't fix it, the UNC_CLOCK.SOCKET of cbox_0 will never be displayed.

Thanks,
Kan

reviewer
#7 lore raw
Quoted 152 lines · Kan
Quoted 139 lines · Ian Rogers
Quoted 138 lines · Kan

Hi Ian,

Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 9 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The failure is caused by the below change in the e2641db83f18.

(quoted diff) +2 -0
+    {
+        "BriefDescription": "This 48-bit fixed counter counts the UCLK

cycles",

(quoted diff) +5 -0
+        "Counter": "FIXED",
+        "EventCode": "0xff",
+        "EventName": "UNC_CLOCK.SOCKET",
+        "PerPkg": "1",
+        "PublicDescription": "This 48-bit fixed counter counts the UCLK

cycles.",
+ "Unit": "cbox_0"

     }

The other cbox events have the unit name "CBOX", while the fixed counter has a unit name "cbox_0". So the events_table will maintain separate entries for cbox and cbox_0.

The perf_pmus__print_pmu_events() calculate the total number of events, allocate an aliases buffer, store all the events into the buffer, sort, and print all the aliases one by one.

The problem is that the calculated total number of events doesn't match the stored events on the SKL machine.

The perf_pmu__num_events() is used to calculate the number of events. It invokes the pmu_events_table__num_events() to go through the entire events_table to find all events. Because of the pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So the events for cbox and cbox_0 are all counted.

When storing events into the aliases buffer, the perf_pmu__for_each_event() only process the events for cbox.

Since a bigger buffer was allocated, the last entry are all 0. When printing all the aliases, null will be outputed.

$ perf list pmu

List of pre-defined events (to be used in -e or -M):

  (null)                                             [Kernel PMU event]
  branch-instructions OR cpu/branch-instructions/    [Kernel PMU event]
  branch-misses OR cpu/branch-misses/                [Kernel PMU event]

I'm thinking of two ways to address it.
One is to only print all the stored events. The below patch can fix it.

tools/perf/util/pmus.c +0 -0
index 3fcabfd8fca1..2b2f5117ff84 100644
@@ -485,6 +485,7 @@ void perf_pmus__print_pmu_events(const struct

print_callbacks *print_cb, void *p

                perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
                                        perf_pmus__print_pmu_events__callback);
        }
+       len = state.index;
        qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
        for (int j = 0; j < len; j++) {
                /* Skip duplicates */

The only drawback is that perf list will not show the new cbox_0 event. (But the event name still works. Users can still apply perf stat -e unc_clock.socket.)

Since the cbox_0 event is only available on old machines (SKL and earlier), people should already use the equivalent kernel event. It doesn't sounds a big issue for me. I prefer this simple fix.

I think the other way would be to modify the perf_pmu__for_each_event() to go through all the possible PMUs.
It seems complicated and may impact others ARCHs (e.g., S390). I haven't tried it yet.

Quoted 8 lines
Quoted 2 lines

What do you think?
Do you see any other ways to address the issue?

Ugh. It seems the sizing and then iterating approach is just prone to keep breaking. Perhaps we can switch to realloc-ed arrays to avoid the need for perf_pmu__num_events, which seems to be the source of the problems.

Quoted 5 lines

I think a realloc-ed array should have the same drawback as the first way, but bad performance.
Because the pmu_add_cpu_aliases() in the perf_pmu__for_each_event() only add the events from the first matched PMU. If we don't fix it, the UNC_CLOCK.SOCKET of cbox_0 will never be displayed.

Ok, but I don't think we need to optimize perf list for speed. Fwiw, I think this was the fix for the last bug in this code:
https://lore.kernel.org/r/[email protected]

Thanks,
Ian

#8 lore raw
Quoted 159 lines · Ian Rogers
Quoted 152 lines · Kan
Quoted 139 lines · Ian Rogers
Quoted 138 lines · Kan

Hi Ian,

Quoted 48 lines · kernel test robot

Hello,

kernel test robot noticed "perf-sanity-tests.perf_all_PMU_test.fail" on:

commit: e2641db83f18782f57a0e107c50d2d1731960fb8 ("perf vendor events: Add/update skylake events/metrics")
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git master

[test failed on linux-next/master 82d01fe6ee52086035b201cfa1410a3b04384257]

in testcase: perf-sanity-tests
version:
with following parameters:

perf_compiler: gcc

compiler: gcc-13
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)

we also observed two cases which also failed on parent can pass on this commit. FYI.

caccae3ce7b988b6 e2641db83f18782f57a0e107c50
---------------- ---------------------------
fail:runs %reproduction fail:runs
| | |
:6 100% 6:6 perf-sanity-tests.perf_all_PMU_test.fail
:6 100% 6:6 perf-sanity-tests.perf_all_metricgroups_test.pass
:6 100% 6:6 perf-sanity-tests.perf_all_metrics_test.pass

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-lkp/[email protected]

Quoted 9 lines
2024-07-09 07:09:53 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 105
105: perf all metricgroups test                                      : Ok
2024-07-09 07:10:11 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 106
106: perf all metrics test                                           : Ok
2024-07-09 07:10:23 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 107
107: perf all libpfm4 events test                                    : Ok
2024-07-09 07:10:47 sudo /usr/src/linux-perf-x86_64-rhel-8.3-bpf-e2641db83f18782f57a0e107c50d2d1731960fb8/tools/perf/perf test 108

108: perf all PMU test : FAILED!

The failure is caused by the below change in the e2641db83f18.

(quoted diff) +2 -0
+    {
+        "BriefDescription": "This 48-bit fixed counter counts the UCLK

cycles",

(quoted diff) +5 -0
+        "Counter": "FIXED",
+        "EventCode": "0xff",
+        "EventName": "UNC_CLOCK.SOCKET",
+        "PerPkg": "1",
+        "PublicDescription": "This 48-bit fixed counter counts the UCLK

cycles.",
+ "Unit": "cbox_0"

     }

The other cbox events have the unit name "CBOX", while the fixed counter has a unit name "cbox_0". So the events_table will maintain separate entries for cbox and cbox_0.

The perf_pmus__print_pmu_events() calculate the total number of events, allocate an aliases buffer, store all the events into the buffer, sort, and print all the aliases one by one.

The problem is that the calculated total number of events doesn't match the stored events on the SKL machine.

The perf_pmu__num_events() is used to calculate the number of events. It invokes the pmu_events_table__num_events() to go through the entire events_table to find all events. Because of the pmu_uncore_alias_match(), the suffix of uncore PMU will be ignored. So the events for cbox and cbox_0 are all counted.

When storing events into the aliases buffer, the perf_pmu__for_each_event() only process the events for cbox.

Since a bigger buffer was allocated, the last entry are all 0. When printing all the aliases, null will be outputed.

$ perf list pmu

List of pre-defined events (to be used in -e or -M):

  (null)                                             [Kernel PMU event]
  branch-instructions OR cpu/branch-instructions/    [Kernel PMU event]
  branch-misses OR cpu/branch-misses/                [Kernel PMU event]

I'm thinking of two ways to address it.
One is to only print all the stored events. The below patch can fix it.

tools/perf/util/pmus.c +0 -0
index 3fcabfd8fca1..2b2f5117ff84 100644
@@ -485,6 +485,7 @@ void perf_pmus__print_pmu_events(const struct

print_callbacks *print_cb, void *p

                perf_pmu__for_each_event(pmu, skip_duplicate_pmus, &state,
                                        perf_pmus__print_pmu_events__callback);
        }
+       len = state.index;
        qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
        for (int j = 0; j < len; j++) {
                /* Skip duplicates */

The only drawback is that perf list will not show the new cbox_0 event. (But the event name still works. Users can still apply perf stat -e unc_clock.socket.)

Since the cbox_0 event is only available on old machines (SKL and earlier), people should already use the equivalent kernel event. It doesn't sounds a big issue for me. I prefer this simple fix.

I think the other way would be to modify the perf_pmu__for_each_event() to go through all the possible PMUs.
It seems complicated and may impact others ARCHs (e.g., S390). I haven't tried it yet.

Quoted 8 lines
Quoted 2 lines

What do you think?
Do you see any other ways to address the issue?

Ugh. It seems the sizing and then iterating approach is just prone to keep breaking. Perhaps we can switch to realloc-ed arrays to avoid the need for perf_pmu__num_events, which seems to be the source of the problems.

Quoted 5 lines

I think a realloc-ed array should have the same drawback as the first way, but bad performance.
Because the pmu_add_cpu_aliases() in the perf_pmu__for_each_event() only add the events from the first matched PMU. If we don't fix it, the UNC_CLOCK.SOCKET of cbox_0 will never be displayed.

Quoted 3 lines

Ok, but I don't think we need to optimize perf list for speed. Fwiw, I think this was the fix for the last bug in this code:
https://lore.kernel.org/r/[email protected]

That seems a different issue.
The root cause of this issue should be that different methods are used between calculating the total number of events and adding the events. A realloc-ed array should just hide the problem. There will still be an issue if anyone use the pair of perf_pmu__num_events() and pmu_add_cpu_aliases() somewhere else later.

It looks like the difference is introduced from the commit e3edd6cf6399
("perf pmu-events: Reduce processed events by passing PMU").

The pmu_events_table__for_each_event() stops immediately once a pmu is set. But for uncore, especially this case, the method is wrong and mismatch what perf does in the perf_pmu__num_events().

The below patch can fix it.

(quoted diff) +0 -0

b/tools/perf/pmu-events/jevents.py
index ac9b7ca41856..97a3b018f865 100755

tools/perf/pmu-events/jevents.py +0 -0
@@ -923,7 +923,7 @@ int pmu_events_table__for_each_event(const struct

pmu_events_table *table,
continue;

ret = pmu_events_table__for_each_event_pmu(table, table_pmu, fn, data);
- if (pmu || ret)
+ if (ret)
return ret;
}
return 0;

The event can be displayed by the perf list pmu with the patch.

$ perf list pmu | grep -A 1 clock.socket
   unc_clock.socket
        [This 48-bit fixed counter counts the UCLK cycles. Unit:
uncore_cbox_0
& perf test "perf all PMU test"
  107: perf all PMU test                                               : Ok

I will send the patch shortly.

Thanks,
Kan