On Fri, Mar 10, 2023 at 10:28:39AM +0900, Damien Le Moal wrote:
> When using per-priority statistics for workloads using multiple
> different priority values, the statistics output displays the priority
> class and value (level) for each set of statistics. However, this is
> done using linux priority values coding, that is, assuming that the
> priority level is at most 7 (lower 3-bits). However, this is not always
> the case for all OSes. E.g. dragonfly allows IO priorities up to a
> value of 10.
>
> Introduce the OS dependent ioprio_class() and ioprio() macros to extract
> the fields from an ioprio value according to the OS capabilities. A
> generic definition (always returning 0) for these macros in os/os.h is
> added and used for all OSes that do not define these macros.
>
> The functions show_ddir_status() and add_ddir_status_json() are modified
> to use these new macros to fix per priority statistics output. The
> modification includes changes to the loops over the clat_prio array to
> reduce indentation levels, making the code a little cleaner.
>
> Fixes: 692dec0cfb4b ("stat: report clat stats on a per priority granularity")
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> ---
> os/os-dragonfly.h | 2 ++
> os/os-linux.h | 3 ++
> os/os.h | 2 ++
> stat.c | 85 +++++++++++++++++++++++++----------------------
> 4 files changed, 52 insertions(+), 40 deletions(-)
>
> diff --git a/os/os-dragonfly.h b/os/os-dragonfly.h
> index 5b37a37e19fd..bde39101b536 100644
> --- a/os/os-dragonfly.h
> +++ b/os/os-dragonfly.h
> @@ -175,6 +175,8 @@ static inline int fio_getaffinity(int pid, os_cpu_mask_t *mask)
> #define ioprio_set(which, who, ioprio_class, ioprio) \
> ioprio_set(which, who, ioprio)
>
> +#define ioprio(ioprio) (ioprio)
> +
> static inline int blockdev_size(struct fio_file *f, unsigned long long *bytes)
> {
> struct partinfo pi;
> diff --git a/os/os-linux.h b/os/os-linux.h
> index 7a78b42d4d10..2f9f7e796dac 100644
> --- a/os/os-linux.h
> +++ b/os/os-linux.h
> @@ -153,6 +153,9 @@ static inline int ioprio_set(int which, int who, int ioprio_class, int ioprio)
> ioprio_value(ioprio_class, ioprio));
> }
>
> +#define ioprio_class(ioprio) ((ioprio) >> IOPRIO_CLASS_SHIFT)
> +#define ioprio(ioprio) ((ioprio) & 7)
> +
> #ifndef CONFIG_HAVE_GETTID
> static inline int gettid(void)
> {
> diff --git a/os/os.h b/os/os.h
> index ebaf8af5e2f3..036fc233fe2e 100644
> --- a/os/os.h
> +++ b/os/os.h
> @@ -116,12 +116,14 @@ extern int fio_cpus_split(os_cpu_mask_t *mask, unsigned int cpu);
> #endif
>
> #ifndef FIO_HAVE_IOPRIO_CLASS
> +#define ioprio_class(prio) 0
> #define ioprio_value_is_class_rt(prio) (false)
> #define IOPRIO_MIN_PRIO_CLASS 0
> #define IOPRIO_MAX_PRIO_CLASS 0
> #endif
> #ifndef FIO_HAVE_IOPRIO
> #define ioprio_value(prioclass, prio) (0)
> +#define ioprio(ioprio) 0
> #define ioprio_set(which, who, prioclass, prio) (0)
> #define IOPRIO_MIN_PRIO 0
> #define IOPRIO_MAX_PRIO 0
> diff --git a/stat.c b/stat.c
> index b963973a5824..b60d0a75eec0 100644
> --- a/stat.c
> +++ b/stat.c
> @@ -590,17 +590,18 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
> /* Only print per prio stats if there are >= 2 prios with samples */
> if (get_nr_prios_with_samples(ts, ddir) >= 2) {
> for (i = 0; i < ts->nr_clat_prio[ddir]; i++) {
> - if (calc_lat(&ts->clat_prio[ddir][i].clat_stat, &min,
> - &max, &mean, &dev)) {
> - char buf[64];
> + char buf[64];
>
> - snprintf(buf, sizeof(buf),
> - "%s prio %u/%u",
> - clat_type,
> - ts->clat_prio[ddir][i].ioprio >> 13,
> - ts->clat_prio[ddir][i].ioprio & 7);
> - display_lat(buf, min, max, mean, dev, out);
> - }
> + if (!calc_lat(&ts->clat_prio[ddir][i].clat_stat, &min,
> + &max, &mean, &dev))
> + continue;
> +
> + snprintf(buf, sizeof(buf),
> + "%s prio %u/%u",
> + clat_type,
> + ioprio_class(ts->clat_prio[ddir][i].ioprio),
> + ioprio(ts->clat_prio[ddir][i].ioprio));
> + display_lat(buf, min, max, mean, dev, out);
> }
> }
>
> @@ -632,20 +633,22 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
> /* Only print per prio stats if there are >= 2 prios with samples */
> if (get_nr_prios_with_samples(ts, ddir) >= 2) {
> for (i = 0; i < ts->nr_clat_prio[ddir]; i++) {
> - uint64_t prio_samples = ts->clat_prio[ddir][i].clat_stat.samples;
> -
> - if (prio_samples > 0) {
> - snprintf(prio_name, sizeof(prio_name),
> - "%s prio %u/%u (%.2f%% of IOs)",
> - clat_type,
> - ts->clat_prio[ddir][i].ioprio >> 13,
> - ts->clat_prio[ddir][i].ioprio & 7,
> - 100. * (double) prio_samples / (double) samples);
> - show_clat_percentiles(ts->clat_prio[ddir][i].io_u_plat,
> - prio_samples, ts->percentile_list,
> - ts->percentile_precision,
> - prio_name, out);
> - }
> + uint64_t prio_samples =
> + ts->clat_prio[ddir][i].clat_stat.samples;
> +
> + if (!prio_samples)
> + continue;
> +
> + snprintf(prio_name, sizeof(prio_name),
> + "%s prio %u/%u (%.2f%% of IOs)",
> + clat_type,
> + ioprio_class(ts->clat_prio[ddir][i].ioprio),
> + ioprio(ts->clat_prio[ddir][i].ioprio),
> + 100. * (double) prio_samples / (double) samples);
> + show_clat_percentiles(ts->clat_prio[ddir][i].io_u_plat,
> + prio_samples, ts->percentile_list,
> + ts->percentile_precision,
> + prio_name, out);
> }
> }
> }
> @@ -1508,22 +1511,24 @@ static void add_ddir_status_json(struct thread_stat *ts,
> json_object_add_value_array(dir_object, "prios", array);
>
> for (i = 0; i < ts->nr_clat_prio[ddir]; i++) {
> - if (ts->clat_prio[ddir][i].clat_stat.samples > 0) {
> - struct json_object *obj = json_create_object();
> - unsigned long long class, level;
> -
> - class = ts->clat_prio[ddir][i].ioprio >> 13;
> - json_object_add_value_int(obj, "prioclass", class);
> - level = ts->clat_prio[ddir][i].ioprio & 7;
> - json_object_add_value_int(obj, "prio", level);
> -
> - tmp_object = add_ddir_lat_json(ts,
> - ts->clat_percentiles | ts->lat_percentiles,
> - &ts->clat_prio[ddir][i].clat_stat,
> - ts->clat_prio[ddir][i].io_u_plat);
> - json_object_add_value_object(obj, obj_name, tmp_object);
> - json_array_add_value_object(array, obj);
> - }
> + struct json_object *obj;
> +
> + if (!ts->clat_prio[ddir][i].clat_stat.samples)
> + continue;
> +
> + obj = json_create_object();
> +
> + json_object_add_value_int(obj, "prioclass",
> + ioprio_class(ts->clat_prio[ddir][i].ioprio));
> + json_object_add_value_int(obj, "prio",
> + ioprio(ts->clat_prio[ddir][i].ioprio));
> +
> + tmp_object = add_ddir_lat_json(ts,
> + ts->clat_percentiles | ts->lat_percentiles,
> + &ts->clat_prio[ddir][i].clat_stat,
> + ts->clat_prio[ddir][i].io_u_plat);
> + json_object_add_value_object(obj, obj_name, tmp_object);
> + json_array_add_value_object(array, obj);
> }
> }
>
> --
> 2.39.2
>
Reviewed-by: Niklas Cassel <niklas.cassel@xxxxxxx>
