RE: [PATCH] perf metricgroup: Fix system PMU metrics

[Joakim Zhang]

> -----Original Message-----
> From: John Garry <john.garry@xxxxxxxxxx>
> Sent: 2021年1月19日 19:05
> To: Joakim Zhang <qiangqing.zhang@xxxxxxx>; peterz@xxxxxxxxxxxxx;
> mingo@xxxxxxxxxx; acme@xxxxxxxxxx; mark.rutland@xxxxxxx;
> alexander.shishkin@xxxxxxxxxxxxxxx; jolsa@xxxxxxxxxx;
> namhyung@xxxxxxxxxx; irogers@xxxxxxxxxx; kjain@xxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxxxxx
> Subject: Re: [PATCH] perf metricgroup: Fix system PMU metrics
> 
> On 19/01/2021 10:56, Joakim Zhang wrote:
> >> Joakim reports that getting "perf stat" for multiple system PMU
> >> metrics
> >> segfaults:
> >> ./perf stat -a -I 1000 -M imx8mm_ddr_write.all,imx8mm_ddr_write.all
> >> Segmentation fault
> >>
> >> While the same works without issue for a single metric.
> >>
> >> The logic in metricgroup__add_metric_sys_event_iter() is broken, in
> >> that
> >> add_metric() @m argument should be NULL for each new metric. Fix by
> >> not passing a holder for that, and rather make local in
> >> metricgroup__add_metric_sys_event_iter().
> >>
> >> Fixes: be335ec28efa ("perf metricgroup: Support adding metrics for
> >> system
> >> PMUs")
> >> Reported-by: Joakim Zhang<qiangqing.zhang@xxxxxxx>
> >> Signed-off-by: John Garry<john.garry@xxxxxxxxxx>
> > root@imx8mmevk:~# ./perf stat -a -I 1000 -M
> imx8mm_ddr_read.all,imx8mm_ddr_write
> 
>        .all
> > #           time             counts unit events
> >       1.001446500              40832
> imx8mm_ddr.read_cycles    #    638.0 KB  imx8mm_ddr_read.all
> >       1.001446500              16973
> imx8mm_ddr.write_cycles   #    265.2 KB  imx8mm_ddr_write.all
> >       2.003150250              28836
> imx8mm_ddr.read_cycles    #    450.6 KB  imx8mm_ddr_read.all
> >       2.003150250               6705
> imx8mm_ddr.write_cycles   #    104.8 KB  imx8mm_ddr_write.all
> >
> > For this issue, Tested-by: Joakim Zhang<qiangqing.zhang@xxxxxxx>
> >
> > Hi John,
> >
> > It seems have other issue compared to 5.10 kernel after switching to this
> framework, below metric can't work.
> > "MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ )
> * 4 * 4 / duration_time) / (750 * 1000000 * 4 * 4)"
> > After change to:
> > "MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles ) *
> > 4 * 4 / duration_time) / (750 * 1000000 * 4 * 4)",
> 
> Hmmm... not sure what you mean by "compared to 5.10 kernel". As far as I'm
> concerned, none of this was supported in 5.10 and metrics did not work for
> arm64. Support for sys PMU events+metrics only came in 5.11-rc.

Yes, 5.10 doesn't support ARM64. I add some code let it work locally. And,
"MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ ) * 4 * 4 / duration_time) / (750 * 1000000 * 4 * 4)"
Above metric expression can work fine.

> Anyway, can you share the full metric event which you say does not work, not
> just the "MetricExpr"?

OK, Could help check below metric? Thanks.
"MetricExpr": "(( imx8_ddr0@read\\-cycles@ + imx8_ddr0@write\\-cycles@ ) * 4 * 4 / duration_time) / (750 * 1000000 * 4 * 4)"
or
"MetricExpr": "(( imx8mm_ddr.read_cycles + imx8mm_ddr.write_cycles ) * 4 * 4 / duration_time) / (750 * 1000000 * 4 * 4)"

Best Regards,
Joakim Zhang
 
> Thanks,
> John