Re: [PATCH v2 06/18] perf s390 s390_cpumcfdg_dump: Don't scan all PMUs
From: Ian Rogers
Date: Thu Aug 24 2023 - 13:32:55 EST
On Thu, Aug 24, 2023 at 7:00 AM Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> Em Wed, Aug 23, 2023 at 09:13:18PM -0700, Ian Rogers escreveu:
> > Rather than scanning all PMUs for a counter name, scan the PMU
> > associated with the evsel of the sample. This is done to remove a
> > dependence on pmu-events.h.
>
> I'm applying this one, and CCing the S/390 developers so that they can
> try this and maybe provide an Acked-by/Tested-by,
Thanks Arnaldo! You're right that I wasn't able to test this s390
change on an s390.
Ian
> - Arnaldo
>
> > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> > ---
> > tools/perf/util/s390-sample-raw.c | 50 ++++++++++++++++---------------
> > 1 file changed, 26 insertions(+), 24 deletions(-)
> >
> > diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
> > index 91330c874170..dc1ed3e95d4d 100644
> > --- a/tools/perf/util/s390-sample-raw.c
> > +++ b/tools/perf/util/s390-sample-raw.c
> > @@ -27,7 +27,7 @@
> > #include "color.h"
> > #include "sample-raw.h"
> > #include "s390-cpumcf-kernel.h"
> > -#include "pmu-events/pmu-events.h"
> > +#include "util/pmu.h"
> > #include "util/sample.h"
> >
> > static size_t ctrset_size(struct cf_ctrset_entry *set)
> > @@ -132,56 +132,57 @@ static int get_counterset_start(int setnr)
> >
> > struct get_counter_name_data {
> > int wanted;
> > - const char *result;
> > + char *result;
> > };
> >
> > -static int get_counter_name_callback(const struct pmu_event *evp,
> > - const struct pmu_events_table *table __maybe_unused,
> > - void *vdata)
> > +static int get_counter_name_callback(void *vdata, struct pmu_event_info *info)
> > {
> > struct get_counter_name_data *data = vdata;
> > int rc, event_nr;
> > + const char *event_str;
> >
> > - if (evp->name == NULL || evp->event == NULL)
> > + if (info->str == NULL)
> > return 0;
> > - rc = sscanf(evp->event, "event=%x", &event_nr);
> > +
> > + event_str = strstr(info->str, "event=");
> > + if (!event_str)
> > + return 0;
> > +
> > + rc = sscanf(event_str, "event=%x", &event_nr);
> > if (rc == 1 && event_nr == data->wanted) {
> > - data->result = evp->name;
> > + data->result = strdup(info->name);
> > return 1; /* Terminate the search. */
> > }
> > return 0;
> > }
> >
> > -/* Scan the PMU table and extract the logical name of a counter from the
> > - * PMU events table. Input is the counter set and counter number with in the
> > - * set. Construct the event number and use this as key. If they match return
> > - * the name of this counter.
> > +/* Scan the PMU and extract the logical name of a counter from the event. Input
> > + * is the counter set and counter number with in the set. Construct the event
> > + * number and use this as key. If they match return the name of this counter.
> > * If no match is found a NULL pointer is returned.
> > */
> > -static const char *get_counter_name(int set, int nr, const struct pmu_events_table *table)
> > +static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
> > {
> > struct get_counter_name_data data = {
> > .wanted = get_counterset_start(set) + nr,
> > .result = NULL,
> > };
> >
> > - if (!table)
> > + if (!pmu)
> > return NULL;
> >
> > - pmu_events_table__for_each_event(table, get_counter_name_callback, &data);
> > + perf_pmu__for_each_event(pmu, &data, get_counter_name_callback);
> > return data.result;
> > }
> >
> > -static void s390_cpumcfdg_dump(struct perf_sample *sample)
> > +static void s390_cpumcfdg_dump(struct perf_pmu *pmu, struct perf_sample *sample)
> > {
> > size_t i, len = sample->raw_size, offset = 0;
> > unsigned char *buf = sample->raw_data;
> > const char *color = PERF_COLOR_BLUE;
> > struct cf_ctrset_entry *cep, ce;
> > - const struct pmu_events_table *table;
> > u64 *p;
> >
> > - table = pmu_events_table__find();
> > while (offset < len) {
> > cep = (struct cf_ctrset_entry *)(buf + offset);
> >
> > @@ -199,11 +200,12 @@ static void s390_cpumcfdg_dump(struct perf_sample *sample)
> > color_fprintf(stdout, color, " [%#08zx] Counterset:%d"
> > " Counters:%d\n", offset, ce.set, ce.ctr);
> > for (i = 0, p = (u64 *)(cep + 1); i < ce.ctr; ++i, ++p) {
> > - const char *ev_name = get_counter_name(ce.set, i, table);
> > + char *ev_name = get_counter_name(ce.set, i, pmu);
> >
> > color_fprintf(stdout, color,
> > "\tCounter:%03d %s Value:%#018lx\n", i,
> > ev_name ?: "<unknown>", be64_to_cpu(*p));
> > + free(ev_name);
> > }
> > offset += ctrset_size(&ce);
> > }
> > @@ -216,14 +218,14 @@ static void s390_cpumcfdg_dump(struct perf_sample *sample)
> > */
> > void evlist__s390_sample_raw(struct evlist *evlist, union perf_event *event, struct perf_sample *sample)
> > {
> > - struct evsel *ev_bc000;
> > + struct evsel *evsel;
> >
> > if (event->header.type != PERF_RECORD_SAMPLE)
> > return;
> >
> > - ev_bc000 = evlist__event2evsel(evlist, event);
> > - if (ev_bc000 == NULL ||
> > - ev_bc000->core.attr.config != PERF_EVENT_CPUM_CF_DIAG)
> > + evsel = evlist__event2evsel(evlist, event);
> > + if (evsel == NULL ||
> > + evsel->core.attr.config != PERF_EVENT_CPUM_CF_DIAG)
> > return;
> >
> > /* Display raw data on screen */
> > @@ -231,5 +233,5 @@ void evlist__s390_sample_raw(struct evlist *evlist, union perf_event *event, str
> > pr_err("Invalid counter set data encountered\n");
> > return;
> > }
> > - s390_cpumcfdg_dump(sample);
> > + s390_cpumcfdg_dump(evsel->pmu, sample);
> > }
> > --
> > 2.42.0.rc1.204.g551eb34607-goog
> >
>
> --
>
> - Arnaldo