Re: [PATCH v2 06/18] perf s390 s390_cpumcfdg_dump: Don't scan all PMUs

[Arnaldo Carvalho de Melo]

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,

- 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