Messages in this thread | | | From | Ian Rogers <> | Date | Fri, 30 Jun 2023 13:16:52 -0700 | Subject | Re: [PATCH RFC 4/9] perf jevents: Add sys_events_find_events_table() |
| |
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. > > If an event is specific to a particular PMU, shouldn't the metric name > the PMU with the event? 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@", > > 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 looking at the later patches, they are making it so the PMU doesn't need to be specified, 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. 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://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
Thanks, Ian
> Thanks, > Ian > > > > Signed-off-by: John Garry <john.g.garry@oracle.com> > > --- > > tools/perf/pmu-events/empty-pmu-events.c | 6 ++++++ > > tools/perf/pmu-events/jevents.py | 11 +++++++++++ > > tools/perf/pmu-events/pmu-events.h | 3 +++ > > 3 files changed, 20 insertions(+) > > > > diff --git a/tools/perf/pmu-events/empty-pmu-events.c b/tools/perf/pmu-events/empty-pmu-events.c > > index a630c617e879..ae431b6bdf91 100644 > > --- a/tools/perf/pmu-events/empty-pmu-events.c > > +++ b/tools/perf/pmu-events/empty-pmu-events.c > > @@ -290,6 +290,12 @@ int pmu_metrics_table_for_each_metric(const struct pmu_metrics_table *table, pmu > > return 0; > > } > > > > +const struct pmu_events_table * > > +sys_events_find_events_table(__maybe_unused const struct pmu_metrics_table *metrics) > > +{ > > + return NULL; > > +} > > + > > const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu) > > { > > const struct pmu_events_table *table = NULL; > > diff --git a/tools/perf/pmu-events/jevents.py b/tools/perf/pmu-events/jevents.py > > index 80b569b8634b..947e8b1efa26 100755 > > --- a/tools/perf/pmu-events/jevents.py > > +++ b/tools/perf/pmu-events/jevents.py > > @@ -786,6 +786,17 @@ int pmu_metrics_table_for_each_metric(const struct pmu_metrics_table *table, > > return 0; > > } > > > > +const struct pmu_events_table * > > +sys_events_find_events_table(const struct pmu_metrics_table *metrics) > > +{ > > + for (const struct pmu_sys_events *tables = &pmu_sys_event_tables[0]; > > + tables->name; tables++) { > > + if (&tables->metric_table == metrics) > > + return &tables->event_table; > > + } > > + return NULL; > > +} > > + > > const struct pmu_events_table *perf_pmu__find_events_table(struct perf_pmu *pmu) > > { > > const struct pmu_events_table *table = NULL; > > diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h > > index caf59f23cd64..a3642c08e39d 100644 > > --- a/tools/perf/pmu-events/pmu-events.h > > +++ b/tools/perf/pmu-events/pmu-events.h > > @@ -77,6 +77,9 @@ typedef int (*pmu_metric_iter_fn)(const struct pmu_metric *pm, > > const struct pmu_metrics_table *table, > > void *data); > > > > +const struct pmu_events_table * > > +sys_events_find_events_table(const struct pmu_metrics_table *metrics); > > + > > int pmu_events_table_for_each_event(const struct pmu_events_table *table, pmu_event_iter_fn fn, > > void *data); > > int pmu_metrics_table_for_each_metric(const struct pmu_metrics_table *table, pmu_metric_iter_fn fn, > > -- > > 2.35.3 > >
| |