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

[sashiko-bot@kernel.org]

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

commit 1084156951c4d64330b35bda1bdc7630ed3bc441
Author: Ian Rogers <irogers@google.com>

perf vendor events intel: Update sandybridge metrics to TMA 5.2

This commit updates the Intel Sandybridge metrics to TopDown Microarchitecture
Analysis (TMA) version 5.2. The update incorporates new metric groups, adds
new metrics, and modifies existing metric expressions and descriptions.

> 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

[ ... ]

> @@ -316,7 +306,7 @@
>          "MetricExpr": "64 * (UNC_ARB_TRK_REQUESTS.ALL + UNC_ARB_COH_TRK_REQUESTS.ALL) / 1e6 / tma_info_system_time / 1e3",
>          "MetricGroup": "HPC;MemOffcore;MemoryBW;SoC;tma_issueBW",
>          "MetricName": "tma_info_system_dram_bw_use",
> -        "PublicDescription": "Average external Memory Bandwidth Use for reads and writes [GB / sec]. Related metrics: tma_mem_bandwidth"
> +        "PublicDescription": "Average external Memory Bandwidth Use for reads and writes [GB / sec]. Related metrics: tma_mem_bandwidth, tma_uc_bound"

[Severity: Low]
Does this description introduce a dangling reference? The PublicDescription
adds a reference to tma_uc_bound, but this metric does not appear to be
defined for the Sandybridge architecture, which could result in dead metric
links when users query it in the perf tool.

>      },
>      {
>          "BriefDescription": "Giga Floating Point Operations Per Second",

[ ... ]

> @@ -431,7 +421,16 @@
>          "MetricGroup": "CacheHits;MemoryBound;TmaL3mem;TopdownL3;tma_L3_group;tma_memory_bound_group",
>          "MetricName": "tma_l3_bound",
>          "MetricThreshold": "tma_l3_bound > 0.05 & (tma_memory_bound > 0.2 & tma_backend_bound > 0.2)",
> -        "PublicDescription": "This metric estimates how often the CPU was stalled due to loads accesses to L3 cache or contended with a sibling Core.  Avoiding cache misses (i.e. L2 misses/L3 hits) can improve the latency and increase performance. Sample with: MEM_LOAD_UOPS_RETIRED.L3_HIT_PS",
> +        "PublicDescription": "This metric estimates how often the CPU was stalled due to loads accesses to L3 cache or contended with a sibling Core.  Avoiding cache misses (i.e. L2 misses/L3 hits) can improve the latency and increase performance. Sample with: MEM_LOAD_RETIRED.L3_HIT_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]
Is the "MetricConstraint": "NO_GROUP_EVENTS_SMT" attribute missing here?

The replaced tma_dram_bound metric included this constraint because the
underlying hardware events (like MEM_LOAD_UOPS_RETIRED.LLC_HIT and
CYCLE_ACTIVITY.STALLS_L2_PENDING) have PMU grouping restrictions on
Sandybridge when SMT is enabled.

Without this constraint, will perf stat attempt to group these events and
cause PMU scheduling failures?

> +        "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%"
>      },
>      {

[ ... ]

> @@ -508,7 +507,7 @@
>          "MetricGroup": "FetchLat;MicroSeq;TopdownL3;tma_L3_group;tma_fetch_latency_group;tma_issueMC;tma_issueMS;tma_issueMV;tma_issueSO",
>          "MetricName": "tma_ms_switches",
>          "MetricThreshold": "tma_ms_switches > 0.05 & (tma_fetch_latency > 0.1 & tma_frontend_bound > 0.15)",
> -        "PublicDescription": "This metric estimates the fraction of cycles when the CPU was stalled due to switches of uop delivery to the Microcode Sequencer (MS). Commonly used instructions are optimized for delivery by the DSB (decoded i-cache) or MITE (legacy instruction decode) pipelines. Certain operations cannot be handled natively by the execution pipeline; and must be performed by microcode (small programs injected into the execution stream). Switching to the MS too often can negatively impact performance. The MS is designated to deliver long uop flows required by CISC instructions like CPUID; or uncommon conditions like Floating Point Assists when dealing with Denormals. Sample with: IDQ.MS_SWITCHES. Related metrics: tma_clears_resteers, tma_l1_bound, tma_machine_clears, tma_microcode_sequencer, tma_mixing_vectors, tma_serializing_operation",
> +        "PublicDescription": "This metric estimates the fraction of cycles when the CPU was stalled due to switches of uop delivery to the Microcode Sequencer (MS). Commonly used instructions are optimized for delivery by the DSB (decoded i-cache) or MITE (legacy instruction decode) pipelines. Certain operations cannot be handled natively by the execution pipeline; and must be performed by microcode (small programs injected into the execution stream). Switching to the MS too often can negatively impact performance. The MS is designated to deliver long uop flows required by CISC instructions like CPUID; or uncommon conditions like Floating Point Assists when dealing with Denormals. Sample with: IDQ.MS_SWITCHES. Related metrics: tma_clears_resteers, tma_info_inst_mix_vectormixpki, tma_l1_bound, tma_machine_clears, tma_microcode_sequencer, tma_mixing_vectors, tma_serializing_operation",

[Severity: Low]
Could this description introduce another missing metric reference?

The related metrics list in the PublicDescription adds
tma_info_inst_mix_vectormixpki, which is absent from the Sandybridge metric
definitions.

>          "ScaleUnit": "100%"
>      },
>      {

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