Re: [PATCH RFC 4/9] perf jevents: Add sys_events_find_events_table()

[Ian Rogers]

On Thu, Jul 13, 2023 at 8:07 AM John Garry <john.g.garry@xxxxxxxxxx> wrote:
>
> On 12/07/2023 18:52, Ian Rogers wrote:
> >> To be crystal clear, when you say ">1 PMU", do you mean ">1 PMU instance
> >> of the same type" or ">1 PMU type"?
> > So I'm meaning that if you have a MetricExpr where the events are from
> >> 1 PMU, for example memory bandwidth coming from uncore PMUs and then
> > instructions from a core PMU, and you do something like a ratio
> > between these two.
> >
> >>    in a metric wouldn't work though as it would
> >>> appear the event was missing. Having the metric specify the PMU avoids
> >>> these problems, but is verbose.
> >> The message I'm getting - correct me if I am wrong - is that you would
> >> still prefer the PMU specified per event in metric expr, right? We don't
> >> do that exactly for sys PMU metrics today - we specify "Unit" instead, like:
> >>
> >> MetricExpr: "sys_pmu_bar_event_foo1 + sys_pmu_bar_event_foo2"
> >> Compat: "baz"
> >> Unit:"sys_pmu_bar"
> >>
> >> And so you prefer something like the following, right?
> >> MetricExpr: "sys_pmu_foo@bar1@ + sys_pmu_foo@bar2@"
> >>
> >> If so, I think that is ok - I just want to get rid of Unit and Compat.
> > I think we're agreeing 😄
>
> Ok, fine. I need to check on implementing support for that.
>
> Then would you have any idea for testing here?
>
> What I do is to ensure that if we 2x tables like following for separate
> SoCs:
>
> soc_a.json
>
>
> {
>                 "MetricExpr": "pmu_unit@event_foo@ * pmu_unit@event_bar@ * 1",
>                 "MetricName": "metric_baz",
> },
> {
>                 "EventCode": "0x84",
>                 "EventName": "event_foo ",
>                 "Compat": "0x00000030",
>                 "Unit": "pmu_unit"
> },
> {
>                 "EventCode": "0x85",
>                 "EventName": "event_bar ",
>                 "Compat": "0x00000030",
>                 "Unit": "pmu_unit"
> },
>
>
>
> soc_b.json
>
>
> {
>                 "MetricExpr": "pmu_unit@event_foo@ * pmu_unit@event_bar@ * 2",
>                 "MetricName": "metric_baz",
> },
> {
>                 "EventCode": "0x84",
>                 "EventName": "event_foo ",
>                 "Compat": "0x00000040",
>                 "Unit": "pmu_unit"
> },
> {
>                 "EventCode": "0x85",
>                 "EventName": "event_bar ",
>                 "Compat": "0x00000040",
>                 "Unit": "pmu_unit"
> },
>
> And we have a pmu with name and hw id matching "pmu_unit" and
> "0x00000040" present, that we select metric metric_baz for soc_b

Not sure I'm fully understanding.With the sysfs layout we'd have to
have a way of supporting CPUIDs, we could have a mapfile.csv style
approach or perhaps encode the CPUID into the path. It is complex as
CPUIDs are wildcards in the tool.

> >I think Unit may be useful, say on Intel
> > hybrid I want the tma_fround_bound metric on just cpu_atom. Currently
> > the use of Unit is messy for metrics, ie uncore metrics are associated
> > with core PMUs, and what to do with a MetricExpr with >1 PMU. I think
> > we're learning from trying. I'm just hoping the migration to a sysfs
> > style layout will still be possible, as I can see lots of upside in
> > terms of testing, 1 approach, etc.
>
> Do you have an RFC or something for this "sysfs style layout"? I think
> that it would be easier for me to understand your idea by seeing the SW.

When I get a chance :-) My thought is to first extend jevents.py to
have a second output format, so sysfs style rather than pmu-events.c.
This way we can merge the changes as a jevents.py feature even if we
don't change the perf tool to support it.

Thanks,
Ian

> Thanks,
> John