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

[Ian Rogers]

On Fri, Jul 14, 2023 at 4:58 AM John Garry <john.g.garry@xxxxxxxxxx> wrote:
>
> On 13/07/2023 22:35, Ian Rogers wrote:
> >>
> >> {
> >>                  "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 am not sure why you mention CPUIDs. sys events and their metrics are
> matched only on Unit and Compat.
>
> Furthermore, my solution here is based what we have today, and would not
> be based on this sysfs solution which you mention.
>
> >
> >>> 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.
>
> ok, fine, but I would still like this progress this work now. It has
> been sitting around for some time and was really difficult to rebase
> (since I was not tumbling my baseline).

Sure, the sysfs thing is a distraction until we have it. In this
series my main concern was in the changes of the event lookup and
having implied PMUs. You mentioned doing these changes so I was
waiting for a v2.

Thanks,
Ian

> Thanks,
> John