[PATCH 0/2] fix FIO_OPT_STR_SET and update io_uring doc

[PATCH 0/2] fix FIO_OPT_STR_SET and update io_uring doc

[Ankit Kumar]

This series has 2 small patches.

First patch deals with type FIO_OPT_STR_SET.

FIO_OPT_STR_SET suggests the passed value should be
empty or boolean value ([0|1]).
But for the given type it accepts any integer value as there is
no limit for min and max values. Add a limit for that during
the option initialization.

Note: With this the existing job files, test scripts which
used to pass any integer value will not work.

Other way to fix this would be to update the typehelp for
FIO_OPT_STR_SET as "empty or integer value (opt=100)"

Second patch sets the type for sqthread_poll to FIO_OPT_STR_SET
instead of FIO_OPT_INT. This I believe is what was originally
intended when the option was added.
This patch also adds the missing types for io_uring options
in the documentation.

This now fixes the issue:
https://github.com/axboe/fio/issues/927

Ankit Kumar (2):
  parse: set min and max limit for FIO_OPT_STR_SET
  io_uring: update documentation and small fix for sqthread_poll

 HOWTO.rst          | 6 +++---
 engines/io_uring.c | 2 +-
 fio.1              | 6 +++---
 parse.c            | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

-- 
2.17.1

[PATCH 1/2] parse: set min and max limit for FIO_OPT_STR_SET

[Ankit Kumar]

type FIO_OPT_STR_SET suggests the passed value should be
empty or boolean value ([0|1]).
But the given type accepts any integer value as there is
no limit for min and max values. Add a limit to that.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index 656a5025..6bad0d94 100644
--- a/parse.c
+++ b/parse.c
@@ -1434,7 +1434,7 @@ static void option_init(struct fio_option *o)
 		return;
 	if (o->name && !o->lname)
 		log_err("Option %s: missing long option name\n", o->name);
