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

John Garry
2020-10-06T14:22:22Z

On 05/10/2020 19:05, John Garry wrote:
>> 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"
>     }
> ]
>

It seems that the code in find_evsel_group() does not properly handle
the scenario of event alias matching different PMUs (as I already said).

So I got it working on top of "perf metricgroup: Fix uncore metric
expressions" with the following change:

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index d948a7f910cf..6293378c019c 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -213,7 +213,8 @@ static struct evsel *find_evsel_group(struct evlist
*perf_evlist,
/* Ignore event if already used and merging is disabled. */
if (metric_no_merge && test_bit(ev->idx, evlist_used))
continue;
- if (!has_constraint && ev->leader != current_leader) {
+ if (!has_constraint && (!current_leader ||
strcmp(current_leader->name, ev->leader->name))) {
/*
* Start of a new group, discard the whole match and
* start again.
@@ -279,7 +280,8 @@ static struct evsel *find_evsel_group(struct evlist
*perf_evlist,
* when then group is left.
*/
if (!has_constraint &&
- ev->leader != metric_events[i]->leader)
+ strcmp(ev->leader->name, metric_events[i]->leader->name))
break;
if (!strcmp(metric_events[i]->name, ev->name)) {
set_bit(ev->idx, evlist_used);

which gives for my test metric:

./perf stat -v -M test_metric_inc sleep 1
Using CPUID GenuineIntel-6-3D-4
metric expr unc_cbo_xsnp_response.miss_xcore /
unc_cbo_xsnp_response.miss_eviction for test_metric_inc
found event unc_cbo_xsnp_response.miss_eviction
found event unc_cbo_xsnp_response.miss_xcore
adding
{unc_cbo_xsnp_response.miss_eviction,unc_cbo_xsnp_response.miss_xcore}:W
unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_1/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_eviction -> uncore_cbox_0/umask=0x81,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_1/umask=0x41,event=0x22/
unc_cbo_xsnp_response.miss_xcore -> uncore_cbox_0/umask=0x41,event=0x22/
Control descriptor is not initialized
unc_cbo_xsnp_response.miss_eviction: 595175 1001021311 1001021311
unc_cbo_xsnp_response.miss_eviction: 592516 1001020037 1001020037
unc_cbo_xsnp_response.miss_xcore: 39139 1001021311 1001021311
unc_cbo_xsnp_response.miss_xcore: 38718 1001020037 1001020037

Performance counter stats for 'system wide':

1,187,691 unc_cbo_xsnp_response.miss_eviction # 0.07
test_metric_inc
77,857 unc_cbo_xsnp_response.miss_xcore

1.001068918 seconds time elapsed

John