Re: [PATCH 07/18] perf metric: Add add_metric function

From: Ian Rogers
Date: Tue Jul 14 2020 - 17:11:17 EST


On Sun, Jul 12, 2020 at 6:27 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>
> Decouple metric adding login into add_metric function,

s/login/logging/ ?

> so it can be used from other places in following changes.
>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>

Acked-by: Ian Rogers <irogers@xxxxxxxxxx>

Thanks,
Ian

> ---
> tools/perf/util/metricgroup.c | 42 ++++++++++++++++++++++-------------
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 72552608ff7d..9a168f3df7a4 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -634,18 +634,11 @@ static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *
> return NULL;
> }
>
> -static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> - struct strbuf *events,
> - struct list_head *group_list,
> - struct pmu_events_map *map)
> +static int add_metric(struct list_head *group_list,
> + struct pmu_event *pe,
> + bool metric_no_group)
> {
> - struct pmu_event *pe;
> - struct egroup *eg;
> - int ret;
> -
> - pe = find_metric(metric, map);
> - if (!pe)
> - return -EINVAL;
> + int ret = 0;
>
> pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
>
> @@ -654,8 +647,6 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> pe,
> metric_no_group,
> 1);
> - if (ret)
> - return ret;
> } else {
> int j, count;
>
> @@ -666,14 +657,33 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> * those events to group_list.
> */
>
> - for (j = 0; j < count; j++) {
> + for (j = 0; j < count && !ret; j++) {
> ret = __metricgroup__add_metric(
> group_list, pe,
> metric_no_group, j);
> - if (ret)
> - return ret;
> }
> }
> +
> + return ret;
> +}
> +
> +static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> + struct strbuf *events,
> + struct list_head *group_list,
> + struct pmu_events_map *map)
> +{
> + struct pmu_event *pe;
> + struct egroup *eg;
> + int ret;
> +
> + pe = find_metric(metric, map);
> + if (!pe)
> + return -EINVAL;
> +
> + ret = add_metric(group_list, pe, metric_no_group);
> + if (ret)
> + return ret;
> +
> list_for_each_entry(eg, group_list, nd) {
> if (events->len > 0)
> strbuf_addf(events, ",");
> --
> 2.25.4
>