Re: [RFT PATCH v2 6/7] perf-iostat: Make x86 iostat compatible with new iostat framework

[Yushan Wang <wangyushan12@huawei.com>]

On 5/8/2026 12:17 AM, Ian Rogers wrote:
> On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@huawei.com> wrote:
>>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> Change the original x86 iio iostat supporter to be compatible with the
>> set of iostat frameworks.
>>
>> The matching function of x86 iio may not be correct.
>>
>> Signed-off-by: Shiju Jose  <shiju.jose@huawei.com>
>> Co-developed-by: Yushan Wang <wangyushan12@huawei.com>
>> Signed-off-by: Yushan Wang <wangyushan12@huawei.com>
>> ---
>>  tools/perf/util/iostat.c     | 26 +++++++--------
>>  tools/perf/util/x86-iostat.c | 62 ++++++++++++++++++++++++++----------
>>  2 files changed, 59 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/perf/util/x86-iostat.c b/tools/perf/util/x86-iostat.c
>> index f8d4c9718594..778655cbc2a0 100644
>> --- a/tools/perf/util/x86-iostat.c
>> +++ b/tools/perf/util/x86-iostat.c
>> @@ -332,9 +332,9 @@ static int iostat_event_group(struct evlist *evl,
>>         return ret;
>>  }
>>

[...]

>> +
>> +/*
>> + * FIXME: pmu name prefix match might not work for x86 iio.
>> + */
>> +static bool iio_iostat_probe(struct iostat_pmu *iostat_pmu)
>> +{
>> +       return perf_pmus__scan_matching_wildcard(NULL, iostat_pmu->pmu_name_wildcard);
>
> It feels more readable to me as:
> ```
>   return perf_pmus__scan_matching_wildcard(NULL, "uncore_iio") != NULL;
> ```

Yes, I can do that refactor.

>
>
>> +}
>> +
>> +static struct iostat_pmu x86_iio_iostat_pmu_list[]  = {
>> +       {
>> +               .pmu_name_wildcard = "uncore_iio*",
>> +               .match = iio_iostat_probe,
>> +               .prepare = iio_iostat_prepare,
>> +               .parse = iio_iostat_parse,
>> +               .list = iio_iostat_list,
>> +               .print_header_prefix = iio_iostat_print_header_prefix,
>> +               .print_metric = iio_iostat_print_metric,
>
> Is there scope to push the metric printing into json? For example, you
> can have PMUs with json:
> tools/perf/pmu-events/arch/x86/alderlake/adl-metrics.json
> ```
>     {
>        "BriefDescription": "Uncore frequency per die [GHZ]",
>        "MetricExpr": "tma_info_system_socket_clks / #num_dies /
> duration_time / 1e9",
>        "MetricGroup": "SoC",
>        "MetricName": "UNCORE_FREQ",
>        "Unit": "cpu_core"
>    },
> ```
> and do uncore type things:
> tools/perf/pmu-events/arch/x86/amdzen6/recommended.json
> ```
>   {
>    "MetricName": "umc_activate_cmd_rate",
>    "BriefDescription": "Memory controller ACTIVATE command rate.",
>    "MetricExpr": "d_ratio(umc_act_cmd.all * 1000, umc_mem_clk)",
>    "MetricGroup": "memory_controller",
>    "PerPkg": "1",
>    "ScaleUnit": "1per_memclk"
>  },
> ```

Currently it's a bit far to reach json configurable PMU iostat structural
display, since the PMUs are vendor specific and may have different meaningful
combinations of events, and they may be hard to be handled with generalized
handler.

Taking currently availble uncore_iio* and hisi_pcie* pmu as example, they have
different sets of events to display, different ways of calculation, and
different PMU to PCIe root port mapping topology, which requires dedicated
handler.

But admittedly there are a lot of redundant code that can be optimized, I can
see if those iostat_* callbacks can be merged or generalized so that variated
functions can be minimized.

Thanks,
Yushan

>
>> +               .print_counters = iio_iostat_print_counters,
>> +               .release = iio_iostat_release,
>> +       },
>> +};
>> +
>> +static void __attribute__((constructor)) x86_iio_iostat_pmu_init(void)
>
> I don't believe these attributes are what we normally do.
>
> Thanks,
> Ian
>
>
>> +{
>> +       unsigned int i;
>> +
>> +       for (i = 0; i < ARRAY_SIZE(x86_iio_iostat_pmu_list); i++)
>> +               register_iostat_pmu(&x86_iio_iostat_pmu_list[i]);
>> +}
>> --
>> 2.33.0
>>