Re: [PATCH v2 8/8] perf pmu: Move pmu_metrics_table__find and remove ARM override

[Yicong Yang <yangyicong@huawei.com>]

On 2024/11/10 0:10, Ian Rogers wrote:
> On Sat, Nov 9, 2024 at 2:54 AM Yicong Yang <yangyicong@huawei.com> wrote:
>>
>> Hi,
>>
>> On 2024/11/8 0:20, Ian Rogers wrote:
>>> Move pmu_metrics_table__find to the jevents.py generated pmu-events.c
>>> and remove indirection override for ARM. The movement removes
>>> perf_pmu__find_metrics_table that exists to enable the ARM
>>> override. The ARM override isn't necessary as just the CPUID, not PMU,
>>> is used in the metric table lookup. On non-ARM the CPU argument is
>>> just ignored for the CPUID, for ARM -1 is passed so that the CPUID for
>>> the first logical CPU is read.
>>
>> Since the logic here's already been touching, is it possible to step it further to just
>> ignore the CPUID matching when finding the system metrics/events tables? It's may not be
>> that reasonable for finding a system metrics/events from the CPUID, since one system PMU may
>> exists on different platforms with different CPU types.
> 
> The issue is for conciseness reasons we let metrics and metric
> thresholds refer to other metrics, for example:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/pmu-events/arch/x86/icelakex/icx-metrics.json#n78
> ```
>     {
>         "BriefDescription": "This category represents fraction of
> slots where no uops are being delivered due to a lack of required
> resources for accepting new uops in the Backend",
>         "MetricExpr": "topdown\\-be\\-bound / (topdown\\-fe\\-bound +
> topdown\\-bad\\-spec + topdown\\-retiring + topdown\\-be\\-bound) + 5
> * cpu@INT_MISC.RECOVERY_CYCLES\\,cmask\\=1\\,edge@ / tma_info_slots",
>         "MetricGroup": "TmaL1;TopdownL1;tma_L1_group",
>         "MetricName": "tma_backend_bound",
>         "MetricThreshold": "tma_backend_bound > 0.2",
>         "MetricgroupNoGroup": "TopdownL1",
>         "PublicDescription": "This category represents fraction of
> slots where no uops are being delivered due to a lack of required
> resources for accepting new uops in the Backend. Backend is the
> portion of the processor core where the out-of-order scheduler
> dispatches ready uops into their respective execution units; and once
> completed these uops get retired according to program order. For
> example; stalls due to data-cache misses or stalls due to the divider
> unit being overloaded are both categorized under Backend Bound.
> Backend Bound is further divided into two main categories: Memory
> Bound and Core Bound. Sample with: TOPDOWN.BACKEND_BOUND_SLOTS",
>         "ScaleUnit": "100%"
>     },
> ```
> 
> The system metrics were added on top of this and we never rethought
> the design. For a metric to refer to another metric there needs to be
> some kind of place we look up from and for that we use the CPUID
> associated table. Perhaps the easiest thing is that if no CPUID table
> is matched we have an empty table.
> 

thanks for the clarificaiton. ok then it's different from my problem and I
shouldn't have mixed them up.

Thanks.