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

[Ian Rogers]

On Wed, May 6, 2026 at 11:37 PM Yushan Wang <wangyushan12@xxxxxxxxxx> wrote:
>
> From: Shiju Jose <shiju.jose@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Co-developed-by: Yushan Wang <wangyushan12@xxxxxxxxxx>
> Signed-off-by: Yushan Wang <wangyushan12@xxxxxxxxxx>
> ---
>  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/iostat.c b/tools/perf/util/iostat.c
> index 90607d1cf3fa..6c51f4a521af 100644
> --- a/tools/perf/util/iostat.c
> +++ b/tools/perf/util/iostat.c
> @@ -13,7 +13,7 @@ static struct iostat_pmu *iostat_pmu;
>
>  enum iostat_mode_t iostat_mode = IOSTAT_NONE;
>
> -__weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
> +int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)

Less "weak", beautiful!

>  {
>         if (!iostat_pmu)
>                 return -1;
> @@ -21,7 +21,7 @@ __weak int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config
>         return iostat_pmu->prepare(evlist, config);
>  }
>
> -__weak int iostat_parse(const struct option *opt, const char *str, int unset)
> +int iostat_parse(const struct option *opt, const char *str, int unset)
>  {
>         if (!iostat_pmu)
>                 return -1;
> @@ -29,33 +29,33 @@ __weak int iostat_parse(const struct option *opt, const char *str, int unset)
>         return iostat_pmu->parse(opt, str, unset);
>  }
>
> -__weak void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
> +void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         iostat_pmu->list(evlist, config);
>  }
>
> -__weak void iostat_release(struct evlist *evlist)
> +void iostat_release(struct evlist *evlist)
>  {
>         iostat_pmu->release(evlist);
>  }
>
> -__weak void iostat_print_header_prefix(struct perf_stat_config *config)
> +void iostat_print_header_prefix(struct perf_stat_config *config)
>  {
>         iostat_pmu->print_header_prefix(config);
>  }
>
> -__weak void iostat_print_metric(struct perf_stat_config *config,
> -                               struct evsel *evsel,
> -                               struct perf_stat_output_ctx *out)
> +void iostat_print_metric(struct perf_stat_config *config,
> +                        struct evsel *evsel,
> +                        struct perf_stat_output_ctx *out)
>  {
>         iostat_pmu->print_metric(config, evsel, out);
>  }
>
> -__weak void iostat_print_counters(struct evlist *evlist,
> -                                 struct perf_stat_config *config,
> -                                 struct timespec *ts, char *prefix,
> -                                 iostat_print_counter_t print_cnt_cb,
> -                                 void *arg)
> +void iostat_print_counters(struct evlist *evlist,
> +                          struct perf_stat_config *config,
> +                          struct timespec *ts, char *prefix,
> +                          iostat_print_counter_t print_cnt_cb,
> +                          void *arg)
>  {
>         iostat_pmu->print_counters(evlist, config, ts, prefix,
>                                    print_cnt_cb, arg);
> 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;
>  }
>
> -static void iostat_prefix(struct evlist *evlist,
> -                         struct perf_stat_config *config,
> -                         char *prefix, struct timespec *ts)
> +static void iio_iostat_prefix(struct evlist *evlist,
> +                             struct perf_stat_config *config,
> +                             char *prefix, struct timespec *ts)
>  {
>         struct iio_root_port *rp = evlist->selected->priv;
>
> @@ -354,7 +354,7 @@ static void iostat_prefix(struct evlist *evlist,
>         }
>  }
>
> -int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
> +static int iio_iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         if (evlist->core.nr_entries > 0) {
>                 pr_warning("The -e and -M options are not supported."
> @@ -371,8 +371,8 @@ int iostat_prepare(struct evlist *evlist, struct perf_stat_config *config)
>         return iostat_event_group(evlist, root_ports);
>  }
>
> -int iostat_parse(const struct option *opt, const char *str,
> -                int unset __maybe_unused)
> +static int iio_iostat_parse(const struct option *opt, const char *str,
> +                           int unset __maybe_unused)
>  {
>         int ret;
>         struct perf_stat_config *config = (struct perf_stat_config *)opt->data;
> @@ -392,7 +392,7 @@ int iostat_parse(const struct option *opt, const char *str,
>         return ret;
>  }
>
> -void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
> +static void iio_iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>  {
>         struct evsel *evsel;
>         struct iio_root_port *rp = NULL;
> @@ -405,7 +405,7 @@ void iostat_list(struct evlist *evlist, struct perf_stat_config *config)
>         }
>  }
>
> -void iostat_release(struct evlist *evlist)
> +static void iio_iostat_release(struct evlist *evlist)
>  {
>         struct evsel *evsel;
>         struct iio_root_port *rp = NULL;
> @@ -418,7 +418,7 @@ void iostat_release(struct evlist *evlist)
>         }
>  }
>
> -void iostat_print_header_prefix(struct perf_stat_config *config)
> +static void iio_iostat_print_header_prefix(struct perf_stat_config *config)
>  {
>         if (config->csv_output)
>                 fputs("port,", config->output);
> @@ -428,8 +428,8 @@ void iostat_print_header_prefix(struct perf_stat_config *config)
>                 fprintf(config->output, "   port         ");
>  }
>
> -void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> -                        struct perf_stat_output_ctx *out)
> +static void iio_iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
> +                                   struct perf_stat_output_ctx *out)
>  {
>         double iostat_value = 0;
>         u64 prev_count_val = 0;
> @@ -452,24 +452,54 @@ void iostat_print_metric(struct perf_stat_config *config, struct evsel *evsel,
>                           iostat_value / (256 * 1024));
>  }
>
> -void iostat_print_counters(struct evlist *evlist,
> -                          struct perf_stat_config *config, struct timespec *ts,
> -                          char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
> +static void iio_iostat_print_counters(struct evlist *evlist,
> +                                     struct perf_stat_config *config, struct timespec *ts,
> +                                     char *prefix, iostat_print_counter_t print_cnt_cb, void *arg)
>  {
>         void *perf_device = NULL;
>         struct evsel *counter = evlist__first(evlist);
>
>         evlist__set_selected(evlist, counter);
> -       iostat_prefix(evlist, config, prefix, ts);
> +       iio_iostat_prefix(evlist, config, prefix, ts);
>         fprintf(config->output, "%s", prefix);
>         evlist__for_each_entry(evlist, counter) {
>                 perf_device = evlist->selected->priv;
>                 if (perf_device && perf_device != counter->priv) {
>                         evlist__set_selected(evlist, counter);
> -                       iostat_prefix(evlist, config, prefix, ts);
> +                       iio_iostat_prefix(evlist, config, prefix, ts);
>                         fprintf(config->output, "\n%s", prefix);
>                 }
>                 print_cnt_cb(config, counter, arg);
>         }
>         fputc('\n', config->output);
>  }
> +
> +/*
> + * 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;
```


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

> +               .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
>