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:   Sun, 28 Jun 2020 23:59:57 +0200
From:   Jiri Olsa <jolsa@redhat.com>
To:     Ian Rogers <irogers@google.com>
Cc:     Jiri Olsa <jolsa@kernel.org>,
        Arnaldo Carvalho de Melo <acme@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 09/10] perf tools: Compute other metrics
Message-ID: <20200628215957.GO2988321@krava>
References: <20200626194720.2915044-1-jolsa@kernel.org>
 <20200626194720.2915044-10-jolsa@kernel.org>
 <CAP-5=fUODuF+LY6cbU4BPEcxu_YXDYg42pgRYU=yY5e47z05=g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAP-5=fUODuF+LY6cbU4BPEcxu_YXDYg42pgRYU=yY5e47z05=g@mail.gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 26, 2020 at 02:24:38PM -0700, Ian Rogers wrote:

SNIP

> > +
> > +                                       if (expr__get_id(ctx, lookup, &data) || !data) {
> >                                                 pr_debug("%s not found\n", $1);
> >                                                 free($1);
> >                                                 YYABORT;
> >                                         }
> > +
> > +                                       pr_debug2("lookup: is_other %d, counted %d: %s\n",
> > +                                                 data->is_other, data->other.counted, lookup);
> > +
> > +                                       if (data->is_other && !data->other.counted) {
> > +                                               data->other.counted = true;
> > +                                               if (expr__parse(&data->val, ctx, data->other.metric_expr, 1)) {
> 
> Ah, so this handles the problem the referenced metric isn't calculated
> and calculates it - with the sharing of events this doesn't impose
> extra pmu cost. Do we need to worry about detecting recursion? For
> example:
> 
> "MetricName": "Foo",
> "MetricExpr": "1/metric:Foo",

right, we should add some recursion check,
I'll lcheck on it

> 
> It seems unfortunate to have the MetricExpr calculated twice, but it

hum, not sure what you mean by twice.. we do that just once for
each involved metric and store the value.. the metric is also
processed before for 'other' metrics

jirka

> is understandable. Is it also a property that referenced/other metrics
> won't be reported individually? Perhaps these are sub-metrics?

> 
> Thanks,
> Ian
> 
> > +                                                       pr_debug("%s failed to count\n", $1);
> > +                                                       free($1);
> > +                                                       YYABORT;
> > +                                               }
> > +                                       }
> > +
> >                                         $$ = data->val;
> >                                         free($1);
> >                                 }
> > --
> > 2.25.4
> >
>