[PATCH 0/4] Add support for IO priority hints

[PATCH 0/4] Add support for IO priority hints

[Damien Le Moal]

Linux kernel 6.5 will add support for IO priority hints, which can be
used for now to specify a command duration limit for block devices
supporting this feature.

This patch series extends fio IO priority options and adds new options
to allow users to specify IO priority hints.

Damien Le Moal (4):
  os-linux: add initial support for IO priority hints
  options: add priohint option
  cmdprio: Add support for per I/O priority hint
  stats: Add hint information to per priority level stats

 HOWTO.rst          | 31 +++++++++++++++++++++++++++----
 backend.c          |  9 ++++++---
 cconv.c            |  2 ++
 engines/cmdprio.c  |  9 ++++++---
 engines/cmdprio.h  |  1 +
 engines/io_uring.c | 25 +++++++++++++++++++++++--
 engines/libaio.c   | 21 +++++++++++++++++++++
 fio.1              | 28 +++++++++++++++++++++++++---
 options.c          | 31 ++++++++++++++++++++++++++++---
 os/os-dragonfly.h  |  4 ++--
 os/os-linux.h      | 21 +++++++++++++++++----
 os/os.h            |  7 +++++--
 stat.c             | 10 +++++++---
 thread_options.h   |  3 ++-
 14 files changed, 172 insertions(+), 30 deletions(-)

-- 
2.41.0

[PATCH 1/4] os-linux: add initial support for IO priority hints

[Damien Le Moal]

From: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Add initial support for Linux to allow specifying a hint for any
priority value. With this change, a priority value becomes the
combination of a priority class, a priority level and a hint.
The generic os.h ioprio manipulation macros, as well as the
os-dragonfly.h ioprio manipulation macros  are modified to ignore this
hint.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 backend.c         |  5 +++--
 engines/cmdprio.c |  4 ++--
 options.c         |  2 +-
 os/os-dragonfly.h |  4 ++--
 os/os-linux.h     | 17 +++++++++++++----
 os/os.h           |  4 ++--
 6 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/backend.c b/backend.c
index d67a4a07..6641441c 100644
--- a/backend.c
+++ b/backend.c
@@ -1791,12 +1791,13 @@ static void *thread_main(void *data)
 	/* ioprio_set() has to be done before td_io_init() */
 	if (fio_option_is_set(o, ioprio) ||
 	    fio_option_is_set(o, ioprio_class)) {
-		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class, o->ioprio);
+		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class,
+				 o->ioprio, 0);
 		if (ret == -1) {
 			td_verror(td, errno, "ioprio_set");
 			goto err;
 		}
-		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio);
+		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio, 0);
 		td->ts.ioprio = td->ioprio;
 	}
 
diff --git a/engines/cmdprio.c b/engines/cmdprio.c
index 979a81b6..e6ff1fc2 100644
--- a/engines/cmdprio.c
+++ b/engines/cmdprio.c
@@ -342,7 +342,7 @@ static int fio_cmdprio_gen_perc(struct thread_data *td, struct cmdprio *cmdprio)
 		prio = &cmdprio->perc_entry[ddir];
 		prio->perc = options->percentage[ddir];
 		prio->prio = ioprio_value(options->class[ddir],
-					  options->level[ddir]);
+					  options->level[ddir], 0);
 		assign_clat_prio_index(prio, &values[ddir]);
 
 		ret = init_ts_clat_prio(ts, ddir, &values[ddir]);
@@ -400,7 +400,7 @@ static int fio_cmdprio_parse_and_gen_bssplit(struct thread_data *td,
 			goto err;
 
 		implicit_cmdprio = ioprio_value(options->class[ddir],
-						options->level[ddir]);
+						options->level[ddir], 0);
 
 		ret = fio_cmdprio_generate_bsprio_desc(&cmdprio->bsprio_desc[ddir],
 						       &parse_res[ddir],
diff --git a/options.c b/options.c
index a7c4ef6e..c5343d4b 100644
--- a/options.c
+++ b/options.c
@@ -344,7 +344,7 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
 	case 4: /* bs/perc/class/level case */
 		class = min(class, (unsigned int) IOPRIO_MAX_PRIO_CLASS);
 		level = min(level, (unsigned int) IOPRIO_MAX_PRIO);
-		entry->prio = ioprio_value(class, level);
+		entry->prio = ioprio_value(class, level, 0);
 		break;
 	default:
 		log_err("fio: invalid cmdprio_bssplit format\n");
diff --git a/os/os-dragonfly.h b/os/os-dragonfly.h
index bde39101..4ce72539 100644
--- a/os/os-dragonfly.h
+++ b/os/os-dragonfly.h
@@ -171,8 +171,8 @@ static inline int fio_getaffinity(int pid, os_cpu_mask_t *mask)
  * ioprio_set() with 4 arguments, so define fio's ioprio_set() as a macro.
  * Note that there is no idea of class within ioprio_set(2) unlike Linux.
  */
-#define ioprio_value(ioprio_class, ioprio)	(ioprio)
-#define ioprio_set(which, who, ioprio_class, ioprio)	\
+#define ioprio_value(ioprio_class, ioprio, ioprio_hint)	(ioprio)
+#define ioprio_set(which, who, ioprio_class, ioprio, ioprio_hint)	\
 	ioprio_set(which, who, ioprio)
 
 #define ioprio(ioprio)		(ioprio)
diff --git a/os/os-linux.h b/os/os-linux.h
index 2f9f7e79..6f241d09 100644
--- a/os/os-linux.h
+++ b/os/os-linux.h
@@ -125,13 +125,19 @@ enum {
 #define IOPRIO_BITS		16
 #define IOPRIO_CLASS_SHIFT	13
 
+#define IOPRIO_HINT_BITS	10
+#define IOPRIO_HINT_SHIFT	3
+
 #define IOPRIO_MIN_PRIO		0	/* highest priority */
 #define IOPRIO_MAX_PRIO		7	/* lowest priority */
 
 #define IOPRIO_MIN_PRIO_CLASS	0
 #define IOPRIO_MAX_PRIO_CLASS	3
 
-static inline int ioprio_value(int ioprio_class, int ioprio)
+#define IOPRIO_MIN_PRIO_HINT	0
+#define IOPRIO_MAX_PRIO_HINT	((1 << IOPRIO_HINT_BITS) - 1)
+
+static inline int ioprio_value(int ioprio_class, int ioprio, int ioprio_hint)
 {
 	/*
 	 * If no class is set, assume BE
@@ -139,7 +145,9 @@ static inline int ioprio_value(int ioprio_class, int ioprio)
         if (!ioprio_class)
                 ioprio_class = IOPRIO_CLASS_BE;
 
-	return (ioprio_class << IOPRIO_CLASS_SHIFT) | ioprio;
+	return (ioprio_class << IOPRIO_CLASS_SHIFT) |
+		(ioprio_hint << IOPRIO_HINT_SHIFT) |
+		ioprio;
 }
 
 static inline bool ioprio_value_is_class_rt(unsigned int priority)
@@ -147,10 +155,11 @@ static inline bool ioprio_value_is_class_rt(unsigned int priority)
 	return (priority >> IOPRIO_CLASS_SHIFT) == IOPRIO_CLASS_RT;
 }
 
-static inline int ioprio_set(int which, int who, int ioprio_class, int ioprio)
+static inline int ioprio_set(int which, int who, int ioprio_class, int ioprio,
+			     int ioprio_hint)
 {
 	return syscall(__NR_ioprio_set, which, who,
-		       ioprio_value(ioprio_class, ioprio));
+		       ioprio_value(ioprio_class, ioprio, ioprio_hint));
 }
 
 #define ioprio_class(ioprio)	((ioprio) >> IOPRIO_CLASS_SHIFT)
diff --git a/os/os.h b/os/os.h
index 036fc233..2217d5f8 100644
--- a/os/os.h
+++ b/os/os.h
@@ -122,9 +122,9 @@ extern int fio_cpus_split(os_cpu_mask_t *mask, unsigned int cpu);
 #define IOPRIO_MAX_PRIO_CLASS		0
 #endif
 #ifndef FIO_HAVE_IOPRIO
-#define ioprio_value(prioclass, prio)	(0)
+#define ioprio_value(prioclass, prio, priohint)	(0)
 #define ioprio(ioprio)			0
-#define ioprio_set(which, who, prioclass, prio)	(0)
+#define ioprio_set(which, who, prioclass, prio, priohint) (0)
 #define IOPRIO_MIN_PRIO			0
 #define IOPRIO_MAX_PRIO			0
 #endif
-- 
2.41.0

[PATCH 2/4] options: add priohint option

[Damien Le Moal]

From: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Introduce the new option priohint to allow a user to specify an I/O
priority hint applying to all IOs issued by a job.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 HOWTO.rst        |  6 ++++++
 backend.c        |  8 +++++---
 cconv.c          |  2 ++
 fio.1            |  4 ++++
 options.c        | 18 ++++++++++++++++++
 thread_options.h |  3 ++-
 6 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 2e1e55c2..359d606b 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3409,6 +3409,12 @@ Threads, processes and job synchronization
 	priority setting, see I/O engine specific :option:`cmdprio_percentage`
 	and :option:`cmdprio_class` options.
 
+.. option:: priohint=int
+
+	Set the I/O priority hint. See man :manpage:`ionice(1)`. For per-command
+	priority setting, see the I/O engine specific :option:`cmdprio_hint`
+	option.
+
 .. option:: cpus_allowed=str
 
 	Controls the same options as :option:`cpumask`, but accepts a textual
diff --git a/backend.c b/backend.c
index 6641441c..8e7bb70d 100644
--- a/backend.c
+++ b/backend.c
@@ -1790,14 +1790,16 @@ static void *thread_main(void *data)
 
 	/* ioprio_set() has to be done before td_io_init() */
 	if (fio_option_is_set(o, ioprio) ||
-	    fio_option_is_set(o, ioprio_class)) {
+	    fio_option_is_set(o, ioprio_class) ||
+	    fio_option_is_set(o, ioprio_hint)) {
 		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class,
-				 o->ioprio, 0);
+				 o->ioprio, o->ioprio_hint);
 		if (ret == -1) {
 			td_verror(td, errno, "ioprio_set");
 			goto err;
 		}
-		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio, 0);
+		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio,
+					  o->ioprio_hint);
 		td->ts.ioprio = td->ioprio;
 	}
 
