> On 13 Nov 2024, at 6:49 AM, Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> At the RISC-V summit the topic of avoiding event data being in the
> RISC-V PMU kernel driver came up. There is a preference for sysfs/JSON
> events being the priority when no PMU is provided so that legacy
> events maybe supported via json. Originally Mark Rutland also
> expressed at LPC 2023 that doing this would resolve bugs on ARM Apple
> M? processors, but James Clark more recently tested this and believes
> the driver issues there may not have existed or have been resolved. In
> any case, it is inconsistent that with a PMU event names avoid legacy
> encodings, but when wildcarding PMUs (ie without a PMU with the event
> name) the legacy encodings have priority.
>
> The patch doing this work was reverted in a v6.10 release candidate
> as, even though the patch was posted for weeks and had been on
> linux-next for weeks without issue, Linus was in the habit of using
> explicit legacy events with unsupported precision options on his
> Neoverse-N1. This machine has SLC PMU events for bus and CPU cycles
> where ARM decided to call the events bus_cycles and cycles, the latter
> being also a legacy event name. ARM haven't renamed the cycles event
> to a more consistent cpu_cycles and avoided the problem. With these
> changes the problematic event will now be skipped, a large warning
> produced, and perf record will continue for the other PMU events. This
> solution was proposed by Arnaldo.
>
> Two minor changes have been added to help with the error message and
> to work around issues occurring with "perf stat metrics (shadow stat)
> test".
>
> The patches have only been tested on my x86 non-hybrid laptop.
Hi,
After applying this patch series,we observed a regression while running the perf test suite on powerpc system. Specifically, test case for "Check branch stack sampling" now fails.
Upon investigation, identified that patch "perf record: Skip don't fail for events that don't open" is causing the breakage. This test case uses branch-filter as "save_type" and it is supposed to be skipped in powerpc.
Snippet of code:
skip the test if the hardware doesn't support branch stack sampling
and if the architecture doesn't support filter types: any,save_type,u
if ! perf record -o- --no-buildid --branch-filter any,save_type,u -- true > /dev/null 2>&1 ; then
echo "skip: system doesn't support filter types: any,save_type,u"
exit 2
fi
Before applying the patch, running the command:
./perf record -o- --no-buildid --branch-filter any,save_type,u -- true
cycles:PH: PMU Hardware or event type doesn't support branch stack sampling.
# echo $?
255
would return 255 (indicating not supported) with the error.
After applying the patch, the same command now returns 0, which is incorrect. The output is as follows:
# ./perf record -o- --no-buildid --branch-filter any,save_type,u -- true
Lowering default frequency rate from 4000 to 2000.
Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
Error:
Failure to open event 'cycles:PH' on PMU 'cpu' which will be removed.
cycles:PH: PMU Hardware or event type doesn't support branch stack sampling.
libperf: Miscounted nr_mmaps 0 vs 8
WARNING: No sample_id_all support, falling back to unordered processing
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.008 MB - ]
# echo $?
0
Also there were some junk result in the output which I have skipped in above result. The patch appears to alter behavior such that the unsupported or failed event open still proceeds and leading to this.
Ian ,
Is this behaviour expected ?
Thank you,
Aditya
>
> v2: Rebase and add tested-by tags from James Clark, Leo Yan and Atish
> Patra who have tested on RISC-V and ARM CPUs, including the
> problem case from before.
>
> Ian Rogers (4):
> perf evsel: Add pmu_name helper
> perf stat: Fix find_stat for mixed legacy/non-legacy events
> perf record: Skip don't fail for events that don't open
> perf parse-events: Reapply "Prefer sysfs/JSON hardware events over
> legacy"
>
> tools/perf/builtin-record.c | 22 +++++++---
> tools/perf/util/evsel.c | 10 +++++
> tools/perf/util/evsel.h | 1 +
> tools/perf/util/parse-events.c | 26 +++++++++---
> tools/perf/util/parse-events.l | 76 +++++++++++++++++-----------------
> tools/perf/util/parse-events.y | 60 ++++++++++++++++++---------
> tools/perf/util/pmus.c | 20 +++++++--
> tools/perf/util/stat-shadow.c | 3 +-
> 8 files changed, 145 insertions(+), 73 deletions(-)
>
> --
> 2.47.0.277.g8800431eea-goog
>
>
