tools/perf/tests/pmu.c | 95 ++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/pmu.c | 4 +- tools/perf/util/pmus.c | 53 ++++++++++++----------- tools/perf/util/pmus.h | 7 +++- 4 files changed, 131 insertions(+), 28 deletions(-)
The mrvl_ddr_pmu is uncore and has a hexadecimal address suffix. Current PMU sorting/merging code assumes uncore PMU names start with uncore_ and have a decimal suffix. Add support for hexadecimal suffixes and add tests. v3. Rebase and move tests from pmus.c to the existing pmu.c. Ian Rogers (2): perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu perf tests: Add some pmu core functionality tests tools/perf/tests/pmu.c | 95 ++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/pmu.c | 4 +- tools/perf/util/pmus.c | 53 ++++++++++++----------- tools/perf/util/pmus.h | 7 +++- 4 files changed, 131 insertions(+), 28 deletions(-) -- 2.45.0.rc1.225.g2a3ae87e7f-goog
On Sat, May 4, 2024 at 8:16 PM Ian Rogers <irogers@google.com> wrote: > > The mrvl_ddr_pmu is uncore and has a hexadecimal address > suffix. Current PMU sorting/merging code assumes uncore PMU names > start with uncore_ and have a decimal suffix. Add support for > hexadecimal suffixes and add tests. > > v3. Rebase and move tests from pmus.c to the existing pmu.c. > > Ian Rogers (2): > perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu > perf tests: Add some pmu core functionality tests Hi, these patches have been hanging around since March [1], it would be nice to either be landing them or getting feedback on what to fix. Thanks, Ian [1] https://lore.kernel.org/lkml/20240329064803.3058900-1-irogers@google.com/ > tools/perf/tests/pmu.c | 95 ++++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/pmu.c | 4 +- > tools/perf/util/pmus.c | 53 ++++++++++++----------- > tools/perf/util/pmus.h | 7 +++- > 4 files changed, 131 insertions(+), 28 deletions(-) > > -- > 2.45.0.rc1.225.g2a3ae87e7f-goog >
On Fri, May 10, 2024 at 11:13:36AM -0700, Ian Rogers wrote: > On Sat, May 4, 2024 at 8:16 PM Ian Rogers <irogers@google.com> wrote: > > > > The mrvl_ddr_pmu is uncore and has a hexadecimal address > > suffix. Current PMU sorting/merging code assumes uncore PMU names > > start with uncore_ and have a decimal suffix. Add support for > > hexadecimal suffixes and add tests. > > > > v3. Rebase and move tests from pmus.c to the existing pmu.c. > > > > Ian Rogers (2): > > perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu > > perf tests: Add some pmu core functionality tests > > Hi, these patches have been hanging around since March [1], it would > be nice to either be landing them or getting feedback on what to fix. Thanks, applied to perf-tools-next, - Arnaldo > Thanks, > Ian > > [1] https://lore.kernel.org/lkml/20240329064803.3058900-1-irogers@google.com/ > > > tools/perf/tests/pmu.c | 95 ++++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/pmu.c | 4 +- > > tools/perf/util/pmus.c | 53 ++++++++++++----------- > > tools/perf/util/pmus.h | 7 +++- > > 4 files changed, 131 insertions(+), 28 deletions(-) > > > > -- > > 2.45.0.rc1.225.g2a3ae87e7f-goog > >
On Fri, May 10, 2024 at 11:13 AM Ian Rogers <irogers@google.com> wrote: > > On Sat, May 4, 2024 at 8:16 PM Ian Rogers <irogers@google.com> wrote: > > > > The mrvl_ddr_pmu is uncore and has a hexadecimal address > > suffix. Current PMU sorting/merging code assumes uncore PMU names > > start with uncore_ and have a decimal suffix. Add support for > > hexadecimal suffixes and add tests. > > > > v3. Rebase and move tests from pmus.c to the existing pmu.c. > > > > Ian Rogers (2): > > perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu > > perf tests: Add some pmu core functionality tests > > Hi, these patches have been hanging around since March [1], it would > be nice to either be landing them or getting feedback on what to fix. > > Thanks, > Ian > > [1] https://lore.kernel.org/lkml/20240329064803.3058900-1-irogers@google.com/ +Tuan Phan, Robin Murphy Here is another PMU with the same suffix convention/issue: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/drivers/perf/arm_dmc620_pmu.c?h=perf-tools-next#n706 Thanks, Ian > > tools/perf/tests/pmu.c | 95 ++++++++++++++++++++++++++++++++++++++++++ > > tools/perf/util/pmu.c | 4 +- > > tools/perf/util/pmus.c | 53 ++++++++++++----------- > > tools/perf/util/pmus.h | 7 +++- > > 4 files changed, 131 insertions(+), 28 deletions(-) > > > > -- > > 2.45.0.rc1.225.g2a3ae87e7f-goog > >
On 2024-05-10 8:15 pm, Ian Rogers wrote: > On Fri, May 10, 2024 at 11:13 AM Ian Rogers <irogers@google.com> wrote: >> >> On Sat, May 4, 2024 at 8:16 PM Ian Rogers <irogers@google.com> wrote: >>> >>> The mrvl_ddr_pmu is uncore and has a hexadecimal address >>> suffix. Current PMU sorting/merging code assumes uncore PMU names >>> start with uncore_ and have a decimal suffix. Add support for >>> hexadecimal suffixes and add tests. >>> >>> v3. Rebase and move tests from pmus.c to the existing pmu.c. >>> >>> Ian Rogers (2): >>> perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu >>> perf tests: Add some pmu core functionality tests >> >> Hi, these patches have been hanging around since March [1], it would >> be nice to either be landing them or getting feedback on what to fix. >> >> Thanks, >> Ian >> >> [1] https://lore.kernel.org/lkml/20240329064803.3058900-1-irogers@google.com/ > > +Tuan Phan, Robin Murphy > > Here is another PMU with the same suffix convention/issue: > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/drivers/perf/arm_dmc620_pmu.c?h=perf-tools-next#n706 There are at least one or two more as well - certainly arm_smmuv3_pmu which I think may have been where this pattern first started. Now that we've finally done the right thing with the parent mechanism to provide a user-visible relationship of PMU instances to their corresponding Devicetree/ACPI devices, hopefully we can discourage any further use of this rather clunky trick of using the MMIO address as an identifier in the PMU name. However there's then also stuff like dwc_pcie_pmu which encodes a PCI ID as a hex suffix, so understanding hex suffixes in general might still be a reasonable idea for the tool, if the alternative would be maintaining a list of specific prefixes (even if that would be hoped to remain fairly small). Thanks, Robin.
On Mon, May 13, 2024 at 7:35 AM Robin Murphy <robin.murphy@arm.com> wrote: > > On 2024-05-10 8:15 pm, Ian Rogers wrote: > > On Fri, May 10, 2024 at 11:13 AM Ian Rogers <irogers@google.com> wrote: > >> > >> On Sat, May 4, 2024 at 8:16 PM Ian Rogers <irogers@google.com> wrote: > >>> > >>> The mrvl_ddr_pmu is uncore and has a hexadecimal address > >>> suffix. Current PMU sorting/merging code assumes uncore PMU names > >>> start with uncore_ and have a decimal suffix. Add support for > >>> hexadecimal suffixes and add tests. > >>> > >>> v3. Rebase and move tests from pmus.c to the existing pmu.c. > >>> > >>> Ian Rogers (2): > >>> perf pmus: Sort/merge/aggregate PMUs like mrvl_ddr_pmu > >>> perf tests: Add some pmu core functionality tests > >> > >> Hi, these patches have been hanging around since March [1], it would > >> be nice to either be landing them or getting feedback on what to fix. > >> > >> Thanks, > >> Ian > >> > >> [1] https://lore.kernel.org/lkml/20240329064803.3058900-1-irogers@google.com/ > > > > +Tuan Phan, Robin Murphy > > > > Here is another PMU with the same suffix convention/issue: > > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/drivers/perf/arm_dmc620_pmu.c?h=perf-tools-next#n706 > > There are at least one or two more as well - certainly arm_smmuv3_pmu > which I think may have been where this pattern first started. Now that > we've finally done the right thing with the parent mechanism to provide > a user-visible relationship of PMU instances to their corresponding > Devicetree/ACPI devices, hopefully we can discourage any further use of > this rather clunky trick of using the MMIO address as an identifier in > the PMU name. However there's then also stuff like dwc_pcie_pmu which > encodes a PCI ID as a hex suffix, so understanding hex suffixes in > general might still be a reasonable idea for the tool, if the > alternative would be maintaining a list of specific prefixes (even if > that would be hoped to remain fairly small). Thanks Robin, the problem that Kan identified was that IBM s390 have a PMU called cpum_cf. So the _cf is a valid hex suffix. Maybe there needs to be a minimum hex suffix length to deal with this. We made the 0x optional in perf "raw" events like `perf stat -e 'r12abcd'...` and now we have the problem that the word `read` is both quite a desirable perf event name and also a valid hex encoding. We prefer the event name if it exists, but we have to make the code more complex to deal with this. It would be nice if we were capturing the conventions in: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/ABI/testing Like with the sysfs-bus-event_source-* files. I'll do a v4 of these changes and see if I can do something to document the suffixes, would be great to get an Acked-by/Tested-by. Thanks, Ian > Thanks, > Robin.
© 2016 - 2024 Red Hat, Inc.