diff --git a/cconv.c b/cconv.c
index 9095d519..9ad8d6bb 100644
--- a/cconv.c
+++ b/cconv.c
@@ -281,6 +281,7 @@ int convert_thread_options_to_cpu(struct thread_options *o,
 	o->nice = le32_to_cpu(top->nice);
 	o->ioprio = le32_to_cpu(top->ioprio);
 	o->ioprio_class = le32_to_cpu(top->ioprio_class);
+	o->ioprio_hint = le32_to_cpu(top->ioprio_hint);
 	o->file_service_type = le32_to_cpu(top->file_service_type);
 	o->group_reporting = le32_to_cpu(top->group_reporting);
 	o->stats = le32_to_cpu(top->stats);
@@ -495,6 +496,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
 	top->nice = cpu_to_le32(o->nice);
 	top->ioprio = cpu_to_le32(o->ioprio);
 	top->ioprio_class = cpu_to_le32(o->ioprio_class);
+	top->ioprio_hint = cpu_to_le32(o->ioprio_hint);
 	top->file_service_type = cpu_to_le32(o->file_service_type);
 	top->group_reporting = cpu_to_le32(o->group_reporting);
 	top->stats = cpu_to_le32(o->stats);
diff --git a/fio.1 b/fio.1
index 73b7e8c9..6dbdb293 100644
--- a/fio.1
+++ b/fio.1
@@ -3116,6 +3116,10 @@ Set the I/O priority class. See man \fBionice\fR\|(1). For per-command
 priority setting, see the I/O engine specific `cmdprio_percentage` and
 `cmdprio_class` options.
 .TP
+.BI priohint \fR=\fPint
+Set the I/O priority hint. See man \fBionice\fR\|(1). For per-command
+priority setting, see the I/O engine specific `cmdprio_hint` option.
+.TP
 .BI cpus_allowed \fR=\fPstr
 Controls the same options as \fBcpumask\fR, but accepts a textual
 specification of the permitted CPUs instead and CPUs are indexed from 0. So
diff --git a/options.c b/options.c
index c5343d4b..c1246297 100644
--- a/options.c
+++ b/options.c
@@ -3786,6 +3786,18 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.category = FIO_OPT_C_GENERAL,
 		.group	= FIO_OPT_G_CRED,
 	},
+	{
+		.name	= "priohint",
+		.lname	= "I/O nice priority hint",
+		.type	= FIO_OPT_INT,
+		.off1	= offsetof(struct thread_options, ioprio_hint),
+		.help	= "Set job IO priority hint",
+		.minval	= IOPRIO_MIN_PRIO_HINT,
+		.maxval	= IOPRIO_MAX_PRIO_HINT,
+		.interval = 1,
+		.category = FIO_OPT_C_GENERAL,
+		.group	= FIO_OPT_G_CRED,
+	},
 #else
 	{
 		.name	= "prioclass",
@@ -3793,6 +3805,12 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.type	= FIO_OPT_UNSUPPORTED,
 		.help	= "Your platform does not support IO priority classes",
 	},
