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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: missing NO_GROUP_EVENTS_SMT constraint for tma_l3_miss_bound metric on Sandybridge
--

commit 2ddca9ec203f3367beb0ec776bd232119da3e230
Author: Ian Rogers <irogers@google.com>

perf vendor events intel: Update sandybridge metrics to TMA 5.2

This commit updates the Sandybridge metrics to Intel's TMA 5.2 by modifying
metricgroups.json and snb-metrics.json. It adjusts several metric thresholds
and groups, notably replacing tma_dram_bound with tma_l3_miss_bound.

> diff --git a/tools/perf/pmu-events/arch/x86/sandybridge/snb-metrics.json b/tools/perf/pmu-events/arch/x86/sandybridge/snb-metrics.json
> index d407619034292..48f9ccc0a7275 100644
> --- a/tools/perf/pmu-events/arch/x86/sandybridge/snb-metrics.json
> +++ b/tools/perf/pmu-events/arch/x86/sandybridge/snb-metrics.json
> @@ -129,16 +129,6 @@
>          "PublicDescription": "This metric represents fraction of cycles where the Divider unit was active. Divide and square root instructions are performed by the Divider unit and can take considerably longer latency than integer or Floating Point addition; subtraction; or multiplication. Sample with: ARITH.DIVIDER_ACTIVE",
>          "ScaleUnit": "100%"
>      },
> -    {
> -        "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_MISC_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_MISC_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",

[Severity: Medium]
Will dropping the NO_GROUP_EVENTS_SMT constraint from the newly named
tma_l3_miss_bound metric cause PMU multiplexing errors on SMT systems?

The previous tma_dram_bound metric used the exact same event expression
(MEM_LOAD_UOPS_RETIRED.LLC_HIT, MEM_LOAD_UOPS_MISC_RETIRED.LLC_MISS) but
explicitly enforced this constraint to prevent hardware scheduling issues
when SMT is enabled.

By omitting this constraint, it looks like perf will inappropriately attempt
to schedule these events into a single PMU group under SMT.

> +        "MetricThreshold": "tma_l3_miss_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_RETIRED.L3_MISS",
>          "ScaleUnit": "100%"
>      },

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