Messages in this thread | | | Date | Fri, 20 May 2022 10:50:45 +0530 | Subject | Re: [PATCH v3 2/5] perf header: Parse non-cpu pmu capabilities | From | Ravi Bangoria <> |
| |
On 20-May-22 10:01 AM, Ian Rogers wrote: > On Thu, May 19, 2022 at 8:49 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote: >> >> Hi Ian, >> >> On 20-May-22 3:57 AM, Ian Rogers wrote: >>> On Wed, May 18, 2022 at 10:45 PM Ravi Bangoria <ravi.bangoria@amd.com> wrote: >>>> >>>> Pmus advertise their capabilities via sysfs attribute files but >>>> perf tool currently parses only core(cpu) pmu capabilities. Add >>>> support for parsing non-cpu pmu capabilities. >>>> >>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com> >>>> --- >>>> .../Documentation/perf.data-file-format.txt | 18 ++ >>>> tools/perf/util/env.c | 48 +++++ >>>> tools/perf/util/env.h | 11 + >>>> tools/perf/util/header.c | 198 ++++++++++++++++++ >>>> tools/perf/util/header.h | 1 + >>>> tools/perf/util/pmu.c | 15 +- >>>> tools/perf/util/pmu.h | 2 + >>>> 7 files changed, 289 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt >>>> index f56d0e0fbff6..7f8341db9134 100644 >>>> --- a/tools/perf/Documentation/perf.data-file-format.txt >>>> +++ b/tools/perf/Documentation/perf.data-file-format.txt >>>> @@ -435,6 +435,24 @@ struct { >>>> } [nr_pmu]; >>>> }; >>>> >>>> + HEADER_PMU_CAPS = 32, >>>> + >>>> + List of pmu capabilities (except cpu pmu which is already >>>> + covered by HEADER_CPU_PMU_CAPS) >>> >>> Sorry for the ignorance, is this currently broken for hybrid then? >>> Will hybrid have a HEADER_CPU_PMU_CAPS? Presumably this varies between >>> ARM's big.little and Alderlake. >> >> It's covered by HEADER_HYBRID_CPU_PMU_CAPS, but that too covers only >> cpu pmu. I think I should update the above comment to: >> >> List of pmu capabilities (except cpu pmu which is already >> covered by HEADER_CPU_PMU_CAPS / HEADER_HYBRID_CPU_PMU_CAPS) >> >>>> + >>>> +struct { >>>> + u32 nr_pmus; >>>> + struct { >>>> + u32 core_type; /* For hybrid topology */ >>> >>> Could this be pmu_type as presumably we can have capabilities on any >>> kind of PMU? >> >> Not sure I follow that question but let me just put my thoughts here. >> >> {core_type, pmu_name} is the unique key here. Considering a hypothetical >> scenario: A system has two types of cores P-core and E-core. Certain pmu >> inside P-core has some capabilities which are missing in the identical >> pmu belonging to E-core. The header will look something like: >> >> struct { >> .nr_pmus = 2, >> [0] = struct { >> .core_type = 0, /* P-core */ >> .pmu_name = xyz_pmu, >> .nr_caps = 2, >> [0] = { .name = cap1, .value = value1 }, >> [1] = { .name = cap2, .value = value2 }, >> }, >> [1] = struct { >> .core_type = 1; /* E-core */ >> .pmu_name = xyz_pmu; >> .nr_caps = 1; >> [0] = { .name = cap1, .value = value1 }; >> }, >> }; >> >> Does that answer your question? >> >> Thanks for the review, >> Ravi > > I may be being a little ahead of the current code as I'm wondering > about heterogeneous systems with many non-CPU PMUs. It seems such a > scenario just wouldn't touch the core_type field here. Could the p or > e core-ness of a PMU be implied by the name?
Using just pmu_name to identify the type of core it belongs to; yeah that might work assuming perf_pmu_register() doesn't allow registering multiple pmus with the same name. I'll remove 'core_type'.
> Is there something similar to core_type in sysfs
I don't think so. I don't have any Intel ADL or ARM big.LITTLE system to try. But that's not required now.
Thanks, Ravi
| |