+	{
+		.name	= "priohint",
+		.lname	= "I/O nice priority hint",
+		.type	= FIO_OPT_UNSUPPORTED,
+		.help	= "Your platform does not support IO priority hints",
+	},
 #endif
 	{
 		.name	= "thinktime",
diff --git a/thread_options.h b/thread_options.h
index a24ebee6..4fb72651 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -248,6 +248,7 @@ struct thread_options {
 	unsigned int nice;
 	unsigned int ioprio;
 	unsigned int ioprio_class;
+	unsigned int ioprio_hint;
 	unsigned int file_service_type;
 	unsigned int group_reporting;
 	unsigned int stats;
@@ -567,6 +568,7 @@ struct thread_options_pack {
 	uint32_t nice;
 	uint32_t ioprio;
 	uint32_t ioprio_class;
+	uint32_t ioprio_hint;
 	uint32_t file_service_type;
 	uint32_t group_reporting;
 	uint32_t stats;
@@ -600,7 +602,6 @@ struct thread_options_pack {
 	uint32_t lat_percentiles;
 	uint32_t slat_percentiles;
 	uint32_t percentile_precision;
-	uint32_t pad5;
 	fio_fp64_t percentile_list[FIO_IO_U_LIST_MAX_LEN];
 
 	uint8_t read_iolog_file[FIO_TOP_STR_MAX];
-- 
2.41.0

[PATCH 3/4] cmdprio: Add support for per I/O priority hint

[Damien Le Moal]

From: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Introduce the new option cmdprio_hint to allow specifying I/O priority
hints per IO with the iouring and libaio IO engines. A third acceptable
format for the cmdprio_bssplit option is also introduced to allow
specifying an IO hint in addition to a priority class and level.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 HOWTO.rst          | 25 +++++++++++++++++++++----
 engines/cmdprio.c  |  9 ++++++---
 engines/cmdprio.h  |  1 +
 engines/io_uring.c | 25 +++++++++++++++++++++++--
 engines/libaio.c   | 21 +++++++++++++++++++++
 fio.1              | 24 +++++++++++++++++++++---
 options.c          | 13 ++++++++++---
 7 files changed, 103 insertions(+), 15 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 359d606b..16985269 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2275,6 +2275,16 @@ with the caveat that when used on the command line, they must come after the
 	reads and writes. See :manpage:`ionice(1)`. See also the
 	:option:`prioclass` option.
 
+.. option:: cmdprio_hint=int[,int] : [io_uring] [libaio]
+
+	Set the I/O priority hint to use for I/Os that must be issued with
+	a priority when :option:`cmdprio_percentage` or
+	:option:`cmdprio_bssplit` is set. If not specified when
+	:option:`cmdprio_percentage` or :option:`cmdprio_bssplit` is set,
+	this defaults to 0 (no hint). A single value applies to reads and
+	writes. Comma-separated values may be specified for reads and writes.
+	See also the :option:`priohint` option.
+
 .. option:: cmdprio=int[,int] : [io_uring] [libaio]
 
 	Set the I/O priority value to use for I/Os that must be issued with
@@ -2301,9 +2311,9 @@ with the caveat that when used on the command line, they must come after the
 
 		cmdprio_bssplit=blocksize/percentage:blocksize/percentage
 
-	In this case, each entry will use the priority class and priority
-	level defined by the options :option:`cmdprio_class` and
-	:option:`cmdprio` respectively.
+	In this case, each entry will use the priority class, priority hint
+	and priority level defined by the options :option:`cmdprio_class`,
+        :option:`cmdprio` and :option:`cmdprio_hint` respectively.
 
 	The second accepted format for this option is:
 
@@ -2314,7 +2324,14 @@ with the caveat that when used on the command line, they must come after the
 	accepted format does not restrict all entries to have the same priority
 	class and priority level.
 
-	For both formats, only the read and write data directions are supported,
+	The third accepted format for this option is:
+
+		cmdprio_bssplit=blocksize/percentage/class/level/hint:...
+
+	This is an extension of the second accepted format that allows to also
+	specify a priority hint.
+
+	For all formats, only the read and write data directions are supported,
 	values for trim IOs are ignored. This option is mutually exclusive with
 	the :option:`cmdprio_percentage` option.
 
diff --git a/engines/cmdprio.c b/engines/cmdprio.c
index e6ff1fc2..153e3691 100644
--- a/engines/cmdprio.c
+++ b/engines/cmdprio.c
@@ -267,7 +267,8 @@ static int fio_cmdprio_percentage(struct cmdprio *cmdprio, struct io_u *io_u,
  * to be set. If the random percentage value is within the user specified
  * percentage of I/Os that should use a cmdprio priority value (rather than
  * the default priority), then this function updates the io_u with an ioprio
- * value as defined by the cmdprio/cmdprio_class or cmdprio_bssplit options.
+ * value as defined by the cmdprio/cmdprio_hint/cmdprio_class or
+ * cmdprio_bssplit options.
  *
  * Return true if the io_u ioprio was changed and false otherwise.
  */
@@ -342,7 +343,8 @@ static int fio_cmdprio_gen_perc(struct thread_data *td, struct cmdprio *cmdprio)
 		prio = &cmdprio->perc_entry[ddir];
 		prio->perc = options->percentage[ddir];
 		prio->prio = ioprio_value(options->class[ddir],
-					  options->level[ddir], 0);
+					  options->level[ddir],
+					  options->hint[ddir]);
 		assign_clat_prio_index(prio, &values[ddir]);
 
 		ret = init_ts_clat_prio(ts, ddir, &values[ddir]);
@@ -400,7 +402,8 @@ static int fio_cmdprio_parse_and_gen_bssplit(struct thread_data *td,
 			goto err;
 
 		implicit_cmdprio = ioprio_value(options->class[ddir],
-						options->level[ddir], 0);
+						options->level[ddir],
+						options->hint[ddir]);
 
 		ret = fio_cmdprio_generate_bsprio_desc(&cmdprio->bsprio_desc[ddir],
 						       &parse_res[ddir],
diff --git a/engines/cmdprio.h b/engines/cmdprio.h
index 755da8d0..9b351828 100644
--- a/engines/cmdprio.h
+++ b/engines/cmdprio.h
@@ -39,6 +39,7 @@ struct cmdprio_options {
 	unsigned int percentage[CMDPRIO_RWDIR_CNT];
 	unsigned int class[CMDPRIO_RWDIR_CNT];
 	unsigned int level[CMDPRIO_RWDIR_CNT];
+	unsigned int hint[CMDPRIO_RWDIR_CNT];
 	char *bssplit_str;
 };
 
diff --git a/engines/io_uring.c b/engines/io_uring.c
index 5021239e..975a0a4d 100644
--- a/engines/io_uring.c
+++ b/engines/io_uring.c
@@ -157,6 +157,21 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_IOURING,
 	},
+	{
+		.name	= "cmdprio_hint",
+		.lname	= "Asynchronous I/O priority hint",
+		.type	= FIO_OPT_INT,
+		.off1	= offsetof(struct ioring_options,
+				   cmdprio_options.hint[DDIR_READ]),
+		.off2	= offsetof(struct ioring_options,
+				   cmdprio_options.hint[DDIR_WRITE]),
+		.help	= "Set asynchronous IO priority hint",
+		.minval	= IOPRIO_MIN_PRIO_HINT,
+		.maxval	= IOPRIO_MAX_PRIO_HINT,
+		.interval = 1,
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_IOURING,
+	},
 	{
 		.name	= "cmdprio",
 		.lname	= "Asynchronous I/O priority level",
@@ -195,6 +210,12 @@ static struct fio_option options[] = {
 		.type	= FIO_OPT_UNSUPPORTED,
 		.help	= "Your platform does not support I/O priority classes",
 	},
+	{
+		.name	= "cmdprio_hint",
+		.lname	= "Asynchronous I/O priority hint",
+		.type	= FIO_OPT_UNSUPPORTED,
+		.help	= "Your platform does not support I/O priority classes",
+	},
 	{
 		.name	= "cmdprio",
 		.lname	= "Asynchronous I/O priority level",
@@ -365,8 +386,8 @@ static int fio_ioring_prep(struct thread_data *td, struct io_u *io_u)
 		/*
 		 * Since io_uring can have a submission context (sqthread_poll)
 		 * that is different from the process context, we cannot rely on
-		 * the IO priority set by ioprio_set() (option prio/prioclass)
-		 * to be inherited.
+		 * the IO priority set by ioprio_set() (options prio, prioclass,
+		 * and priohint) to be inherited.
 		 * td->ioprio will have the value of the "default prio", so set
 		 * this unconditionally. This value might get overridden by
 		 * fio_ioring_cmdprio_prep() if the option cmdprio_percentage or
diff --git a/engines/libaio.c b/engines/libaio.c
index 6a0745aa..c52d05fe 100644
--- a/engines/libaio.c
+++ b/engines/libaio.c
@@ -102,6 +102,21 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBAIO,
 	},
+	{
+		.name	= "cmdprio_hint",
+		.lname	= "Asynchronous I/O priority hint",
+		.type	= FIO_OPT_INT,
+		.off1	= offsetof(struct libaio_options,
+				   cmdprio_options.hint[DDIR_READ]),
+		.off2	= offsetof(struct libaio_options,
+				   cmdprio_options.hint[DDIR_WRITE]),
+		.help	= "Set asynchronous IO priority hint",
+		.minval	= IOPRIO_MIN_PRIO_HINT,
+		.maxval	= IOPRIO_MAX_PRIO_HINT,
+		.interval = 1,
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBAIO,
+	},
 	{
 		.name	= "cmdprio",
 		.lname	= "Asynchronous I/O priority level",
@@ -140,6 +155,12 @@ static struct fio_option options[] = {
 		.type	= FIO_OPT_UNSUPPORTED,
 		.help	= "Your platform does not support I/O priority classes",
 	},
+	{
+		.name	= "cmdprio_hint",
+		.lname	= "Asynchronous I/O priority hint",
+		.type	= FIO_OPT_UNSUPPORTED,
+		.help	= "Your platform does not support I/O priority classes",
+	},
 	{
 		.name	= "cmdprio",
 		.lname	= "Asynchronous I/O priority level",
diff --git a/fio.1 b/fio.1
index 6dbdb293..72afff65 100644
--- a/fio.1
+++ b/fio.1
@@ -2072,6 +2072,14 @@ is set, this defaults to the highest priority class. A single value applies
 to reads and writes. Comma-separated values may be specified for reads and
 writes. See man \fBionice\fR\|(1). See also the \fBprioclass\fR option.
 .TP
+.BI (io_uring,libaio)cmdprio_hint \fR=\fPint[,int]
+Set the I/O priority hint to use for I/Os that must be issued with a
+priority when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR is set.
+If not specified when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR
+is set, this defaults to 0 (no hint). A single value applies to reads and
+writes. Comma-separated values may be specified for reads and writes.
+See also the \fBpriohint\fR option.
+.TP
 .BI (io_uring,libaio)cmdprio \fR=\fPint[,int]
 Set the I/O priority value to use for I/Os that must be issued with a
 priority when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR is set.
@@ -2097,8 +2105,9 @@ The first accepted format for this option is the same as the format of the
 cmdprio_bssplit=blocksize/percentage:blocksize/percentage
 .RE
 .P
-In this case, each entry will use the priority class and priority level defined
-by the options \fBcmdprio_class\fR and \fBcmdprio\fR respectively.
+In this case, each entry will use the priority class, priority hint and
+priority level defined by the options \fBcmdprio_class\fR, \fBcmdprio\fR
+and \fBcmdprio_hint\fR respectively.
 .P
 The second accepted format for this option is:
 .RS
@@ -2111,7 +2120,16 @@ entry. In comparison with the first accepted format, the second accepted format
 does not restrict all entries to have the same priority class and priority
 level.
 .P
-For both formats, only the read and write data directions are supported, values
+The third accepted format for this option is:
+.RS
+.P
+cmdprio_bssplit=blocksize/percentage/class/level/hint:...
+.RE
+.P
+This is an extension of the second accepted format that allows to also
+specify a priority hint.
+.P
+For all formats, only the read and write data directions are supported, values
 for trim IOs are ignored. This option is mutually exclusive with the
 \fBcmdprio_percentage\fR option.
 .RE
diff --git a/options.c b/options.c
index c1246297..3fcbd2b3 100644
--- a/options.c
+++ b/options.c
@@ -313,15 +313,17 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
 	int matches = 0;
 	char *bs_str = NULL;
 	long long bs_val;
-	unsigned int perc = 0, class, level;
+	unsigned int perc = 0, class, level, hint;
 
 	/*
 	 * valid entry formats:
 	 * bs/ - %s/ - set perc to 0, prio to -1.
 	 * bs/perc - %s/%u - set prio to -1.
 	 * bs/perc/class/level - %s/%u/%u/%u
+	 * bs/perc/class/level/hint - %s/%u/%u/%u/%u
 	 */
-	matches = sscanf(str, "%m[^/]/%u/%u/%u", &bs_str, &perc, &class, &level);
+	matches = sscanf(str, "%m[^/]/%u/%u/%u/%u",
+			 &bs_str, &perc, &class, &level, &hint);
 	if (matches < 1) {
 		log_err("fio: invalid cmdprio_bssplit format\n");
 		return 1;
@@ -342,9 +344,14 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
 	case 2: /* bs/perc case */
 		break;
 	case 4: /* bs/perc/class/level case */
+	case 5: /* bs/perc/class/level/hint case */
 		class = min(class, (unsigned int) IOPRIO_MAX_PRIO_CLASS);
 		level = min(level, (unsigned int) IOPRIO_MAX_PRIO);
-		entry->prio = ioprio_value(class, level, 0);
+		if (matches == 5)
+			hint = min(hint, (unsigned int) IOPRIO_MAX_PRIO_HINT);
+		else
+			hint = 0;
+		entry->prio = ioprio_value(class, level, hint);
 		break;
 	default:
 		log_err("fio: invalid cmdprio_bssplit format\n");
-- 
2.41.0

[PATCH 4/4] stats: Add hint information to per priority level stats

[Damien Le Moal]

From: Damien Le Moal <damien.lemoal@opensource.wdc.com>

Introduce the OS dependent ioprio_hint() macro to extract an IO
priority hint from an ioprio value. This macro defaults to always
returning 0 for OSes that do not support IO priority hints.

The json and standard output stats are modified to display the hint
value together with the priority class and level.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 os/os-linux.h |  4 ++++
 os/os.h       |  3 +++
 stat.c        | 10 +++++++---
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/os/os-linux.h b/os/os-linux.h
index 6f241d09..b9c07891 100644
--- a/os/os-linux.h
+++ b/os/os-linux.h
@@ -150,6 +150,10 @@ static inline int ioprio_value(int ioprio_class, int ioprio, int ioprio_hint)
 		ioprio;
 }
 
+#define ioprio_class(ioprio)	((ioprio) >> IOPRIO_CLASS_SHIFT)
+#define ioprio_hint(ioprio)	(((ioprio) >> IOPRIO_HINT_SHIFT) & 0x3ff)
+#define ioprio(ioprio)		((ioprio) & 7)
+
 static inline bool ioprio_value_is_class_rt(unsigned int priority)
 {
 	return (priority >> IOPRIO_CLASS_SHIFT) == IOPRIO_CLASS_RT;
diff --git a/os/os.h b/os/os.h
index 2217d5f8..0f182324 100644
--- a/os/os.h
+++ b/os/os.h
@@ -120,6 +120,9 @@ extern int fio_cpus_split(os_cpu_mask_t *mask, unsigned int cpu);
 #define ioprio_value_is_class_rt(prio)	(false)
 #define IOPRIO_MIN_PRIO_CLASS		0
 #define IOPRIO_MAX_PRIO_CLASS		0
+#define ioprio_hint(prio)		0
+#define IOPRIO_MIN_PRIO_HINT		0
+#define IOPRIO_MAX_PRIO_HINT		0
 #endif
 #ifndef FIO_HAVE_IOPRIO
 #define ioprio_value(prioclass, prio, priohint)	(0)
diff --git a/stat.c b/stat.c
index 015b8e28..4f943602 100644
--- a/stat.c
+++ b/stat.c
@@ -597,10 +597,11 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
 				continue;
 
 			snprintf(buf, sizeof(buf),
-				 "%s prio %u/%u",
+				 "%s prio %u/%u/%u",
 				 clat_type,
 				 ioprio_class(ts->clat_prio[ddir][i].ioprio),
-				 ioprio(ts->clat_prio[ddir][i].ioprio));
+				 ioprio(ts->clat_prio[ddir][i].ioprio),
+				 ioprio_hint(ts->clat_prio[ddir][i].ioprio));
 			display_lat(buf, min, max, mean, dev, out);
 		}
 	}
@@ -640,10 +641,11 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
 					continue;
 
 				snprintf(prio_name, sizeof(prio_name),
-					 "%s prio %u/%u (%.2f%% of IOs)",
+					 "%s prio %u/%u/%u (%.2f%% of IOs)",
 					 clat_type,
 					 ioprio_class(ts->clat_prio[ddir][i].ioprio),
 					 ioprio(ts->clat_prio[ddir][i].ioprio),
+					 ioprio_hint(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,
@@ -1522,6 +1524,8 @@ static void add_ddir_status_json(struct thread_stat *ts,
 				ioprio_class(ts->clat_prio[ddir][i].ioprio));
 			json_object_add_value_int(obj, "prio",
 				ioprio(ts->clat_prio[ddir][i].ioprio));
+			json_object_add_value_int(obj, "priohint",
+				ioprio_hint(ts->clat_prio[ddir][i].ioprio));
 
 			tmp_object = add_ddir_lat_json(ts,
 					ts->clat_percentiles | ts->lat_percentiles,
-- 
2.41.0

RE: [PATCH 0/4] Add support for IO priority hints

[Avri Altman]

> Linux kernel 6.5 will add support for IO priority hints, which can be used for
> now to specify a command duration limit for block devices supporting this
> feature.
> 
> This patch series extends fio IO priority options and adds new options to allow
> users to specify IO priority hints.
Maybe while at it, remove the write_hint option that was deprecated in the kernel a while ago.

Otherwise, this series looks good to me.

Thanks,
Avri

> 
> Damien Le Moal (4):
>   os-linux: add initial support for IO priority hints
>   options: add priohint option
>   cmdprio: Add support for per I/O priority hint
>   stats: Add hint information to per priority level stats
> 
>  HOWTO.rst          | 31 +++++++++++++++++++++++++++----
>  backend.c          |  9 ++++++---
>  cconv.c            |  2 ++
>  engines/cmdprio.c  |  9 ++++++---
>  engines/cmdprio.h  |  1 +
>  engines/io_uring.c | 25 +++++++++++++++++++++++--
>  engines/libaio.c   | 21 +++++++++++++++++++++
>  fio.1              | 28 +++++++++++++++++++++++++---
>  options.c          | 31 ++++++++++++++++++++++++++++---
>  os/os-dragonfly.h  |  4 ++--
>  os/os-linux.h      | 21 +++++++++++++++++----
>  os/os.h            |  7 +++++--
>  stat.c             | 10 +++++++---
>  thread_options.h   |  3 ++-
>  14 files changed, 172 insertions(+), 30 deletions(-)
> 
> --
> 2.41.0

Re: [PATCH 0/4] Add support for IO priority hints

[Damien Le Moal]

On 7/6/23 17:43, Avri Altman wrote:
>> Linux kernel 6.5 will add support for IO priority hints, which can be used
>> for now to specify a command duration limit for block devices supporting
>> this feature.
>> 
>> This patch series extends fio IO priority options and adds new options to
>> allow users to specify IO priority hints.
> Maybe while at it, remove the write_hint option that was deprecated in the
> kernel a while ago.

write_hint is completely unrelated to IO priority. So removing that option
should be a different patch series.

In any case, write_hint is indeed not supported anymore by current kernels but
it can still be used with older kernels. Removing that option from fio is thus
not a good idea at all as that would prevent using write_hints with older
kernels and drives that support this feature. There are cases of that in the
field. So nack on removing write_hint.

> 
> Otherwise, this series looks good to me.
> 
> Thanks, Avri
> 
>> 
>> Damien Le Moal (4): os-linux: add initial support for IO priority hints 
>> options: add priohint option cmdprio: Add support for per I/O priority
>> hint stats: Add hint information to per priority level stats
>> 
>> HOWTO.rst          | 31 +++++++++++++++++++++++++++---- backend.c
>> |  9 ++++++--- cconv.c            |  2 ++ engines/cmdprio.c  |  9
>> ++++++--- engines/cmdprio.h  |  1 + engines/io_uring.c | 25
>> +++++++++++++++++++++++-- engines/libaio.c   | 21 +++++++++++++++++++++ 
>> fio.1              | 28 +++++++++++++++++++++++++--- options.c          |
>> 31 ++++++++++++++++++++++++++++--- os/os-dragonfly.h  |  4 ++-- 
>> os/os-linux.h      | 21 +++++++++++++++++---- os/os.h            |  7
>> +++++-- stat.c             | 10 +++++++--- thread_options.h   |  3 ++- 14
>> files changed, 172 insertions(+), 30 deletions(-)
>> 
>> -- 2.41.0
> 

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH 1/4] os-linux: add initial support for IO priority hints

[Niklas Cassel]

On Thu, Jul 06, 2023 at 07:12:59AM +0900, Damien Le Moal wrote:
> From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> 
> Add initial support for Linux to allow specifying a hint for any
> priority value. With this change, a priority value becomes the
> combination of a priority class, a priority level and a hint.
> The generic os.h ioprio manipulation macros, as well as the
> os-dragonfly.h ioprio manipulation macros  are modified to ignore this
> hint.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  backend.c         |  5 +++--
>  engines/cmdprio.c |  4 ++--
>  options.c         |  2 +-
>  os/os-dragonfly.h |  4 ++--
>  os/os-linux.h     | 17 +++++++++++++----
>  os/os.h           |  4 ++--
>  6 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/backend.c b/backend.c
> index d67a4a07..6641441c 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -1791,12 +1791,13 @@ static void *thread_main(void *data)
>  	/* ioprio_set() has to be done before td_io_init() */
>  	if (fio_option_is_set(o, ioprio) ||
>  	    fio_option_is_set(o, ioprio_class)) {
> -		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class, o->ioprio);
> +		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class,
> +				 o->ioprio, 0);
>  		if (ret == -1) {
>  			td_verror(td, errno, "ioprio_set");
>  			goto err;
>  		}
> -		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio);
> +		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio, 0);
>  		td->ts.ioprio = td->ioprio;
>  	}
>  
> diff --git a/engines/cmdprio.c b/engines/cmdprio.c
> index 979a81b6..e6ff1fc2 100644
> --- a/engines/cmdprio.c
> +++ b/engines/cmdprio.c
> @@ -342,7 +342,7 @@ static int fio_cmdprio_gen_perc(struct thread_data *td, struct cmdprio *cmdprio)
>  		prio = &cmdprio->perc_entry[ddir];
>  		prio->perc = options->percentage[ddir];
>  		prio->prio = ioprio_value(options->class[ddir],
> -					  options->level[ddir]);
> +					  options->level[ddir], 0);
>  		assign_clat_prio_index(prio, &values[ddir]);
>  
>  		ret = init_ts_clat_prio(ts, ddir, &values[ddir]);
> @@ -400,7 +400,7 @@ static int fio_cmdprio_parse_and_gen_bssplit(struct thread_data *td,
>  			goto err;
>  
>  		implicit_cmdprio = ioprio_value(options->class[ddir],
> -						options->level[ddir]);
> +						options->level[ddir], 0);
>  
>  		ret = fio_cmdprio_generate_bsprio_desc(&cmdprio->bsprio_desc[ddir],
>  						       &parse_res[ddir],
> diff --git a/options.c b/options.c
> index a7c4ef6e..c5343d4b 100644
> --- a/options.c
> +++ b/options.c
> @@ -344,7 +344,7 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
>  	case 4: /* bs/perc/class/level case */
>  		class = min(class, (unsigned int) IOPRIO_MAX_PRIO_CLASS);
>  		level = min(level, (unsigned int) IOPRIO_MAX_PRIO);
> -		entry->prio = ioprio_value(class, level);
> +		entry->prio = ioprio_value(class, level, 0);
>  		break;
>  	default:
>  		log_err("fio: invalid cmdprio_bssplit format\n");
> diff --git a/os/os-dragonfly.h b/os/os-dragonfly.h
> index bde39101..4ce72539 100644
> --- a/os/os-dragonfly.h
> +++ b/os/os-dragonfly.h
> @@ -171,8 +171,8 @@ static inline int fio_getaffinity(int pid, os_cpu_mask_t *mask)
>   * ioprio_set() with 4 arguments, so define fio's ioprio_set() as a macro.
>   * Note that there is no idea of class within ioprio_set(2) unlike Linux.
>   */
> -#define ioprio_value(ioprio_class, ioprio)	(ioprio)
> -#define ioprio_set(which, who, ioprio_class, ioprio)	\
> +#define ioprio_value(ioprio_class, ioprio, ioprio_hint)	(ioprio)
> +#define ioprio_set(which, who, ioprio_class, ioprio, ioprio_hint)	\
>  	ioprio_set(which, who, ioprio)
>  
>  #define ioprio(ioprio)		(ioprio)
> diff --git a/os/os-linux.h b/os/os-linux.h
> index 2f9f7e79..6f241d09 100644
> --- a/os/os-linux.h
> +++ b/os/os-linux.h
> @@ -125,13 +125,19 @@ enum {
>  #define IOPRIO_BITS		16
>  #define IOPRIO_CLASS_SHIFT	13
>  
> +#define IOPRIO_HINT_BITS	10
> +#define IOPRIO_HINT_SHIFT	3
> +
>  #define IOPRIO_MIN_PRIO		0	/* highest priority */
>  #define IOPRIO_MAX_PRIO		7	/* lowest priority */
>  
>  #define IOPRIO_MIN_PRIO_CLASS	0
>  #define IOPRIO_MAX_PRIO_CLASS	3
>  
> -static inline int ioprio_value(int ioprio_class, int ioprio)
> +#define IOPRIO_MIN_PRIO_HINT	0
> +#define IOPRIO_MAX_PRIO_HINT	((1 << IOPRIO_HINT_BITS) - 1)
> +
> +static inline int ioprio_value(int ioprio_class, int ioprio, int ioprio_hint)
>  {
>  	/*
>  	 * If no class is set, assume BE
> @@ -139,7 +145,9 @@ static inline int ioprio_value(int ioprio_class, int ioprio)
>          if (!ioprio_class)
>                  ioprio_class = IOPRIO_CLASS_BE;
>  
> -	return (ioprio_class << IOPRIO_CLASS_SHIFT) | ioprio;
> +	return (ioprio_class << IOPRIO_CLASS_SHIFT) |
> +		(ioprio_hint << IOPRIO_HINT_SHIFT) |
> +		ioprio;
>  }
>  
>  static inline bool ioprio_value_is_class_rt(unsigned int priority)
> @@ -147,10 +155,11 @@ static inline bool ioprio_value_is_class_rt(unsigned int priority)
>  	return (priority >> IOPRIO_CLASS_SHIFT) == IOPRIO_CLASS_RT;
>  }
>  
> -static inline int ioprio_set(int which, int who, int ioprio_class, int ioprio)
> +static inline int ioprio_set(int which, int who, int ioprio_class, int ioprio,
> +			     int ioprio_hint)
>  {
>  	return syscall(__NR_ioprio_set, which, who,
> -		       ioprio_value(ioprio_class, ioprio));
> +		       ioprio_value(ioprio_class, ioprio, ioprio_hint));
>  }
>  
>  #define ioprio_class(ioprio)	((ioprio) >> IOPRIO_CLASS_SHIFT)
> diff --git a/os/os.h b/os/os.h
> index 036fc233..2217d5f8 100644
> --- a/os/os.h
> +++ b/os/os.h
> @@ -122,9 +122,9 @@ extern int fio_cpus_split(os_cpu_mask_t *mask, unsigned int cpu);
>  #define IOPRIO_MAX_PRIO_CLASS		0
>  #endif
>  #ifndef FIO_HAVE_IOPRIO
> -#define ioprio_value(prioclass, prio)	(0)
> +#define ioprio_value(prioclass, prio, priohint)	(0)
>  #define ioprio(ioprio)			0
> -#define ioprio_set(which, who, prioclass, prio)	(0)
> +#define ioprio_set(which, who, prioclass, prio, priohint) (0)
>  #define IOPRIO_MIN_PRIO			0
>  #define IOPRIO_MAX_PRIO			0
>  #endif
> -- 
> 2.41.0
> 

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 2/4] options: add priohint option

[Niklas Cassel]

On Thu, Jul 06, 2023 at 07:13:00AM +0900, Damien Le Moal wrote:
> From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> 
> Introduce the new option priohint to allow a user to specify an I/O
> priority hint applying to all IOs issued by a job.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  HOWTO.rst        |  6 ++++++
>  backend.c        |  8 +++++---
>  cconv.c          |  2 ++
>  fio.1            |  4 ++++
>  options.c        | 18 ++++++++++++++++++
>  thread_options.h |  3 ++-
>  6 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 2e1e55c2..359d606b 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -3409,6 +3409,12 @@ Threads, processes and job synchronization
>  	priority setting, see I/O engine specific :option:`cmdprio_percentage`
>  	and :option:`cmdprio_class` options.
>  
> +.. option:: priohint=int
> +
> +	Set the I/O priority hint. See man :manpage:`ionice(1)`. For per-command
> +	priority setting, see the I/O engine specific :option:`cmdprio_hint`
> +	option.
> +
>  .. option:: cpus_allowed=str
>  
>  	Controls the same options as :option:`cpumask`, but accepts a textual
> diff --git a/backend.c b/backend.c
> index 6641441c..8e7bb70d 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -1790,14 +1790,16 @@ static void *thread_main(void *data)
>  
>  	/* ioprio_set() has to be done before td_io_init() */
>  	if (fio_option_is_set(o, ioprio) ||
> -	    fio_option_is_set(o, ioprio_class)) {
> +	    fio_option_is_set(o, ioprio_class) ||
> +	    fio_option_is_set(o, ioprio_hint)) {
>  		ret = ioprio_set(IOPRIO_WHO_PROCESS, 0, o->ioprio_class,
> -				 o->ioprio, 0);
> +				 o->ioprio, o->ioprio_hint);
>  		if (ret == -1) {
>  			td_verror(td, errno, "ioprio_set");
>  			goto err;
>  		}
> -		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio, 0);
> +		td->ioprio = ioprio_value(o->ioprio_class, o->ioprio,
> +					  o->ioprio_hint);
>  		td->ts.ioprio = td->ioprio;
>  	}
>  
> diff --git a/cconv.c b/cconv.c
> index 9095d519..9ad8d6bb 100644
> --- a/cconv.c
> +++ b/cconv.c
> @@ -281,6 +281,7 @@ int convert_thread_options_to_cpu(struct thread_options *o,
>  	o->nice = le32_to_cpu(top->nice);
>  	o->ioprio = le32_to_cpu(top->ioprio);
>  	o->ioprio_class = le32_to_cpu(top->ioprio_class);
> +	o->ioprio_hint = le32_to_cpu(top->ioprio_hint);
>  	o->file_service_type = le32_to_cpu(top->file_service_type);
>  	o->group_reporting = le32_to_cpu(top->group_reporting);
>  	o->stats = le32_to_cpu(top->stats);
> @@ -495,6 +496,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
>  	top->nice = cpu_to_le32(o->nice);
>  	top->ioprio = cpu_to_le32(o->ioprio);
>  	top->ioprio_class = cpu_to_le32(o->ioprio_class);
> +	top->ioprio_hint = cpu_to_le32(o->ioprio_hint);

1)
When updating cconv.c I think you also need to bump FIO_SERVER_VER.


>  	top->file_service_type = cpu_to_le32(o->file_service_type);
>  	top->group_reporting = cpu_to_le32(o->group_reporting);
>  	top->stats = cpu_to_le32(o->stats);
> diff --git a/fio.1 b/fio.1
> index 73b7e8c9..6dbdb293 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -3116,6 +3116,10 @@ Set the I/O priority class. See man \fBionice\fR\|(1). For per-command
>  priority setting, see the I/O engine specific `cmdprio_percentage` and
>  `cmdprio_class` options.
>  .TP
> +.BI priohint \fR=\fPint
> +Set the I/O priority hint. See man \fBionice\fR\|(1). For per-command
> +priority setting, see the I/O engine specific `cmdprio_hint` option.

2)
Nit: 'cmdprio_hint' is introduced in a later patch series, so it feels
a bit premature to mention it in this patch series.


> +.TP
>  .BI cpus_allowed \fR=\fPstr
>  Controls the same options as \fBcpumask\fR, but accepts a textual
>  specification of the permitted CPUs instead and CPUs are indexed from 0. So
> diff --git a/options.c b/options.c
> index c5343d4b..c1246297 100644
> --- a/options.c
> +++ b/options.c
> @@ -3786,6 +3786,18 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
>  		.category = FIO_OPT_C_GENERAL,
>  		.group	= FIO_OPT_G_CRED,
>  	},
> +	{
> +		.name	= "priohint",
> +		.lname	= "I/O nice priority hint",
> +		.type	= FIO_OPT_INT,
> +		.off1	= offsetof(struct thread_options, ioprio_hint),
> +		.help	= "Set job IO priority hint",
> +		.minval	= IOPRIO_MIN_PRIO_HINT,
> +		.maxval	= IOPRIO_MAX_PRIO_HINT,
> +		.interval = 1,
> +		.category = FIO_OPT_C_GENERAL,
> +		.group	= FIO_OPT_G_CRED,
> +	},
>  #else
>  	{
>  		.name	= "prioclass",
> @@ -3793,6 +3805,12 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
>  		.type	= FIO_OPT_UNSUPPORTED,
>  		.help	= "Your platform does not support IO priority classes",
>  	},
> +	{
> +		.name	= "priohint",
> +		.lname	= "I/O nice priority hint",
> +		.type	= FIO_OPT_UNSUPPORTED,
> +		.help	= "Your platform does not support IO priority hints",
> +	},
>  #endif
>  	{
>  		.name	= "thinktime",
> diff --git a/thread_options.h b/thread_options.h
> index a24ebee6..4fb72651 100644
> --- a/thread_options.h
> +++ b/thread_options.h
> @@ -248,6 +248,7 @@ struct thread_options {
>  	unsigned int nice;
>  	unsigned int ioprio;
>  	unsigned int ioprio_class;
> +	unsigned int ioprio_hint;
>  	unsigned int file_service_type;
>  	unsigned int group_reporting;
>  	unsigned int stats;
> @@ -567,6 +568,7 @@ struct thread_options_pack {
>  	uint32_t nice;
>  	uint32_t ioprio;
>  	uint32_t ioprio_class;
> +	uint32_t ioprio_hint;
>  	uint32_t file_service_type;
>  	uint32_t group_reporting;
>  	uint32_t stats;
> @@ -600,7 +602,6 @@ struct thread_options_pack {
>  	uint32_t lat_percentiles;
>  	uint32_t slat_percentiles;
>  	uint32_t percentile_precision;
> -	uint32_t pad5;
>  	fio_fp64_t percentile_list[FIO_IO_U_LIST_MAX_LEN];
>  
>  	uint8_t read_iolog_file[FIO_TOP_STR_MAX];
> -- 
> 2.41.0
> 

With the two nits fixed:
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 3/4] cmdprio: Add support for per I/O priority hint

[Niklas Cassel]

On Thu, Jul 06, 2023 at 07:13:01AM +0900, Damien Le Moal wrote:
> From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> 
> Introduce the new option cmdprio_hint to allow specifying I/O priority
> hints per IO with the iouring and libaio IO engines. A third acceptable
> format for the cmdprio_bssplit option is also introduced to allow
> specifying an IO hint in addition to a priority class and level.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  HOWTO.rst          | 25 +++++++++++++++++++++----
>  engines/cmdprio.c  |  9 ++++++---
>  engines/cmdprio.h  |  1 +
>  engines/io_uring.c | 25 +++++++++++++++++++++++--
>  engines/libaio.c   | 21 +++++++++++++++++++++
>  fio.1              | 24 +++++++++++++++++++++---
>  options.c          | 13 ++++++++++---
>  7 files changed, 103 insertions(+), 15 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 359d606b..16985269 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2275,6 +2275,16 @@ with the caveat that when used on the command line, they must come after the
>  	reads and writes. See :manpage:`ionice(1)`. See also the
>  	:option:`prioclass` option.
>  
> +.. option:: cmdprio_hint=int[,int] : [io_uring] [libaio]
> +
> +	Set the I/O priority hint to use for I/Os that must be issued with
> +	a priority when :option:`cmdprio_percentage` or
> +	:option:`cmdprio_bssplit` is set. If not specified when
> +	:option:`cmdprio_percentage` or :option:`cmdprio_bssplit` is set,
> +	this defaults to 0 (no hint). A single value applies to reads and
> +	writes. Comma-separated values may be specified for reads and writes.
> +	See also the :option:`priohint` option.
> +
>  .. option:: cmdprio=int[,int] : [io_uring] [libaio]
>  
>  	Set the I/O priority value to use for I/Os that must be issued with
> @@ -2301,9 +2311,9 @@ with the caveat that when used on the command line, they must come after the
>  
>  		cmdprio_bssplit=blocksize/percentage:blocksize/percentage
>  
> -	In this case, each entry will use the priority class and priority
> -	level defined by the options :option:`cmdprio_class` and
> -	:option:`cmdprio` respectively.
> +	In this case, each entry will use the priority class, priority hint
> +	and priority level defined by the options :option:`cmdprio_class`,
> +        :option:`cmdprio` and :option:`cmdprio_hint` respectively.
>  
>  	The second accepted format for this option is:
>  
> @@ -2314,7 +2324,14 @@ with the caveat that when used on the command line, they must come after the
>  	accepted format does not restrict all entries to have the same priority
>  	class and priority level.
>  
> -	For both formats, only the read and write data directions are supported,
> +	The third accepted format for this option is:
> +
> +		cmdprio_bssplit=blocksize/percentage/class/level/hint:...
> +
> +	This is an extension of the second accepted format that allows to also
> +	specify a priority hint.
> +
> +	For all formats, only the read and write data directions are supported,
>  	values for trim IOs are ignored. This option is mutually exclusive with
>  	the :option:`cmdprio_percentage` option.
>  
> diff --git a/engines/cmdprio.c b/engines/cmdprio.c
> index e6ff1fc2..153e3691 100644
> --- a/engines/cmdprio.c
> +++ b/engines/cmdprio.c
> @@ -267,7 +267,8 @@ static int fio_cmdprio_percentage(struct cmdprio *cmdprio, struct io_u *io_u,
>   * to be set. If the random percentage value is within the user specified
>   * percentage of I/Os that should use a cmdprio priority value (rather than
>   * the default priority), then this function updates the io_u with an ioprio
> - * value as defined by the cmdprio/cmdprio_class or cmdprio_bssplit options.
> + * value as defined by the cmdprio/cmdprio_hint/cmdprio_class or
> + * cmdprio_bssplit options.
>   *
>   * Return true if the io_u ioprio was changed and false otherwise.
>   */
> @@ -342,7 +343,8 @@ static int fio_cmdprio_gen_perc(struct thread_data *td, struct cmdprio *cmdprio)
>  		prio = &cmdprio->perc_entry[ddir];
>  		prio->perc = options->percentage[ddir];
>  		prio->prio = ioprio_value(options->class[ddir],
> -					  options->level[ddir], 0);
> +					  options->level[ddir],
> +					  options->hint[ddir]);
>  		assign_clat_prio_index(prio, &values[ddir]);
>  
>  		ret = init_ts_clat_prio(ts, ddir, &values[ddir]);
> @@ -400,7 +402,8 @@ static int fio_cmdprio_parse_and_gen_bssplit(struct thread_data *td,
>  			goto err;
>  
>  		implicit_cmdprio = ioprio_value(options->class[ddir],
> -						options->level[ddir], 0);
> +						options->level[ddir],
> +						options->hint[ddir]);
>  
>  		ret = fio_cmdprio_generate_bsprio_desc(&cmdprio->bsprio_desc[ddir],
>  						       &parse_res[ddir],
> diff --git a/engines/cmdprio.h b/engines/cmdprio.h
> index 755da8d0..9b351828 100644
> --- a/engines/cmdprio.h
> +++ b/engines/cmdprio.h
> @@ -39,6 +39,7 @@ struct cmdprio_options {
>  	unsigned int percentage[CMDPRIO_RWDIR_CNT];
>  	unsigned int class[CMDPRIO_RWDIR_CNT];
>  	unsigned int level[CMDPRIO_RWDIR_CNT];
> +	unsigned int hint[CMDPRIO_RWDIR_CNT];
>  	char *bssplit_str;
>  };
>  
> diff --git a/engines/io_uring.c b/engines/io_uring.c
> index 5021239e..975a0a4d 100644
> --- a/engines/io_uring.c
> +++ b/engines/io_uring.c
> @@ -157,6 +157,21 @@ static struct fio_option options[] = {
>  		.category = FIO_OPT_C_ENGINE,
>  		.group	= FIO_OPT_G_IOURING,
>  	},
> +	{
> +		.name	= "cmdprio_hint",
> +		.lname	= "Asynchronous I/O priority hint",
> +		.type	= FIO_OPT_INT,
> +		.off1	= offsetof(struct ioring_options,
> +				   cmdprio_options.hint[DDIR_READ]),
> +		.off2	= offsetof(struct ioring_options,
> +				   cmdprio_options.hint[DDIR_WRITE]),
> +		.help	= "Set asynchronous IO priority hint",
> +		.minval	= IOPRIO_MIN_PRIO_HINT,
> +		.maxval	= IOPRIO_MAX_PRIO_HINT,
> +		.interval = 1,
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_IOURING,
> +	},
>  	{
>  		.name	= "cmdprio",
>  		.lname	= "Asynchronous I/O priority level",
> @@ -195,6 +210,12 @@ static struct fio_option options[] = {
>  		.type	= FIO_OPT_UNSUPPORTED,
>  		.help	= "Your platform does not support I/O priority classes",
>  	},
> +	{
> +		.name	= "cmdprio_hint",
> +		.lname	= "Asynchronous I/O priority hint",
> +		.type	= FIO_OPT_UNSUPPORTED,
> +		.help	= "Your platform does not support I/O priority classes",
> +	},
>  	{
>  		.name	= "cmdprio",
>  		.lname	= "Asynchronous I/O priority level",
> @@ -365,8 +386,8 @@ static int fio_ioring_prep(struct thread_data *td, struct io_u *io_u)
>  		/*
>  		 * Since io_uring can have a submission context (sqthread_poll)
>  		 * that is different from the process context, we cannot rely on
> -		 * the IO priority set by ioprio_set() (option prio/prioclass)
> -		 * to be inherited.
> +		 * the IO priority set by ioprio_set() (options prio, prioclass,
> +		 * and priohint) to be inherited.
>  		 * td->ioprio will have the value of the "default prio", so set
>  		 * this unconditionally. This value might get overridden by
>  		 * fio_ioring_cmdprio_prep() if the option cmdprio_percentage or
> diff --git a/engines/libaio.c b/engines/libaio.c
> index 6a0745aa..c52d05fe 100644
> --- a/engines/libaio.c
> +++ b/engines/libaio.c
> @@ -102,6 +102,21 @@ static struct fio_option options[] = {
>  		.category = FIO_OPT_C_ENGINE,
>  		.group	= FIO_OPT_G_LIBAIO,
>  	},
> +	{
> +		.name	= "cmdprio_hint",
> +		.lname	= "Asynchronous I/O priority hint",
> +		.type	= FIO_OPT_INT,
> +		.off1	= offsetof(struct libaio_options,
> +				   cmdprio_options.hint[DDIR_READ]),
> +		.off2	= offsetof(struct libaio_options,
> +				   cmdprio_options.hint[DDIR_WRITE]),
> +		.help	= "Set asynchronous IO priority hint",
> +		.minval	= IOPRIO_MIN_PRIO_HINT,
> +		.maxval	= IOPRIO_MAX_PRIO_HINT,
> +		.interval = 1,
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBAIO,
> +	},
>  	{
>  		.name	= "cmdprio",
>  		.lname	= "Asynchronous I/O priority level",
> @@ -140,6 +155,12 @@ static struct fio_option options[] = {
>  		.type	= FIO_OPT_UNSUPPORTED,
>  		.help	= "Your platform does not support I/O priority classes",
>  	},
> +	{
> +		.name	= "cmdprio_hint",
> +		.lname	= "Asynchronous I/O priority hint",
> +		.type	= FIO_OPT_UNSUPPORTED,
> +		.help	= "Your platform does not support I/O priority classes",
> +	},
>  	{
>  		.name	= "cmdprio",
>  		.lname	= "Asynchronous I/O priority level",
> diff --git a/fio.1 b/fio.1
> index 6dbdb293..72afff65 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2072,6 +2072,14 @@ is set, this defaults to the highest priority class. A single value applies
>  to reads and writes. Comma-separated values may be specified for reads and
>  writes. See man \fBionice\fR\|(1). See also the \fBprioclass\fR option.
>  .TP
> +.BI (io_uring,libaio)cmdprio_hint \fR=\fPint[,int]
> +Set the I/O priority hint to use for I/Os that must be issued with a
> +priority when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR is set.
> +If not specified when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR
> +is set, this defaults to 0 (no hint). A single value applies to reads and
> +writes. Comma-separated values may be specified for reads and writes.
> +See also the \fBpriohint\fR option.
> +.TP
>  .BI (io_uring,libaio)cmdprio \fR=\fPint[,int]
>  Set the I/O priority value to use for I/Os that must be issued with a
>  priority when \fBcmdprio_percentage\fR or \fBcmdprio_bssplit\fR is set.
> @@ -2097,8 +2105,9 @@ The first accepted format for this option is the same as the format of the
>  cmdprio_bssplit=blocksize/percentage:blocksize/percentage
>  .RE
>  .P
> -In this case, each entry will use the priority class and priority level defined
> -by the options \fBcmdprio_class\fR and \fBcmdprio\fR respectively.
> +In this case, each entry will use the priority class, priority hint and
> +priority level defined by the options \fBcmdprio_class\fR, \fBcmdprio\fR
> +and \fBcmdprio_hint\fR respectively.
>  .P
>  The second accepted format for this option is:
>  .RS
> @@ -2111,7 +2120,16 @@ entry. In comparison with the first accepted format, the second accepted format
>  does not restrict all entries to have the same priority class and priority
>  level.
>  .P
> -For both formats, only the read and write data directions are supported, values
> +The third accepted format for this option is:
> +.RS
> +.P
> +cmdprio_bssplit=blocksize/percentage/class/level/hint:...
> +.RE
> +.P
> +This is an extension of the second accepted format that allows to also
> +specify a priority hint.
> +.P
> +For all formats, only the read and write data directions are supported, values
>  for trim IOs are ignored. This option is mutually exclusive with the
>  \fBcmdprio_percentage\fR option.
>  .RE
> diff --git a/options.c b/options.c
> index c1246297..3fcbd2b3 100644
> --- a/options.c
> +++ b/options.c
> @@ -313,15 +313,17 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
>  	int matches = 0;
>  	char *bs_str = NULL;
>  	long long bs_val;
> -	unsigned int perc = 0, class, level;
> +	unsigned int perc = 0, class, level, hint;
>  
>  	/*
>  	 * valid entry formats:
>  	 * bs/ - %s/ - set perc to 0, prio to -1.
>  	 * bs/perc - %s/%u - set prio to -1.
>  	 * bs/perc/class/level - %s/%u/%u/%u
> +	 * bs/perc/class/level/hint - %s/%u/%u/%u/%u
>  	 */
> -	matches = sscanf(str, "%m[^/]/%u/%u/%u", &bs_str, &perc, &class, &level);
> +	matches = sscanf(str, "%m[^/]/%u/%u/%u/%u",
> +			 &bs_str, &perc, &class, &level, &hint);
>  	if (matches < 1) {
>  		log_err("fio: invalid cmdprio_bssplit format\n");
>  		return 1;
> @@ -342,9 +344,14 @@ static int parse_cmdprio_bssplit_entry(struct thread_options *o,
>  	case 2: /* bs/perc case */
>  		break;
>  	case 4: /* bs/perc/class/level case */
> +	case 5: /* bs/perc/class/level/hint case */
>  		class = min(class, (unsigned int) IOPRIO_MAX_PRIO_CLASS);
>  		level = min(level, (unsigned int) IOPRIO_MAX_PRIO);
> -		entry->prio = ioprio_value(class, level, 0);
> +		if (matches == 5)
> +			hint = min(hint, (unsigned int) IOPRIO_MAX_PRIO_HINT);
> +		else
> +			hint = 0;
> +		entry->prio = ioprio_value(class, level, hint);
>  		break;
>  	default:
>  		log_err("fio: invalid cmdprio_bssplit format\n");
> -- 
> 2.41.0
> 

Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 4/4] stats: Add hint information to per priority level stats

[Niklas Cassel]

On Thu, Jul 06, 2023 at 07:13:02AM +0900, Damien Le Moal wrote:
> From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> 
> Introduce the OS dependent ioprio_hint() macro to extract an IO
> priority hint from an ioprio value. This macro defaults to always
> returning 0 for OSes that do not support IO priority hints.
> 
> The json and standard output stats are modified to display the hint
> value together with the priority class and level.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
>  os/os-linux.h |  4 ++++
>  os/os.h       |  3 +++
>  stat.c        | 10 +++++++---
>  3 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/os/os-linux.h b/os/os-linux.h
> index 6f241d09..b9c07891 100644
> --- a/os/os-linux.h
> +++ b/os/os-linux.h
> @@ -150,6 +150,10 @@ static inline int ioprio_value(int ioprio_class, int ioprio, int ioprio_hint)
>  		ioprio;
>  }
>  
> +#define ioprio_class(ioprio)	((ioprio) >> IOPRIO_CLASS_SHIFT)
> +#define ioprio_hint(ioprio)	(((ioprio) >> IOPRIO_HINT_SHIFT) & 0x3ff)
> +#define ioprio(ioprio)		((ioprio) & 7)

1)
ioprio_class() and ioprio() are already defined in os/os-linux.h,
so if you add these again, we will have duplicates in os/os-linux.h.
You probably only want to add ioprio_hint(), after the existing defines.


2)
Perhaps you can also change:
#define ioprio(ioprio)          ((ioprio) & 7)
to
#define ioprio(ioprio)          ((ioprio) & IOPRIO_MAX_PRIO)

and
#define ioprio_hint(ioprio)  (((ioprio) >> IOPRIO_HINT_SHIFT) & 0x3ff)
to
#define ioprio_hint(ioprio)  (((ioprio) >> IOPRIO_HINT_SHIFT) & IOPRIO_MAX_PRIO_HINT)

so we avoid magic numbers.


> +
>  static inline bool ioprio_value_is_class_rt(unsigned int priority)
>  {
>  	return (priority >> IOPRIO_CLASS_SHIFT) == IOPRIO_CLASS_RT;
> diff --git a/os/os.h b/os/os.h
> index 2217d5f8..0f182324 100644
> --- a/os/os.h
> +++ b/os/os.h
> @@ -120,6 +120,9 @@ extern int fio_cpus_split(os_cpu_mask_t *mask, unsigned int cpu);
>  #define ioprio_value_is_class_rt(prio)	(false)
>  #define IOPRIO_MIN_PRIO_CLASS		0
>  #define IOPRIO_MAX_PRIO_CLASS		0
> +#define ioprio_hint(prio)		0

> +#define IOPRIO_MIN_PRIO_HINT		0
> +#define IOPRIO_MAX_PRIO_HINT		0

3)
The MIN_ and MAX_ define should probably go to patch 1/4,
which added the same for os/os-linux.h.


>  #endif
>  #ifndef FIO_HAVE_IOPRIO
>  #define ioprio_value(prioclass, prio, priohint)	(0)
> diff --git a/stat.c b/stat.c
> index 015b8e28..4f943602 100644
> --- a/stat.c
> +++ b/stat.c
> @@ -597,10 +597,11 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
>  				continue;
>  
>  			snprintf(buf, sizeof(buf),
> -				 "%s prio %u/%u",
> +				 "%s prio %u/%u/%u",
>  				 clat_type,
>  				 ioprio_class(ts->clat_prio[ddir][i].ioprio),
> -				 ioprio(ts->clat_prio[ddir][i].ioprio));
> +				 ioprio(ts->clat_prio[ddir][i].ioprio),
> +				 ioprio_hint(ts->clat_prio[ddir][i].ioprio));
>  			display_lat(buf, min, max, mean, dev, out);
>  		}
>  	}
> @@ -640,10 +641,11 @@ static void show_ddir_status(struct group_run_stats *rs, struct thread_stat *ts,
>  					continue;
>  
>  				snprintf(prio_name, sizeof(prio_name),
> -					 "%s prio %u/%u (%.2f%% of IOs)",
> +					 "%s prio %u/%u/%u (%.2f%% of IOs)",
>  					 clat_type,
>  					 ioprio_class(ts->clat_prio[ddir][i].ioprio),
>  					 ioprio(ts->clat_prio[ddir][i].ioprio),
> +					 ioprio_hint(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,
> @@ -1522,6 +1524,8 @@ static void add_ddir_status_json(struct thread_stat *ts,
>  				ioprio_class(ts->clat_prio[ddir][i].ioprio));
>  			json_object_add_value_int(obj, "prio",
>  				ioprio(ts->clat_prio[ddir][i].ioprio));
> +			json_object_add_value_int(obj, "priohint",
> +				ioprio_hint(ts->clat_prio[ddir][i].ioprio));
>  
>  			tmp_object = add_ddir_lat_json(ts,
>  					ts->clat_percentiles | ts->lat_percentiles,
> -- 
> 2.41.0
> 

With the three nits fixed:
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>

Re: [PATCH 2/4] options: add priohint option

[Niklas Cassel]

On Tue, Jul 18, 2023 at 05:44:51PM +0200, Niklas Cassel wrote:
> 2)
> Nit: 'cmdprio_hint' is introduced in a later patch series, so it feels
> a bit premature to mention it in this patch series.

oops... I meant:

".. is introduced in a later patch in the series, so it feels a bit
premature to mention it in this patch."


Kind regards,
Niklas