Re: [RFC PATCH 08/25] perf stat: Add functions to get counter info

[Liang, Kan]

On 2023-09-25 2:18 a.m., weilin.wang@xxxxxxxxx wrote:
> From: Weilin Wang <weilin.wang@xxxxxxxxx>
> 
> Add data structure metricgroup__pmu_counters to represent hardware counters
> available in the system.
> 
> Add functions to parse pmu-events and create the list of pmu_info_list to
> hold the counter information of the system.
> 
> Signed-off-by: Weilin Wang <weilin.wang@xxxxxxxxx>
> ---
>  tools/perf/util/metricgroup.c | 59 +++++++++++++++++++++++++++++++++--
>  tools/perf/util/metricgroup.h | 15 +++++++++
>  tools/perf/util/pmu.c         |  5 +++
>  tools/perf/util/pmu.h         |  1 +
>  4 files changed, 78 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 7f2c1b017..feb5dab26 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -1598,8 +1598,8 @@ static int get_metricgroup_events(const char *full_id,
>  			.event_name = id,
>  			.event_id = full_id,
>  		};
> -		ret = pmu_events_table_for_each_event(table,
> -				metricgroup__add_metric_event_callback, &data);
> +		ret = pmu_events_table__for_each_event(table, /*pmu=*/NULL,
> +						      metricgroup__add_metric_event_callback, &data);
>  		if (ret)
>  			goto out;
>  	}
> @@ -1609,6 +1609,56 @@ static int get_metricgroup_events(const char *full_id,
>  	return ret;
>  }
>  
> +static struct metricgroup__pmu_counters *pmu_layout__new(const struct pmu_layout *pl)
> +{
> +	struct metricgroup__pmu_counters *l;
> +	l = zalloc(sizeof(*l));
> +
> +	if (!l)
> +		return NULL;
> +
> +	l->name = pl->pmu;
> +	l->size = pl->size;
> +	l->fixed_size = pl->fixed_size;
> +	pr_debug("create new pmu_layout: [pmu]=%s, [gp_size]=%ld, [fixed_size]=%ld\n",
> +		l->name, l->size, l->fixed_size);
> +	return l;
> +}
> +
> +static int metricgroup__add_pmu_layout_callback(const struct pmu_layout *pl,
> +						void *data)
> +{
> +	struct metricgroup__pmu_counters *pmu;
> +	struct list_head *d = data;
> +	int ret = 0;
> +
> +	pmu = pmu_layout__new(pl);
> +	if (!pmu)
> +		return -ENOMEM;
> +	list_add(&pmu->nd, d);
> +	return ret;
> +}
> +
> +/**
> + * get_pmu_counter_layouts - Find counter info of the architecture from
> + * the pmu_layouts table
> + * @pmu_info_list: the list that the new counter info of a pmu is added to.
> + * @table: pmu_layouts table that is searched for counter info.
> + */
> +static int get_pmu_counter_layouts(struct list_head *pmu_info_list,
> +				   const struct pmu_layouts_table
> +				   *table)
> +{
> +	LIST_HEAD(list);
> +	int ret;
> +
> +	ret = pmu_layouts_table__for_each_layout(table,
> +						metricgroup__add_pmu_layout_callback, &list);
> +
> +	list_splice(&list, pmu_info_list);
> +	return ret;
> +}
> +
>  /**
>   * hw_aware_build_grouping - Build event groupings by reading counter
>   * requirement of the events and counter available on the system from
> @@ -1627,6 +1677,7 @@ static int hw_aware_build_grouping(struct expr_parse_ctx *ctx __maybe_unused,
>  	LIST_HEAD(event_info_list);
>  	size_t bkt;
>  	const struct pmu_events_table *etable = pmu_events_table__find();
> +	const struct pmu_layouts_table *ltable = pmu_layouts_table__find();
>  
>  #define RETURN_IF_NON_ZERO(x) do { if (x) return x; } while (0)
>  	hashmap__for_each_entry(ctx->ids, cur, bkt) {
> @@ -1638,6 +1689,10 @@ static int hw_aware_build_grouping(struct expr_parse_ctx *ctx __maybe_unused,
>  		if (ret)
>  			return ret;
>  	}
> +	ret = get_pmu_counter_layouts(&pmu_info_list, ltable);
> +	if (ret)
> +		return ret;
> +
>  
>  	return ret;
>  #undef RETURN_IF_NON_ZERO
> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
> index e493f6965..8ee7b434e 100644
> --- a/tools/perf/util/metricgroup.h
> +++ b/tools/perf/util/metricgroup.h
> @@ -85,6 +85,21 @@ struct metricgroup__event_info {
>  	DECLARE_BITMAP(counters, NR_COUNTERS);
>  };
>  
> +/**
> + * A node is the counter availability of a pmu.
> + * This info is built up at the beginning from JSON file and
> + * used as a reference in metric grouping process.
> +*/
> +struct metricgroup__pmu_counters {
> +	struct list_head nd;
> +	/** The name of the pmu the event collected on. */
> +	const char *name;
> +	//DECLARE_BITMAP(counter_bits, NR_COUNTERS);
> +	/** The number of gp counters in the pmu. */
> +	size_t size;
> +	size_t fixed_size;
> +};
> +
>  /**
>   * Each group is one node in the group string list.
>   */
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index cde33e019..af4056a88 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -813,6 +813,11 @@ __weak const struct pmu_metrics_table *pmu_metrics_table__find(void)
>  	return perf_pmu__find_metrics_table(NULL);
>  }
>  
> +__weak const struct pmu_layouts_table *pmu_layouts_table__find(void)

Why __weak? Is there any other ARCHs use the function?

Thanks,
Kan
> +{
> +	return perf_pmu__find_layouts_table(NULL);
> +}
> +
>  /**
>   * perf_pmu__match_ignoring_suffix - Does the pmu_name match tok ignoring any
>   *                                   trailing suffix? The Suffix must be in form
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 6a4e170c6..3e9243e00 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -240,6 +240,7 @@ void pmu_add_cpu_aliases_table(struct perf_pmu *pmu,
>  char *perf_pmu__getcpuid(struct perf_pmu *pmu);
>  const struct pmu_events_table *pmu_events_table__find(void);
>  const struct pmu_metrics_table *pmu_metrics_table__find(void);
> +const struct pmu_layouts_table *pmu_layouts_table__find(void);
>  
>  int perf_pmu__convert_scale(const char *scale, char **end, double *sval);
>