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:   Wed, 15 Jul 2020 23:41:34 +0200
From:   Jiri Olsa <jolsa@redhat.com>
To:     "Paul A. Clarke" <pc@us.ibm.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>,
        Stephane Eranian <eranian@google.com>,
        Ian Rogers <irogers@google.com>
Subject: Re: [PATCH 00/18] perf metric: Add support to reuse metric
Message-ID: <20200715214134.GW183694@krava>
References: <20200712132634.138901-1-jolsa@kernel.org>
 <20200715183327.GA21935@oc3272150783.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200715183327.GA21935@oc3272150783.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Jul 15, 2020 at 01:33:27PM -0500, Paul A. Clarke wrote:
> On Sun, Jul 12, 2020 at 03:26:16PM +0200, Jiri Olsa wrote:
> > hi,
> > this patchset is adding the support to reused metric in another 
> > metric. The metric needs to be referenced by 'metric:' prefix.
> > 
> > For example, to define IPC by using CPI with change like:
> > 
> >          "BriefDescription": "Instructions Per Cycle (per Logical Processor)",
> >  -       "MetricExpr": "INST_RETIRED.ANY / CPU_CLK_UNHALTED.THREAD",
> >  +       "MetricExpr": "1/metric:CPI",
> >          "MetricGroup": "TopDownL1",
> >          "MetricName": "IPC"
> > 
> > I won't be able to find all the possible places we could
> > use this at, so I wonder you guys (who was asking for this)
> > would try it and come up with comments if there's something
> > missing or we could already use it at some places.
> > 
> > It's based on Arnaldo's tmp.perf/core.
> > 
> > v2 changes:
> >   - collected Ian's acks for few patches [Ian]
> >   - renamed expr__add_id to expr__add_id_val [Ian]
> >   - renamed expr_parse_data to expr_id_data [Ian]
> >   - added recursion check [Ian]
> >   - added metric test for DCache_L2 metric [Ian]
> >   - added some renames as discussed in review [Ian]
> > 
> > Also available in here:
> >   git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> >   perf/metric
> 
> I'm having trouble testing this.
> 
> I checked out this tree, and am able to build with a JSON metrics definition
> file which uses other metrics.  I put this aside, though, because of the
> following issue.
> 
> I built the kernel from this same tree and booted it successfully.
> However, the metrics are not working correctly.  (I may very well be
> doing something wrong.)

if you'll share the metric change I can help debugging that

> 
> The base system is RHEL8, but it's now booted with the new kernel.
> ```
> # uname -a
> Linux system 5.8.0-rc4-g7dd02cf0b #1 SMP Wed Jul 15 12:31:45 EDT 2020 ppc64le ppc64le ppc64le GNU/Linux
> # perf stat --metrics cpi_breakdown ./load
> failed: way too many variables

hm, this ^^^ error string was removed in:
  43fe337c86a9 perf expr: Migrate expr ids table to a hashmap

looks like you're not running the correct perf binary

thanks for testing this,
jirka