Re: Issue of metrics for multiple uncore PMUs (was Re: [RFC PATCH v2 23/23] perf metricgroup: remove duped metric group events)

[John Garry]

On 05/10/2020 17:28, Ian Rogers wrote:
> On Mon, Oct 5, 2020 at 3:06 AM John Garry <john.garry@xxxxxxxxxx> wrote:
> > On 02/10/2020 21:46, Ian Rogers wrote:
> > > On Fri, Oct 2, 2020 at 5:00 AM John Garry <john.garry@xxxxxxxxxx> wrote:
> > > > On 07/05/2020 15:08, Ian Rogers wrote:
> > > >
> > > > Hi Ian,
> > > >
> > > > I was wondering if you ever tested commit 2440689d62e9 ("perf
> > > > metricgroup: Remove duped metric group events") for when we have a
> > > > metric which aliases multiple instances of the same uncore PMU in the
> > > > system?
> > >
> > > Sorry for this, I hadn't tested such a metric and wasn't aware of how
> > > the aliasing worked. I sent a fix for this issue here:
> > > https://lore.kernel.org/lkml/20200917201807.4090224-1-irogers@xxxxxxxxxx/
> > > Could you see if this addresses the issue for you? I don't see the
> > > change in Arnaldo's trees yet.
> >
> > Unfortunately this does not seem to fix my issue.
> >
> > So for that patch, you say you fix metric expression for DRAM_BW_Use,
> > which is:
> >
> > {
> >    "BriefDescription": "Average external Memory Bandwidth Use for reads
> > and writes [GB / sec]",
> >    "MetricExpr": "( 64 * ( uncore_imc@cas_count_read@ +
> > uncore_imc@cas_count_write@ ) / 1000000000 ) / duration_time",
> >    "MetricGroup": "Memory_BW",
> > "MetricName": "DRAM_BW_Use"
> > },
> >
> > But this metric expression does not include any alias events; rather I
> > think it is just cas_count_write + cas_count_read event count for PMU
> > uncore_imc / duration_time.
> >
> > When I say alias, I mean - as an example, we have event:
> >
> >       {
> >           "BriefDescription": "write requests to memory controller.
> > Derived from unc_m_cas_count.wr",
> >           "Counter": "0,1,2,3",
> >           "EventCode": "0x4",
> >           "EventName": "LLC_MISSES.MEM_WRITE",
> >           "PerPkg": "1",
> >           "ScaleUnit": "64Bytes",
> >           "UMask": "0xC",
> >           "Unit": "iMC"
> >       },
> >
> > And then reference LLC_MISSES.MEM_WRITE in a metric expression:
> >
> > "MetricExpr": "LLC_MISSES.MEM_WRITE / duration_time",
> >
> > This is what seems to be broken for when the alias matches > 1 PMU.
> >
> > Please check this.
Hi Ian,

> Happy to check. 

So I am, but the code is a little complicated :)

> Can you provide a reproduction? Looking on broadwell
> this metric doesn't exist.

Right, I just added this test metric as my 2x x86 platform has no 
examples which I can find:

diff --git a/tools/perf/pmu-events/arch/x86/broadwell/bdw-metrics.json 
b/tools/perf/pmu-events/arch/x86/broadwell/bdw-metrics.json
index 8cdc7c13dc2a..fc6d9adf996a 100644
--- a/tools/perf/pmu-events/arch/x86/broadwell/bdw-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/broadwell/bdw-metrics.json
@@ -348,5 +348,11 @@
        "MetricExpr": "(cstate_pkg@c7\\-residency@ / msr@tsc@) * 100",
        "MetricGroup": "Power",
        "MetricName": "C7_Pkg_Residency"
+    },
+    {
+        "BriefDescription": "test metric",
+        "MetricExpr": "UNC_CBO_XSNP_RESPONSE.MISS_XCORE * 
UNC_CBO_XSNP_RESPONSE.MISS_EVICTION",
+        "MetricGroup": "Test",
+        "MetricName": "test_metric_inc"
    }
]

I'll try to find a better mainline example, though, but I'm not hopeful ...

Thanks,
John