-	if (o->type == FIO_OPT_BOOL) {
+	if (o->type == FIO_OPT_BOOL || o->type == FIO_OPT_STR_SET) {
 		o->minval = 0;
 		o->maxval = 1;
 	}
-- 
2.17.1

[PATCH 2/2] io_uring: update documentation and small fix for sqthread_poll

[Ankit Kumar]

type for sqthread_poll should be FIO_OPT_STR_SET, as this only
requires it to be set. This also fixes the issue:
https://github.com/axboe/fio/issues/927

Update the HOWTO and man page to specify the missing types for all
the remaining io_uring options.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst          | 6 +++---
 engines/io_uring.c | 2 +-
 fio.1              | 6 +++---
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index e89d05f0..4b6ae4cc 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2274,7 +2274,7 @@ with the caveat that when used on the command line, they must come after the
 	map and release for each IO. This is more efficient, and reduces the
 	IO latency as well.
 
-.. option:: nonvectored : [io_uring] [io_uring_cmd]
+.. option:: nonvectored=int : [io_uring] [io_uring_cmd]
 
 	With this option, fio will use non-vectored read/write commands, where
 	address must contain the address directly. Default is -1.
@@ -2301,7 +2301,7 @@ with the caveat that when used on the command line, they must come after the
 	This frees up cycles for fio, at the cost of using more CPU in the
 	system.
 
-.. option:: sqthread_poll_cpu : [io_uring] [io_uring_cmd]
+.. option:: sqthread_poll_cpu=int : [io_uring] [io_uring_cmd]
 
 	When :option:`sqthread_poll` is set, this option provides a way to
 	define which CPU should be used for the polling thread.
@@ -2351,7 +2351,7 @@ with the caveat that when used on the command line, they must come after the
 	When hipri is set this determines the probability of a pvsync2 I/O being high
 	priority. The default is 100%.
 
-.. option:: nowait : [pvsync2] [libaio] [io_uring]
+.. option:: nowait=bool : [pvsync2] [libaio] [io_uring] [io_uring_cmd]
 
 	By default if a request cannot be executed immediately (e.g. resource starvation,
 	waiting on locks) it is queued and the initiating process will be blocked until
diff --git a/engines/io_uring.c b/engines/io_uring.c
index 6906e0a4..3c656b77 100644
--- a/engines/io_uring.c
+++ b/engines/io_uring.c
@@ -226,7 +226,7 @@ static struct fio_option options[] = {
 	{
 		.name	= "sqthread_poll",
 		.lname	= "Kernel SQ thread polling",
-		.type	= FIO_OPT_INT,
+		.type	= FIO_OPT_STR_SET,
 		.off1	= offsetof(struct ioring_options, sqpoll_thread),
 		.help	= "Offload submission/completion to kernel thread",
 		.category = FIO_OPT_C_ENGINE,
diff --git a/fio.1 b/fio.1
index 4324a975..9e33c9e1 100644
--- a/fio.1
+++ b/fio.1
@@ -2063,7 +2063,7 @@ release them when IO is done. If this option is set, the pages are pre-mapped
 before IO is started. This eliminates the need to map and release for each IO.
 This is more efficient, and reduces the IO latency as well.
 .TP
-.BI (io_uring,io_uring_cmd)nonvectored
+.BI (io_uring,io_uring_cmd)nonvectored \fR=\fPint
 With this option, fio will use non-vectored read/write commands, where address
 must contain the address directly. Default is -1.
 .TP
@@ -2092,7 +2092,7 @@ available items in the SQ ring. If this option is set, the act of submitting IO
 will be done by a polling thread in the kernel. This frees up cycles for fio, at
 the cost of using more CPU in the system.
 .TP
-.BI (io_uring,io_uring_cmd)sqthread_poll_cpu
+.BI (io_uring,io_uring_cmd)sqthread_poll_cpu \fR=\fPint
 When `sqthread_poll` is set, this option provides a way to define which CPU
 should be used for the polling thread.
 .TP
@@ -2115,7 +2115,7 @@ than normal.
 When hipri is set this determines the probability of a pvsync2 I/O being high
 priority. The default is 100%.
 .TP
-.BI (pvsync2,libaio,io_uring)nowait
+.BI (pvsync2,libaio,io_uring,io_uring_cmd)nowait \fR=\fPbool
 By default if a request cannot be executed immediately (e.g. resource starvation,
 waiting on locks) it is queued and the initiating process will be blocked until
 the required resource becomes free.
-- 
2.17.1

Re: [PATCH 0/2] fix FIO_OPT_STR_SET and update io_uring doc

[Vincent Fu]

On 10/21/22 07:32, Ankit Kumar wrote:
 > This series has 2 small patches.
 >
 > First patch deals with type FIO_OPT_STR_SET.
 >
 > FIO_OPT_STR_SET suggests the passed value should be
 > empty or boolean value ([0|1]).
 > But for the given type it accepts any integer value as there is
 > no limit for min and max values. Add a limit for that during
 > the option initialization.
 >
 > Note: With this the existing job files, test scripts which
 > used to pass any integer value will not work.
 >
 > Other way to fix this would be to update the typehelp for
 > FIO_OPT_STR_SET as "empty or integer value (opt=100)"
 >
 > Second patch sets the type for sqthread_poll to FIO_OPT_STR_SET
 > instead of FIO_OPT_INT. This I believe is what was originally
 > intended when the option was added.
 > This patch also adds the missing types for io_uring options
 > in the documentation.
 >
 > This now fixes the issue:
 > https://github.com/axboe/fio/issues/927
 >
 > Ankit Kumar (2):
 >    parse: set min and max limit for FIO_OPT_STR_SET
 >    io_uring: update documentation and small fix for sqthread_poll
 >
 >   HOWTO.rst          | 6 +++---
 >   engines/io_uring.c | 2 +-
 >   fio.1              | 6 +++---
 >   parse.c            | 2 +-
 >   4 files changed, 8 insertions(+), 8 deletions(-)
 >

Ankit, I merged the second patch that cleans up the documentation. The 
first patch may break existing job files and I'm not convinced that 
doing so is worth it, although I agree that your fix is what we should 
have done from the beginning.

Vincent