Re: [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd

[Vincent Fu <vincentfu@gmail.com>]

On 11/21/22 13:29, Alberto Faria wrote:
> When set, the queue's completion fd is enabled even when it isn't used,
> i.e., even if option libblkio_wait_mode is _not_ set to "eventfd".
> 
> Depending on the libblkio driver, this can have an impact on
> performance. This option allows evaluating that overhead.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst          |  9 ++++++++-
>   engines/libblkio.c | 21 +++++++++++++++++++--
>   fio.1              | 10 +++++++++-
>   3 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 4b059cb2..7b3a3ffc 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2333,7 +2333,8 @@ with the caveat that when used on the command line, they must come after the
>      [libblkio]
>   
>   	Use poll queues. This is incompatible with
> -	:option:`libblkio_wait_mode=eventfd <libblkio_wait_mode>`.
> +	:option:`libblkio_wait_mode=eventfd <libblkio_wait_mode>` and
> +	:option:`libblkio_force_enable_completion_eventfd`.
>   
>      [pvsync2]
>   
> @@ -2896,6 +2897,12 @@ with the caveat that when used on the command line, they must come after the
>   	**loop**
>   		Use a busy loop with a non-blocking call to ``blkioq_do_io()``.
>   
> +.. option:: libblkio_force_enable_completion_eventfd=bool : [libblkio]

Same comment as for previous patches regarding option types.

> +
> +	Enable the ``struct blkioq``'s completion eventfd even when unused. This
> +	may impact performance. The default is to enable it only if
> +	:option:`libblkio_wait_mode` is **eventfd**.
> +
>   I/O depth
>   ~~~~~~~~~
>   
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index d7666f69..3ff87e6d 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -37,6 +37,7 @@ struct fio_blkio_options {
>   	unsigned int vectored;
>   	unsigned int write_zeroes_on_trim;
>   	enum fio_blkio_wait_mode wait_mode;
> +	unsigned int force_enable_completion_eventfd;
>   };
>   
>   static struct fio_option options[] = {
> @@ -120,6 +121,16 @@ static struct fio_option options[] = {
>   		.category = FIO_OPT_C_ENGINE,
>   		.group	= FIO_OPT_G_LIBBLKIO,
>   	},
> +	{
> +		.name	= "libblkio_force_enable_completion_eventfd",
> +		.lname	= "Force enable the completion eventfd, even if unused",
> +		.type	= FIO_OPT_STR_SET,
> +		.off1	= offsetof(struct fio_blkio_options,
> +				   force_enable_completion_eventfd),
> +		.help	= "This can impact performance",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
>   
>   	{
>   		.name = NULL,
> @@ -283,6 +294,11 @@ static int fio_blkio_setup(struct thread_data *td)
>   		return 1;
>   	}
>   
> +	if (options->hipri && options->force_enable_completion_eventfd) {
> +		log_err("fio: option hipri is incompatible with option libblkio_force_enable_completion_eventfd\n");
> +		return 1;
> +	}
> +
>   	/* get target size */
>   
>   	if (fio_blkio_create_and_connect(td, &b) != 0)
> @@ -308,7 +324,7 @@ out_blkio_destroy:
>   struct fio_blkio_data {
>   	struct blkio *b;
>   	struct blkioq *q;
> -	int completion_fd; /* -1 if not FIO_BLKIO_WAIT_MODE_EVENTFD */
> +	int completion_fd; /* may be -1 if not FIO_BLKIO_WAIT_MODE_EVENTFD */
>   
>   	bool has_mem_region; /* whether mem_region is valid */
>   	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
> @@ -368,7 +384,8 @@ static int fio_blkio_init(struct thread_data *td)
>   
>   	/* enable completion fd and make it blocking */
>   
> -	if (options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
> +	if (options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD ||
> +		options->force_enable_completion_eventfd) {
>   		blkioq_set_completion_fd_enabled(data->q, true);
>   
>   		data->completion_fd = blkioq_get_completion_fd(data->q);
> diff --git a/fio.1 b/fio.1
> index 88bb232f..e2f92bd1 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2622,7 +2622,8 @@ before starting the \fBstruct blkio\fR. Each property must have the format
>   the engine sets any other properties, so those can be overriden.
>   .TP
>   .BI (libblkio)hipri \fR=\fPbool
> -Use poll queues. This is incompatible with \fBlibblkio_wait_mode=eventfd\fR.
> +Use poll queues. This is incompatible with \fBlibblkio_wait_mode=eventfd\fR and
> +\fBlibblkio_force_enable_completion_eventfd\fR.
>   .TP
>   .BI (libblkio)libblkio_vectored \fR=\fPbool
>   Submit vectored read and write requests. Default is 0.
> @@ -2644,6 +2645,13 @@ Use a blocking call to \fBread()\fR on the completion eventfd.
>   .TP
>   .B loop
>   Use a busy loop with a non-blocking call to \fBblkioq_do_io()\fR.
> +.RE
> +.RE
> +.TP
> +.BI (libblkio)libblkio_force_enable_completion_eventfd \fR=\fPbool
> +Enable the \fBstruct blkioq\fR's completion eventfd even when unused. This may
> +impact performance. The default is to enable it only if \fBlibblkio_wait_mode\fR
> +is \fBeventfd\fR.
>   .SS "I/O depth"
>   .TP
>   .BI iodepth \fR=\fPint