Re: [PATCHv3 00/19] perf metric: Add support to reuse metric

Jiri Olsa
2020-07-20T07:22:50Z

On Mon, Jul 20, 2020 at 12:14:00PM +0530, kajoljain wrote:
>
>
> On 7/19/20 11:43 PM, Jiri Olsa wrote:
> > hi,
> > this patchset is adding the support to reused metric in
> > another metric.
> >
> > 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/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.
> >
> > v3 changes:
> > - added some acks
> > - some patches got merged
> > - added missing zalloc include [John Garry]
> > - added ids array outside the egroup object [Ian]
> > - removed wrong m->has_constraint assignment [Ian]
> > - renamed 'list' to 'metric_list' [Ian]
> > - fixed group metric and added test for it [Paul A. Clarke]
> > - fixed memory leak [Arnaldo]
> > - using lowercase keys for metrics in hashmap, because jevents
> > converts metric_expr to lowercase
> >
> > Also available in here:
> > git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> > perf/metric
> >
>
> Hi Jiri,
> I am trying to review these patches and also test it in power box. I am testing your `perf/metric` branch.
> With your current patches,some of hv-24x7 events not giving appropriate result
> while doing "--metric-only" command. I can't see corresponding output for all chips.

hi,
is that just for --metric-only option?

>
> This is output on power9 machine:
>
> Without your patches on upstream kernel:
>
> command# ./perf stat --metric-only -M PowerBUS_Frequency -I 1000 -C 0
> # time GHz PowerBUS_Frequency_0 GHz PowerBUS_Frequency_1
> 1.000073877 2.0 2.0
> 2.000240551 2.0 1.9
> ^C 2.452590532 0.9 0.9
>
> Performance counter stats for 'CPU(s) 0':
>
> 4.8 4.8
>
> 2.452654834 seconds time elapsed
>
> With your patches on perf/metric branch:
>
> command# ./perf stat --metric-only -M PowerBUS_Frequency -I 1000 -C 0
> assertion failed at util/metricgroup.c:709
> # time GHz PowerBUS_Frequency_0
> 1.000073875 2.0
> 2.000380706 2.0
> ^C 2.658962182 1.3
>
> Performance counter stats for 'CPU(s) 0':
>
> 5.2
>
> Please let me know, if I am missing something.

hum, I'll need to add test for metric with hv-24x7 events

thanks,
jirka