Messages in this thread | | | From | Ian Rogers <> | Date | Tue, 11 Jul 2023 23:05:22 -0700 | Subject | Re: [PATCH RFC 4/9] perf jevents: Add sys_events_find_events_table() |
| |
On Mon, Jul 3, 2023 at 8:15 AM John Garry <john.g.garry@oracle.com> wrote: > > On 30/06/2023 21:16, Ian Rogers wrote: > > On Fri, Jun 30, 2023 at 12:00 PM Ian Rogers<irogers@google.com> wrote: > >> On Wed, Jun 28, 2023 at 3:30 AM John Garry<john.g.garry@oracle.com> wrote: > >>> Add a function to get the events table associated with a metric table for > >>> struct pmu_sys_events. > >>> > >>> We could also use something like: > >>> struct pmu_sys_events *sys = container_of(metrics, struct pmu_sys_events, > >>> metric_table); > >>> > >>> to lookup struct pmu_sys_events, but that relies on the user always passing > >>> a sys events metric struct pointer, so this way is safer, but slower. > > Hi Ian, > > >> If an event is specific to a particular PMU, shouldn't the metric name > >> the PMU with the event? > > I am working on the basis - which I think is quite sane in case of sys > events - that event names are unique to a PMU type. > > > For example: > >> > >> MetricName: "IPC", > >> MetricExpr: "instructions / cycles", > >> > >> Here instructions and cycles can wildcard match on BIG.little/hybrid > >> systems and so we get an IPC metric for each PMU - although, I suspect > >> this isn't currently quite working. We can also, and currently, do: > >> > >> MetricName: "IPC", > >> MetricExpr: "cpu_atom@instructions@ / cpu_atom@cycles@", > >> ... > >> MetricName: "IPC", > >> MetricExpr: "cpu_core@instructions@ / cpu_core@cycles@", > > I did not know that it was possible to state that an event is for a > specific PMU type in this fashion - is this feature new? Does it work > only for known terms, like cycles and instructions?
It has been in metrics a long time (I didn't choose that @ was the / replacement :-) ). It should work for all events.
> >> The @ is used to avoid parsing confusion with / meaning divide. The > >> PMUs for the events are explicitly listed here. We could say the PMU > >> is implied but then it gets complex for uncore events, for metrics > >> that mix core and uncore events. > > So this works ok for IPC and CPU PMUs as we want the same event for many > PMU types and naturally it would have the same name. > > I am still not sure that sys event metrics need to specify a PMU.
There was a similar thought for hybrid metrics. The PMU could be implied from the PMU of the metric. I think there can be confusion from an implied PMU, for example the cycles event without a PMU will open two events on a hybrid CPU. If we imply the PMU then it can mean just 1 PMU, but if the PMU doesn't have the event presumably it means the multiple PMU behavior.
In parse-events there is existing logic to wildcard events but to ignore those that don't match a given PMU. This is used to support the --cputype option in builtin-stat.c, there is a similar option for builtin-list.c. We can use this so that events in a metric only match the PMU of the metric. Currently there are core metrics but whose events are all uncore like: https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/alderlake/adl-metrics.json?h=perf-tools-next#n1802
So we'd need to move these metrics to be on the appropriate uncore PMU. Supporting >1 PMU 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.
Thanks, Ian
> > So looking at the later patches, they are making it so the PMU doesn't > > need to be specified, > > Right, as we assume that we use uniquely named events. Having non-unique > event names seems to create problems. > > > so I think it is the same issue as here. My > > thought was that the PMU would always be required for metrics like > > memory bandwidth per million instructions, ie >1 PMU. > > We treat these sys PMUs as standalone, and it makes no sense (currently) > to have a metric which contains events for multiple PMU types as we > don't know if the system is created with those PMUs, and, if it is, what > topology etc. > > > I know this > > makes the metrics longer, I've tried to avoid writing json metrics and > > have used Python to write them in my own work: > > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/metric.py?h=perf-tools-next*n411__;Iw!!ACWV5N9M2RV99hQ!PE_9BEFVCr25fA9OHzfEDuT-MncA5pnPf5C3eTqYnXGKG9Q2OItrEIiEYz1T366HjAayQmYtZ6N6WxPJBCI$ > > Thanks > John >
| |