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

[John Garry <john.garry@huawei.com>]

On 05/10/2020 17:28, Ian Rogers wrote:
> On Mon, Oct 5, 2020 at 3:06 AM John Garry <john.garry@huawei.com> wrote:
>>
>> On 02/10/2020 21:46, Ian Rogers wrote:
>>> On Fri, Oct 2, 2020 at 5:00 AM John Garry <john.garry@huawei.com> 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@google.com/
>>> 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