Re: [PATCH v2 12/26] perf vendor events intel: Update ivytown metrics to TMA 5.2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf: missing SMT group constraint in ivytown tma_l3_miss_bound
--

commit 394dbde3b15671d6397d45498145f1f7fbbf5686
Author: Ian Rogers <irogers@google.com>

perf vendor events intel: Update ivytown metrics to TMA 5.2

This commit updates the Intel TMA metrics for Ivytown systems to version
5.2. It removes the older tma_dram_bound metric and introduces
tma_l3_miss_bound in its place.

> diff --git a/tools/perf/pmu-events/arch/x86/ivytown/ivt-metrics.json b/tools/perf/pmu-events/arch/x86/ivytown/ivt-metrics.json
> index 1cdd197ac883e..7ceda72b6cafa 100644
> --- a/tools/perf/pmu-events/arch/x86/ivytown/ivt-metrics.json
> +++ b/tools/perf/pmu-events/arch/x86/ivytown/ivt-metrics.json
[ ... ]
> -    {
> -        "BriefDescription": "This metric estimates how often the CPU was stalled on accesses to external memory (DRAM) by loads",
> -        "MetricConstraint": "NO_GROUP_EVENTS_SMT",
> -        "MetricExpr": "(1 - MEM_LOAD_UOPS_RETIRED.LLC_HIT / (MEM_LOAD_UOPS_RETIRED.LLC_HIT + 7 * MEM_LOAD_UOPS_RETIRED.LLC_MISS)) * CYCLE_ACTIVITY.STALLS_L2_PENDING / tma_info_thread_clks",
> -        "MetricGroup": "MemoryBound;TmaL3mem;TopdownL3;tma_L3_group;tma_memory_bound_group",
> -        "MetricName": "tma_dram_bound",
> -        "MetricThreshold": "tma_dram_bound > 0.1 & (tma_memory_bound > 0.2 & tma_backend_bound > 0.2)",
> -        "PublicDescription": "This metric estimates how often the CPU was stalled on accesses to external memory (DRAM) by loads. Better caching can improve the latency and increase performance. Sample with: MEM_LOAD_UOPS_RETIRED.L3_MISS_PS",
> -        "ScaleUnit": "100%"
> -    },
[ ... ]
> +    {
> +        "BriefDescription": "This metric estimates how often the CPU was stalled on accesses to external memory (DRAM) by loads",
> +        "MetricExpr": "(1 - MEM_LOAD_UOPS_RETIRED.LLC_HIT / (MEM_LOAD_UOPS_RETIRED.LLC_HIT + 7 * MEM_LOAD_UOPS_RETIRED.LLC_MISS)) * CYCLE_ACTIVITY.STALLS_L2_PENDING / tma_info_thread_clks",
> +        "MetricGroup": "MemoryBound;Offcore;TmaL3mem;TopdownL3;tma_L3_group;tma_memory_bound_group",
> +        "MetricName": "tma_l3_miss_bound",
> +        "MetricThreshold": "tma_l3_miss_bound > 0.1 & (tma_memory_bound > 0.2 & tma_backend_bound > 0.2)",

[Severity: Low]
Is the NO_GROUP_EVENTS_SMT constraint intentionally omitted for the new
tma_l3_miss_bound metric?

This new metric replaces tma_dram_bound and uses the exact same events,
such as MEM_LOAD_UOPS_RETIRED.LLC_HIT and CYCLE_ACTIVITY.STALLS_L2_PENDING.
The original tma_dram_bound metric and the complementary tma_l3_bound both
specified a MetricConstraint of NO_GROUP_EVENTS_SMT to avoid hardware errata
or grouping issues when SMT is enabled on Ivytown systems.

Could the absence of this constraint cause grouping failures or erroneous perf
metric counts when running perf stat with SMT enabled? A review of subsequent
patches confirms this omission was not addressed later in the series.

> +        "PublicDescription": "This metric estimates how often the CPU was stalled on accesses to external memory (DRAM) by loads. Better caching can improve the latency and increase performance. Sample with: MEM_LOAD_RETIRED.L3_MISS",
> +        "ScaleUnit": "100%"
> +    },

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924170346.3872848-1-irogers@google.com?part=12