Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 28 Jul 2020 09:29:40 -0300
From:   Arnaldo Carvalho de Melo <acme@kernel.org>
To:     Ian Rogers <irogers@google.com>
Cc:     Jiri Olsa <jolsa@kernel.org>, lkml <linux-kernel@vger.kernel.org>,
        Ingo Molnar <mingo@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Peter Zijlstra <a.p.zijlstra@chello.nl>,
        Michael Petlan <mpetlan@redhat.com>,
        Andi Kleen <ak@linux.intel.com>,
        Kajol Jain <kjain@linux.ibm.com>,
        John Garry <john.garry@huawei.com>,
        "Paul A. Clarke" <pc@us.ibm.com>,
        Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH 05/19] perf metric: Add macros for iterating map events
Message-ID: <20200728122940.GL40195@kernel.org>
References: <20200719181320.785305-1-jolsa@kernel.org>
 <20200719181320.785305-6-jolsa@kernel.org>
 <CAP-5=fWYOjCSruJ1=c6896zjJe-e-EMKJTrOUcoeo0fED3tk2A@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAP-5=fWYOjCSruJ1=c6896zjJe-e-EMKJTrOUcoeo0fED3tk2A@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Em Sun, Jul 19, 2020 at 02:46:12PM -0700, Ian Rogers escreveu:
> On Sun, Jul 19, 2020 at 11:13 AM Jiri Olsa <jolsa@kernel.org> wrote:
> >
> > Adding following macros to iterate events and metric:
> >   map_for_each_event(__pe, __idx, __map)
> >     - iterates over all pmu_events_map events
> >   map_for_each_metric(__pe, __idx, __map, __metric)
> >     - iterates over all metrics that match __metric argument
> >
> > and use it in metricgroup__add_metric function. Macros
> > will be be used from other places in following changes.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> Acked-by: Ian Rogers <irogers@google.com>

Thanks, applied.

- Arnaldo
 
> Thanks!
> Ian
> 
> > ---
> >  tools/perf/util/metricgroup.c | 77 ++++++++++++++++++-----------------
> >  1 file changed, 40 insertions(+), 37 deletions(-)
> >
> > diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> > index df0356ec120d..b37008fc253c 100644
> > --- a/tools/perf/util/metricgroup.c
> > +++ b/tools/perf/util/metricgroup.c
> > @@ -614,6 +614,17 @@ static int __metricgroup__add_metric(struct list_head *group_list,
> >         return 0;
> >  }
> >
> > +#define map_for_each_event(__pe, __idx, __map)                         \
> > +       for (__idx = 0, __pe = &__map->table[__idx];                    \
> > +            __pe->name || __pe->metric_group || __pe->metric_name;     \
> > +            __pe = &__map->table[++__idx])
> > +
> > +#define map_for_each_metric(__pe, __idx, __map, __metric)              \
> > +       map_for_each_event(__pe, __idx, __map)                          \
> > +               if (__pe->metric_expr &&                                \
> > +                   (match_metric(__pe->metric_group, __metric) ||      \
> > +                    match_metric(__pe->metric_name, __metric)))
> > +
> >  static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> >                                    struct strbuf *events,
> >                                    struct list_head *group_list,
> > @@ -624,49 +635,41 @@ static int metricgroup__add_metric(const char *metric, bool metric_no_group,
> >         int i, ret;
> >         bool has_match = false;
> >
> > -       for (i = 0; ; i++) {
> > -               pe = &map->table[i];
> > -
> > -               if (!pe->name && !pe->metric_group && !pe->metric_name) {
> > -                       /* End of pmu events. */
> > -                       if (!has_match)
> > -                               return -EINVAL;
> > -                       break;
> > -               }
> > -               if (!pe->metric_expr)
> > -                       continue;
> > -               if (match_metric(pe->metric_group, metric) ||
> > -                   match_metric(pe->metric_name, metric)) {
> > -                       has_match = true;
> > -                       pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
> > -
> > -                       if (!strstr(pe->metric_expr, "?")) {
> > -                               ret = __metricgroup__add_metric(group_list,
> > -                                                               pe,
> > -                                                               metric_no_group,
> > -                                                               1);
> > -                               if (ret)
> > -                                       return ret;
> > -                       } else {
> > -                               int j, count;
> > +       map_for_each_metric(pe, i, map, metric) {
> > +               pr_debug("metric expr %s for %s\n", pe->metric_expr, pe->metric_name);
> > +               has_match = true;
> > +
> > +               if (!strstr(pe->metric_expr, "?")) {
> > +                       ret = __metricgroup__add_metric(group_list,
> > +                                                       pe,
> > +                                                       metric_no_group,
> > +                                                       1);
> > +                       if (ret)
> > +                               return ret;
> > +               } else {
> > +                       int j, count;
> >
> > -                               count = arch_get_runtimeparam();
> > +                       count = arch_get_runtimeparam();
> >
> > -                               /* This loop is added to create multiple
> > -                                * events depend on count value and add
> > -                                * those events to group_list.
> > -                                */
> > +                       /* This loop is added to create multiple
> > +                        * events depend on count value and add
> > +                        * those events to group_list.
> > +                        */
> >
> > -                               for (j = 0; j < count; j++) {
> > -                                       ret = __metricgroup__add_metric(
> > -                                               group_list, pe,
> > -                                               metric_no_group, j);
> > -                                       if (ret)
> > -                                               return ret;
> > -                               }
> > +                       for (j = 0; j < count; j++) {
> > +                               ret = __metricgroup__add_metric(
> > +                                       group_list, pe,
> > +                                       metric_no_group, j);
> > +                               if (ret)
> > +                                       return ret;
> >                         }
> >                 }
> >         }
> > +
> > +       /* End of pmu events. */
> > +       if (!has_match)
> > +               return -EINVAL;
> > +
> >         list_for_each_entry(eg, group_list, nd) {
> >                 if (events->len > 0)
> >                         strbuf_addf(events, ",");
> > --
> > 2.25.4
> >

-- 

- Arnaldo