fio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/10] Add a libblkio engine
@ 2022-11-21 18:28 Alberto Faria
  2022-11-21 18:28 ` [PATCH 01/10] " Alberto Faria
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

The libblkio library provides a unified API for efficiently accessing
block devices using modern high-performance block I/O interfaces like
io_uring and vhost-user-blk. Using libblkio reduces the amount of code
needed for interfacing with storage devices and allows developers to
focus on their applcations.

Add a libblkio engine that uses libblkio to perform I/O. This is useful
to benchmark the library itself, and also adds support for storage
interfaces and devices otherwise not supported by fio, such as
virtio-blk PCI, vhost-user, and vhost-vDPA devices.

See the libblkio documentation [2] or KVM Forum 2022 [3] presentation
for more information on the library itself.

[1] https://gitlab.com/libblkio/libblkio
[2] https://libblkio.gitlab.io/libblkio/index.html
[3] https://static.sched.com/hosted_files/kvmforum2022/8c/libblkio-kvm-forum-2022.pdf

Alberto Faria (10):
  Add a libblkio engine
  Add engine flag FIO_SKIPPABLE_IOMEM_ALLOC
  engines/libblkio: Allow setting option mem/iomem
  engines/libblkio: Add support for poll queues
  engines/libblkio: Add option libblkio_vectored
  engines/libblkio: Add option libblkio_write_zeroes_on_trim
  engines/libblkio: Add option libblkio_wait_mode
  engines/libblkio: Add option libblkio_force_enable_completion_eventfd
  engines/libblkio: Add options for some driver-specific properties
  engines/libblkio: Share a single blkio instance among threads in same
    process

 HOWTO.rst                                 |  86 ++
 Makefile                                  |   6 +
 configure                                 |  25 +
 engines/libblkio.c                        | 966 ++++++++++++++++++++++
 examples/libblkio-io_uring.fio            |  29 +
 examples/libblkio-virtio-blk-vfio-pci.fio |  28 +
 fio.1                                     |  74 ++
 ioengines.h                               |   2 +
 memory.c                                  |  22 +-
 optgroup.h                                |   2 +
 10 files changed, 1230 insertions(+), 10 deletions(-)
 create mode 100644 engines/libblkio.c
 create mode 100644 examples/libblkio-io_uring.fio
 create mode 100644 examples/libblkio-virtio-blk-vfio-pci.fio

-- 
2.38.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH 01/10] Add a libblkio engine
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-11-22  9:17   ` Damien Le Moal
  2022-11-21 18:28 ` [PATCH 02/10] Add engine flag FIO_SKIPPABLE_IOMEM_ALLOC Alberto Faria
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

The libblkio library provides a unified API for efficiently accessing
block devices using modern high-performance block I/O interfaces like
io_uring and vhost-user-blk. Using libblkio reduces the amount of code
needed for interfacing with storage devices and allows developers to
focus on their applcations.

Add a libblkio engine that uses libblkio to perform I/O. This is useful
to benchmark the library itself, and also adds support for storage
interfaces and devices otherwise not supported by fio, such as
virtio-blk PCI, vhost-user, and vhost-vDPA devices.

See the libblkio documentation [2] or KVM Forum 2022 [3] presentation
for more information on the library itself.

[1] https://gitlab.com/libblkio/libblkio
[2] https://libblkio.gitlab.io/libblkio/index.html
[3] https://static.sched.com/hosted_files/kvmforum2022/8c/libblkio-kvm-forum-2022.pdf

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst                                 |  26 ++
 Makefile                                  |   6 +
 configure                                 |  25 ++
 engines/libblkio.c                        | 463 ++++++++++++++++++++++
 examples/libblkio-io_uring.fio            |  19 +
 examples/libblkio-virtio-blk-vfio-pci.fio |  18 +
 fio.1                                     |  19 +
 optgroup.h                                |   2 +
 8 files changed, 578 insertions(+)
 create mode 100644 engines/libblkio.c
 create mode 100644 examples/libblkio-io_uring.fio
 create mode 100644 examples/libblkio-virtio-blk-vfio-pci.fio

diff --git a/HOWTO.rst b/HOWTO.rst
index e796f961..d5a2749c 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2192,6 +2192,12 @@ I/O engine
 			the SPDK NVMe driver, or your own custom NVMe driver. The xnvme engine includes
 			engine specific options. (See https://xnvme.io).
 
+		**libblkio**
+			Use the libblkio library
+			(https://gitlab.com/libblkio/libblkio). The specific
+			*driver* to use must be set using
+			:option:`libblkio_driver`.
+
 I/O engine specific parameters
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -2842,6 +2848,26 @@ with the caveat that when used on the command line, they must come after the
 
 	If this option is set. xnvme will use vectored read/write commands.
 
+.. option:: libblkio_driver=str : [libblkio]
+
+	The driver to be used by libblkio (e.g. **virtio-blk-vfio-pci**).
+
+.. option:: libblkio_pre_connect_props=str : [libblkio]
+
+	A colon-separated list of libblkio properties to be set after creating
+	but before connecting the ``struct blkio``. Each property must have the
+	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
+	set after the engine sets any other properties, so those can be
+	overriden.
+
+.. option:: libblkio_pre_start_props=str : [libblkio]
+
+	A colon-separated list of libblkio properties to be set after connecting
+	but before starting the ``struct blkio``. Each property must have the
+	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
+	set after the engine sets any other properties, so those can be
+	overriden.
+
 I/O depth
 ~~~~~~~~~
 
diff --git a/Makefile b/Makefile
index 7bd572d7..9fd8f59b 100644
--- a/Makefile
+++ b/Makefile
@@ -237,6 +237,12 @@ ifdef CONFIG_LIBXNVME
   xnvme_CFLAGS = $(LIBXNVME_CFLAGS)
   ENGINES += xnvme
 endif
+ifdef CONFIG_LIBBLKIO
+  libblkio_SRCS = engines/libblkio.c
+  libblkio_LIBS = $(LIBBLKIO_LIBS)
+  libblkio_CFLAGS = $(LIBBLKIO_CFLAGS)
+  ENGINES += libblkio
+endif
 ifeq ($(CONFIG_TARGET_OS), Linux)
   SOURCE += diskutil.c fifo.c blktrace.c cgroup.c trim.c engines/sg.c \
 		oslib/linux-dev-lookup.c engines/io_uring.c engines/nvme.c
diff --git a/configure b/configure
index 1b12d268..6d8e3a87 100755
--- a/configure
+++ b/configure
@@ -176,6 +176,7 @@ libiscsi="no"
 libnbd="no"
 libnfs=""
 xnvme=""
+libblkio=""
 libzbc=""
 dfs=""
 seed_buckets=""
@@ -248,6 +249,8 @@ for opt do
   ;;
   --disable-xnvme) xnvme="no"
   ;;
+  --disable-libblkio) libblkio="no"
+  ;;
   --disable-tcmalloc) disable_tcmalloc="yes"
   ;;
   --disable-libnfs) libnfs="no"
@@ -304,6 +307,7 @@ if test "$show_help" = "yes" ; then
   echo "--enable-libiscsi       Enable iscsi support"
   echo "--enable-libnbd         Enable libnbd (NBD engine) support"
   echo "--disable-xnvme         Disable xnvme support even if found"
+  echo "--disable-libblkio      Disable libblkio support even if found"
   echo "--disable-libzbc        Disable libzbc even if found"
   echo "--disable-tcmalloc      Disable tcmalloc support"
   echo "--dynamic-libengines    Lib-based ioengines as dynamic libraries"
@@ -2663,6 +2667,22 @@ if test "$xnvme" != "no" ; then
 fi
 print_config "xnvme engine" "$xnvme"
 
+##########################################
+# Check if we have libblkio
+if test "$libblkio" != "no" ; then
+  if check_min_lib_version blkio 1.0.0; then
+    libblkio="yes"
+    libblkio_cflags=$(pkg-config --cflags blkio)
+    libblkio_libs=$(pkg-config --libs blkio)
+  else
+    if test "$libblkio" = "yes" ; then
+      feature_not_found "libblkio" "libblkio-dev or libblkio-devel"
+    fi
+    libblkio="no"
+  fi
+fi
+print_config "libblkio engine" "$libblkio"
+
 ##########################################
 # check march=armv8-a+crc+crypto
 if test "$march_armv8_a_crc_crypto" != "yes" ; then
@@ -3276,6 +3296,11 @@ if test "$xnvme" = "yes" ; then
   echo "LIBXNVME_CFLAGS=$xnvme_cflags" >> $config_host_mak
   echo "LIBXNVME_LIBS=$xnvme_libs" >> $config_host_mak
 fi
+if test "$libblkio" = "yes" ; then
+  output_sym "CONFIG_LIBBLKIO"
+  echo "LIBBLKIO_CFLAGS=$libblkio_cflags" >> $config_host_mak
+  echo "LIBBLKIO_LIBS=$libblkio_libs" >> $config_host_mak
+fi
 if test "$dynamic_engines" = "yes" ; then
   output_sym "CONFIG_DYNAMIC_ENGINES"
 fi
diff --git a/engines/libblkio.c b/engines/libblkio.c
new file mode 100644
index 00000000..bd9a5c84
--- /dev/null
+++ b/engines/libblkio.c
@@ -0,0 +1,463 @@
+/*
+ * libblkio engine
+ *
+ * IO engine using libblkio to access various block I/O interfaces:
+ * https://gitlab.com/libblkio/libblkio
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <blkio.h>
+
+#include "../fio.h"
+#include "../optgroup.h"
+#include "../options.h"
+#include "../parse.h"
+
+struct fio_blkio_options {
+	void *pad; /* option fields must not have offset 0 */
+
+	char *driver;
+	char *pre_connect_props;
+	char *pre_start_props;
+};
+
+static struct fio_option options[] = {
+	{
+		.name	= "libblkio_driver",
+		.lname	= "libblkio driver name",
+		.type	= FIO_OPT_STR_STORE,
+		.off1	= offsetof(struct fio_blkio_options, driver),
+		.help	= "Name of the driver to be used by libblkio",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
+	{
+		.name	= "libblkio_pre_connect_props",
+		.lname	= "Properties to be set before blkio_connect()",
+		.type	= FIO_OPT_STR_STORE,
+		.off1	= offsetof(struct fio_blkio_options, pre_connect_props),
+		.help	= "",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
+	{
+		.name	= "libblkio_pre_start_props",
+		.lname	= "Properties to be set before blkio_start()",
+		.type	= FIO_OPT_STR_STORE,
+		.off1	= offsetof(struct fio_blkio_options, pre_start_props),
+		.help	= "",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
+
+	{
+		.name = NULL,
+	},
+};
+
+static int fio_blkio_set_props_from_str(struct blkio *b, const char *opt_name,
+					const char *str) {
+	int ret = 0;
+	char *new_str, *name, *value;
+
+	if (!str)
+		return 0;
+
+	/* copy string */
+
+	new_str = strdup(str);
+
+	if (!new_str) {
+		log_err("fio: strdup() failed\n");
+		return 1;
+	}
+
+	/* iterate over property name-value pairs */
+
+	while ((name = get_next_str(&new_str))) {
+		/* split into property name and value */
+
+		value = strchr(name, '=');
+
+		if (!value) {
+			log_err("fio: missing '=' in option %s\n", opt_name);
+			ret = 1;
+			break;
+		}
+
+		*value = '\0';
+		++value;
+
+		/* strip whitespace from property name and value */
+
+		strip_blank_front(&name);
+		strip_blank_end(name);
+
+		if (name[0] == '\0') {
+			log_err("fio: empty property name in option %s\n",
+				opt_name);
+			ret = 1;
+			break;
+		}
+
+		strip_blank_front(&value);
+		strip_blank_end(value);
+
+		/* set property */
+
+		if (blkio_set_str(b, name, value) != 0) {
+			log_err("fio: error setting property '%s' to '%s': %s\n",
+				name, value, blkio_get_error_msg());
+			ret = 1;
+			break;
+		}
+	}
+
+	free(new_str);
+	return ret;
+}
+
+/*
+ * Log the failure of a libblkio function.
+ *
+ * `(void)func` is to ensure `func` exists and prevent typos
+ */
+#define fio_blkio_log_err(func) \
+	({ \
+		(void)func; \
+		log_err("fio: %s() failed: %s\n", #func, \
+			blkio_get_error_msg()); \
+	})
+
+static int fio_blkio_create_and_connect(struct thread_data *td,
+					struct blkio **out_blkio)
+{
+	const struct fio_blkio_options *options = td->eo;
+	struct blkio *b;
+	int ret;
+
+	/* create blkio */
+
+	if (!options->driver) {
+		log_err("fio: engine libblkio requires option libblkio_driver to be set\n");
+		return 1;
+	}
+
+	if (blkio_create(options->driver, &b) != 0) {
+		fio_blkio_log_err(blkio_create);
+		return 1;
+	}
+
+	/* set pre-connect properties */
+
+	/* don't fail if driver doesn't have a "direct" property */
+	ret = blkio_set_bool(b, "direct", td->o.odirect);
+	if (ret != 0 && ret != -ENOENT) {
+		fio_blkio_log_err(blkio_set_bool);
+		goto err_blkio_destroy;
+	}
+
+	if (blkio_set_bool(b, "read-only", read_only) != 0) {
+		fio_blkio_log_err(blkio_set_bool);
+		goto err_blkio_destroy;
+	}
+
+	if (fio_blkio_set_props_from_str(b, "libblkio_pre_connect_props",
+					 options->pre_connect_props) != 0)
+		goto err_blkio_destroy;
+
+	/* connect blkio */
+
+	if (blkio_connect(b) != 0) {
+		fio_blkio_log_err(blkio_connect);
+		goto err_blkio_destroy;
+	}
+
+	/* set pre-start properties */
+
+	if (fio_blkio_set_props_from_str(b, "libblkio_pre_start_props",
+					 options->pre_start_props) != 0)
+		goto err_blkio_destroy;
+
+	*out_blkio = b;
+	return 0;
+
+err_blkio_destroy:
+	blkio_destroy(&b);
+	return 1;
+}
+
+static int fio_blkio_setup(struct thread_data *td)
+{
+	/*
+	 * We have to determine device/file size here, so we create and connect
+	 * a blkio instance. But this callback is called from the main thread in
+	 * the original fio process, not from the processes in which jobs will
+	 * actually run. We thus subsequently destroy the blkio and create it
+	 * again in the init() callback.
+	 */
+
+	struct blkio *b;
+	int ret = 0;
+	uint64_t capacity;
+
+	assert(td->files_index == 1);
+
+	/* get target size */
+
+	if (fio_blkio_create_and_connect(td, &b) != 0)
+		return 1;
+
+	if (blkio_get_uint64(b, "capacity", &capacity) != 0) {
+		fio_blkio_log_err(blkio_get_uint64);
+		ret = 1;
+		goto out_blkio_destroy;
+	}
+
+	/* set file size for fio */
+
+	td->files[0]->real_file_size = capacity;
+	fio_file_set_size_known(td->files[0]);
+
+out_blkio_destroy:
+	blkio_destroy(&b);
+	return ret;
+}
+
+/* per-thread state */
+struct fio_blkio_data {
+	struct blkio *b;
+	struct blkioq *q;
+
+	bool has_mem_region; /* whether mem_region is valid */
+	struct blkio_mem_region mem_region;
+
+	struct blkio_completion *completions;
+};
+
+static int fio_blkio_init(struct thread_data *td)
+{
+	struct fio_blkio_data *data;
+
+	/* allocate per-thread data struct */
+
+	data = calloc(1, sizeof(*data));
+	if (!data) {
+		log_err("fio: calloc() failed\n");
+		goto err_free;
+	}
+
+	data->completions = calloc(td->o.iodepth, sizeof(data->completions[0]));
+	if (!data->completions) {
+		log_err("fio: calloc() failed\n");
+		goto err_free;
+	}
+
+	/* create, connect, and start blkio */
+
+	if (fio_blkio_create_and_connect(td, &data->b) != 0)
+		goto err_free;
+
+	if (blkio_set_int(data->b, "num-queues", 1) != 0) {
+		fio_blkio_log_err(blkio_set_int);
+		goto err_blkio_destroy;
+	}
+
+	if (blkio_start(data->b) != 0) {
+		fio_blkio_log_err(blkio_start);
+		goto err_blkio_destroy;
+	}
+
+	/* get queue */
+
+	data->q = blkio_get_queue(data->b, 0);
+
+	/* Set data only here so cleanup() does nothing if init() fails. */
+	td->io_ops_data = data;
+
+	return 0;
+
+err_blkio_destroy:
+	blkio_destroy(&data->b);
+err_free:
+	free(data->completions);
+	free(data);
+	return 1;
+}
+
+static void fio_blkio_cleanup(struct thread_data *td)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+
+	if (data) {
+		blkio_destroy(&data->b);
+		free(data->completions);
+		free(data);
+	}
+}
+
+#define align_up(x, y) ((((x) + (y) - 1) / (y)) * (y))
+
+static int fio_blkio_iomem_alloc(struct thread_data *td, size_t size)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+	int ret;
+	uint64_t mem_region_alignment;
+	size_t aligned_size;
+
+	/* round up size to satisfy mem-region-alignment */
+
+	if (blkio_get_uint64(data->b, "mem-region-alignment",
+			     &mem_region_alignment) != 0) {
+		fio_blkio_log_err(blkio_get_uint64);
+		return 1;
+	}
+
+	aligned_size = align_up(size, (size_t)mem_region_alignment);
+
+	/* allocate memory region */
+
+	if (blkio_alloc_mem_region(data->b, &data->mem_region,
+				   aligned_size) != 0) {
+		fio_blkio_log_err(blkio_alloc_mem_region);
+		ret = 1;
+		goto out;
+	}
+
+	if (blkio_map_mem_region(data->b, &data->mem_region) != 0) {
+		fio_blkio_log_err(blkio_map_mem_region);
+		ret = 1;
+		goto out_free;
+	}
+
+	td->orig_buffer = data->mem_region.addr;
+	data->has_mem_region = true;
+
+	ret = 0;
+	goto out;
+
+out_free:
+	blkio_free_mem_region(data->b, &data->mem_region);
+out:
+	return ret;
+}
+
+static void fio_blkio_iomem_free(struct thread_data *td)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+
+	if (data && data->has_mem_region) {
+		blkio_unmap_mem_region(data->b, &data->mem_region);
+		blkio_free_mem_region(data->b, &data->mem_region);
+
+		data->has_mem_region = false;
+	}
+}
+
+static int fio_blkio_open_file(struct thread_data *td, struct fio_file *f)
+{
+	return 0;
+}
+
+static enum fio_q_status fio_blkio_queue(struct thread_data *td,
+					 struct io_u *io_u)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+
+	fio_ro_check(td, io_u);
+
+	switch (io_u->ddir) {
+		case DDIR_READ:
+			blkioq_read(data->q, io_u->offset, io_u->xfer_buf,
+				    (size_t)io_u->xfer_buflen, io_u, 0);
+			break;
+
+		case DDIR_WRITE:
+			blkioq_write(data->q, io_u->offset, io_u->xfer_buf,
+				     (size_t)io_u->xfer_buflen, io_u, 0);
+			break;
+
+		case DDIR_TRIM:
+			blkioq_discard(data->q, io_u->offset, io_u->xfer_buflen,
+				       io_u, 0);
+		        break;
+
+		case DDIR_SYNC:
+		case DDIR_DATASYNC:
+			blkioq_flush(data->q, io_u, 0);
+			break;
+
+		default:
+			io_u->error = ENOTSUP;
+			io_u_log_error(td, io_u);
+			return FIO_Q_COMPLETED;
+	}
+
+	return FIO_Q_QUEUED;
+}
+
+static int fio_blkio_getevents(struct thread_data *td, unsigned int min,
+			       unsigned int max, const struct timespec *t)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+	int n;
+
+	n = blkioq_do_io(data->q, data->completions, (int)min, (int)max, NULL);
+	if (n < 0) {
+		fio_blkio_log_err(blkioq_do_io);
+		return -1;
+	}
+
+	return n;
+}
+
+static struct io_u *fio_blkio_event(struct thread_data *td, int event)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+	struct blkio_completion *completion = &data->completions[event];
+	struct io_u *io_u = completion->user_data;
+
+	io_u->error = -completion->ret;
+
+	return io_u;
+}
+
+FIO_STATIC struct ioengine_ops ioengine = {
+	.name			= "libblkio",
+	.version		= FIO_IOOPS_VERSION,
+	.flags			= FIO_DISKLESSIO | FIO_NOEXTEND |
+				  FIO_NO_OFFLOAD,
+
+	.setup			= fio_blkio_setup,
+	.init			= fio_blkio_init,
+	.cleanup		= fio_blkio_cleanup,
+
+	.iomem_alloc		= fio_blkio_iomem_alloc,
+	.iomem_free		= fio_blkio_iomem_free,
+
+	.open_file		= fio_blkio_open_file,
+
+	.queue			= fio_blkio_queue,
+	.getevents		= fio_blkio_getevents,
+	.event			= fio_blkio_event,
+
+	.options		= options,
+	.option_struct_size	= sizeof(struct fio_blkio_options),
+};
+
+static void fio_init fio_blkio_register(void)
+{
+	register_ioengine(&ioengine);
+}
+
+static void fio_exit fio_blkio_unregister(void)
+{
+	unregister_ioengine(&ioengine);
+}
diff --git a/examples/libblkio-io_uring.fio b/examples/libblkio-io_uring.fio
new file mode 100644
index 00000000..d2700e51
--- /dev/null
+++ b/examples/libblkio-io_uring.fio
@@ -0,0 +1,19 @@
+; Benchmark accessing a regular file or block device using libblkio.
+;
+; Replace "libblkio_path" below with the path to your file or device, or
+; override it by passing the '--libblkio_pre_connect_props=path=...' flag to
+; fio.
+;
+; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
+
+[global]
+ioengine=libblkio
+libblkio_driver=io_uring
+libblkio_pre_connect_props=path=/dev/nvme0n1  ; REPLACE THIS WITH THE RIGHT PATH
+rw=randread
+blocksize=4k
+direct=1
+time_based=1
+runtime=10s
+
+[job]
diff --git a/examples/libblkio-virtio-blk-vfio-pci.fio b/examples/libblkio-virtio-blk-vfio-pci.fio
new file mode 100644
index 00000000..79d6228d
--- /dev/null
+++ b/examples/libblkio-virtio-blk-vfio-pci.fio
@@ -0,0 +1,18 @@
+; Benchmark accessing a PCI virtio-blk device using libblkio.
+;
+; Replace "libblkio_path" below with the path to your device's sysfs directory,
+; or override it by passing the '--libblkio_pre_connect_props=path=...' flag to
+; fio. Note that colons in the path must be escaped with a backslash.
+;
+; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
+
+[global]
+ioengine=libblkio
+libblkio_driver=virtio-blk-vfio-pci
+libblkio_pre_connect_props=path=/sys/bus/pci/devices/0000\:00\:01.0  ; REPLACE THIS WITH THE RIGHT PATH
+rw=randread
+blocksize=4k
+time_based=1
+runtime=10s
+
+[job]
diff --git a/fio.1 b/fio.1
index 9e33c9e1..64a774f5 100644
--- a/fio.1
+++ b/fio.1
@@ -1989,6 +1989,10 @@ I/O engine using the xNVMe C API, for NVMe devices. The xnvme engine provides
 flexibility to access GNU/Linux Kernel NVMe driver via libaio, IOCTLs, io_uring,
 the SPDK NVMe driver, or your own custom NVMe driver. The xnvme engine includes
 engine specific options. (See \fIhttps://xnvme.io/\fR).
+.TP
+.B libblkio
+Use the libblkio library (\fIhttps://gitlab.com/libblkio/libblkio\fR). The
+specific \fBdriver\fR to use must be set using \fBlibblkio_driver\fR.
 .SS "I/O engine specific parameters"
 In addition, there are some parameters which are only valid when a specific
 \fBioengine\fR is in use. These are used identically to normal parameters,
@@ -2599,6 +2603,21 @@ xnvme namespace identifier for userspace NVMe driver such as SPDK.
 .TP
 .BI (xnvme)xnvme_iovec
 If this option is set, xnvme will use vectored read/write commands.
+.TP
+.BI (libblkio)libblkio_driver \fR=\fPstr
+The driver to be used by libblkio (e.g. \fBvirtio-blk-vfio-pci\fR).
+.TP
+.BI (libblkio)libblkio_pre_connect_props \fR=\fPstr
+A colon-separated list of libblkio properties to be set after creating but
+before connecting the \fBstruct blkio\fR. Each property must have the format
+\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
+the engine sets any other properties, so those can be overriden.
+.TP
+.BI (libblkio)libblkio_pre_start_props \fR=\fPstr
+A colon-separated list of libblkio properties to be set after connecting but
+before starting the \fBstruct blkio\fR. Each property must have the format
+\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
+the engine sets any other properties, so those can be overriden.
 .SS "I/O depth"
 .TP
 .BI iodepth \fR=\fPint
diff --git a/optgroup.h b/optgroup.h
index dc73c8f3..024b902f 100644
--- a/optgroup.h
+++ b/optgroup.h
@@ -73,6 +73,7 @@ enum opt_category_group {
 	__FIO_OPT_G_NFS,
 	__FIO_OPT_G_WINDOWSAIO,
 	__FIO_OPT_G_XNVME,
+	__FIO_OPT_G_LIBBLKIO,
 
 	FIO_OPT_G_RATE		= (1ULL << __FIO_OPT_G_RATE),
 	FIO_OPT_G_ZONE		= (1ULL << __FIO_OPT_G_ZONE),
@@ -120,6 +121,7 @@ enum opt_category_group {
 	FIO_OPT_G_DFS		= (1ULL << __FIO_OPT_G_DFS),
 	FIO_OPT_G_WINDOWSAIO	= (1ULL << __FIO_OPT_G_WINDOWSAIO),
 	FIO_OPT_G_XNVME         = (1ULL << __FIO_OPT_G_XNVME),
+	FIO_OPT_G_LIBBLKIO	= (1ULL << __FIO_OPT_G_LIBBLKIO),
 };
 
 extern const struct opt_group *opt_group_from_mask(uint64_t *mask);
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 02/10] Add engine flag FIO_SKIPPABLE_IOMEM_ALLOC
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
  2022-11-21 18:28 ` [PATCH 01/10] " Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-11-21 18:28 ` [PATCH 03/10] engines/libblkio: Allow setting option mem/iomem Alberto Faria
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

It makes it valid to set option mem/iomem even when the engine specifies
iomem_alloc and iomem_free callbacks, allowing users to optionally use
fio's customizable memory allocation logic instead of the engine's.

This is in preparation for giving libblkio engine users the choice
between controlling memory allocation or delegating it to the libblkio
library.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 ioengines.h |  2 ++
 memory.c    | 22 ++++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/ioengines.h b/ioengines.h
index 11d2115c..d43540d0 100644
--- a/ioengines.h
+++ b/ioengines.h
@@ -87,6 +87,8 @@ enum fio_ioengine_flags {
 	FIO_NO_OFFLOAD	= 1 << 15,	/* no async offload */
 	FIO_ASYNCIO_SETS_ISSUE_TIME
 			= 1 << 16,	/* async ioengine with commit function that sets issue_time */
+	FIO_SKIPPABLE_IOMEM_ALLOC
+			= 1 << 17,	/* skip iomem_alloc & iomem_free if job sets mem/iomem */
 };
 
 /*
diff --git a/memory.c b/memory.c
index 6cf73333..577d3dd5 100644
--- a/memory.c
+++ b/memory.c
@@ -305,16 +305,18 @@ int allocate_io_mem(struct thread_data *td)
 	dprint(FD_MEM, "Alloc %llu for buffers\n", (unsigned long long) total_mem);
 
 	/*
-	 * If the IO engine has hooks to allocate/free memory, use those. But
-	 * error out if the user explicitly asked for something else.
+	 * If the IO engine has hooks to allocate/free memory and the user
+	 * doesn't explicitly ask for something else, use those. But fail if the
+	 * user asks for something else with an engine that doesn't allow that.
 	 */
-	if (td->io_ops->iomem_alloc) {
-		if (fio_option_is_set(&td->o, mem_type)) {
-			log_err("fio: option 'mem/iomem' conflicts with specified IO engine\n");
-			ret = 1;
-		} else
-			ret = td->io_ops->iomem_alloc(td, total_mem);
-	} else if (td->o.mem_type == MEM_MALLOC)
+	if (td->io_ops->iomem_alloc && fio_option_is_set(&td->o, mem_type) &&
+	    !td_ioengine_flagged(td, FIO_SKIPPABLE_IOMEM_ALLOC)) {
+		log_err("fio: option 'mem/iomem' conflicts with specified IO engine\n");
+		ret = 1;
+	} else if (td->io_ops->iomem_alloc &&
+		   !fio_option_is_set(&td->o, mem_type))
+		ret = td->io_ops->iomem_alloc(td, total_mem);
+	else if (td->o.mem_type == MEM_MALLOC)
 		ret = alloc_mem_malloc(td, total_mem);
 	else if (td->o.mem_type == MEM_SHM || td->o.mem_type == MEM_SHMHUGE)
 		ret = alloc_mem_shm(td, total_mem);
@@ -342,7 +344,7 @@ void free_io_mem(struct thread_data *td)
 	if (td->o.odirect || td->o.oatomic)
 		total_mem += page_mask;
 
-	if (td->io_ops->iomem_alloc) {
+	if (td->io_ops->iomem_alloc && !fio_option_is_set(&td->o, mem_type)) {
 		if (td->io_ops->iomem_free)
 			td->io_ops->iomem_free(td);
 	} else if (td->o.mem_type == MEM_MALLOC)
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 03/10] engines/libblkio: Allow setting option mem/iomem
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
  2022-11-21 18:28 ` [PATCH 01/10] " Alberto Faria
  2022-11-21 18:28 ` [PATCH 02/10] Add engine flag FIO_SKIPPABLE_IOMEM_ALLOC Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-11-21 18:28 ` [PATCH 04/10] engines/libblkio: Add support for poll queues Alberto Faria
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

This allows users to customize data buffer memory using fio's existing
options. Users become responsible for ensuring that the allocated memory
satisfies all constraints imposed by the libblkio driver under use.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst          |  5 ++++-
 engines/libblkio.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 fio.1              |  4 +++-
 3 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index d5a2749c..fb27d3d3 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2196,7 +2196,10 @@ I/O engine
 			Use the libblkio library
 			(https://gitlab.com/libblkio/libblkio). The specific
 			*driver* to use must be set using
-			:option:`libblkio_driver`.
+			:option:`libblkio_driver`. If
+			:option:`mem`/:option:`iomem` is not specified, memory
+			allocation is delegated to libblkio (and so is
+			guaranteed to work with the selected *driver*).
 
 I/O engine specific parameters
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/engines/libblkio.c b/engines/libblkio.c
index bd9a5c84..54fd9ff6 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -237,7 +237,7 @@ struct fio_blkio_data {
 	struct blkioq *q;
 
 	bool has_mem_region; /* whether mem_region is valid */
-	struct blkio_mem_region mem_region;
+	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
 
 	struct blkio_completion *completions;
 };
@@ -292,6 +292,47 @@ err_free:
 	return 1;
 }
 
+static int fio_blkio_post_init(struct thread_data *td)
+{
+	struct fio_blkio_data *data = td->io_ops_data;
+
+	if (!data->has_mem_region) {
+		/*
+		 * Memory was allocated by the fio core and not iomem_alloc(),
+		 * so we need to register it as a memory region here.
+		 *
+		 * `td->orig_buffer_size` is computed like `len` below, but then
+		 * fio can add some padding to it to make sure it is
+		 * sufficiently aligned to the page size and the mem_align
+		 * option. However, this can make it become unaligned to the
+		 * "mem-region-alignment" property in ways that the user can't
+		 * control, so we essentially recompute `td->orig_buffer_size`
+		 * here but without adding that padding.
+		 */
+
+		unsigned long long max_block_size;
+		struct blkio_mem_region region;
+
+		max_block_size = max(td->o.max_bs[DDIR_READ],
+				     max(td->o.max_bs[DDIR_WRITE],
+					 td->o.max_bs[DDIR_TRIM]));
+
+		region = (struct blkio_mem_region) {
+			.addr	= td->orig_buffer,
+			.len	= (size_t)max_block_size *
+					(size_t)td->o.iodepth,
+			.fd	= -1,
+		};
+
+		if (blkio_map_mem_region(data->b, &region) != 0) {
+			fio_blkio_log_err(blkio_map_mem_region);
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 static void fio_blkio_cleanup(struct thread_data *td)
 {
 	struct fio_blkio_data *data = td->io_ops_data;
@@ -433,10 +474,11 @@ FIO_STATIC struct ioengine_ops ioengine = {
 	.name			= "libblkio",
 	.version		= FIO_IOOPS_VERSION,
 	.flags			= FIO_DISKLESSIO | FIO_NOEXTEND |
-				  FIO_NO_OFFLOAD,
+				  FIO_NO_OFFLOAD | FIO_SKIPPABLE_IOMEM_ALLOC,
 
 	.setup			= fio_blkio_setup,
 	.init			= fio_blkio_init,
+	.post_init		= fio_blkio_post_init,
 	.cleanup		= fio_blkio_cleanup,
 
 	.iomem_alloc		= fio_blkio_iomem_alloc,
diff --git a/fio.1 b/fio.1
index 64a774f5..12eeb013 100644
--- a/fio.1
+++ b/fio.1
@@ -1992,7 +1992,9 @@ engine specific options. (See \fIhttps://xnvme.io/\fR).
 .TP
 .B libblkio
 Use the libblkio library (\fIhttps://gitlab.com/libblkio/libblkio\fR). The
-specific \fBdriver\fR to use must be set using \fBlibblkio_driver\fR.
+specific \fBdriver\fR to use must be set using \fBlibblkio_driver\fR. If
+\fBmem\fR/\fBiomem\fR is not specified, memory allocation is delegated to
+libblkio (and so is guaranteed to work with the selected \fBdriver\fR).
 .SS "I/O engine specific parameters"
 In addition, there are some parameters which are only valid when a specific
 \fBioengine\fR is in use. These are used identically to normal parameters,
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 04/10] engines/libblkio: Add support for poll queues
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (2 preceding siblings ...)
  2022-11-21 18:28 ` [PATCH 03/10] engines/libblkio: Allow setting option mem/iomem Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-12-01 17:01   ` Vincent Fu
  2022-11-21 18:28 ` [PATCH 05/10] engines/libblkio: Add option libblkio_vectored Alberto Faria
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

Configure a poll queue instead of a "regular" queue when option hipri is
set.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst          |  4 ++++
 engines/libblkio.c | 26 ++++++++++++++++++++++++--
 fio.1              |  3 +++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index fb27d3d3..cdea3258 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2330,6 +2330,10 @@ with the caveat that when used on the command line, they must come after the
         by the application. The benefits are more efficient IO for high IOPS
         scenarios, and lower latencies for low queue depth IO.
 
+   [libblkio]
+
+	Use poll queues.
+
    [pvsync2]
 
 	Set RWF_HIPRI on I/O, indicating to the kernel that it's of higher priority
diff --git a/engines/libblkio.c b/engines/libblkio.c
index 54fd9ff6..d2ade3f1 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -26,6 +26,8 @@ struct fio_blkio_options {
 	char *driver;
 	char *pre_connect_props;
 	char *pre_start_props;
+
+	unsigned int hipri;
 };
 
 static struct fio_option options[] = {
@@ -57,6 +59,16 @@ static struct fio_option options[] = {
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
 
+	{
+		.name	= "hipri",
+		.lname	= "Use poll queues",
+		.type	= FIO_OPT_STR_SET,
+		.off1	= offsetof(struct fio_blkio_options, hipri),
+		.help	= "Use poll queues",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
+
 	{
 		.name = NULL,
 	},
@@ -244,6 +256,7 @@ struct fio_blkio_data {
 
 static int fio_blkio_init(struct thread_data *td)
 {
+	const struct fio_blkio_options *options = td->eo;
 	struct fio_blkio_data *data;
 
 	/* allocate per-thread data struct */
@@ -265,7 +278,13 @@ static int fio_blkio_init(struct thread_data *td)
 	if (fio_blkio_create_and_connect(td, &data->b) != 0)
 		goto err_free;
 
-	if (blkio_set_int(data->b, "num-queues", 1) != 0) {
+	if (blkio_set_int(data->b, "num-queues", options->hipri ? 0 : 1) != 0) {
+		fio_blkio_log_err(blkio_set_int);
+		goto err_blkio_destroy;
+	}
+
+	if (blkio_set_int(data->b, "num-poll-queues",
+			  options->hipri ? 1 : 0) != 0) {
 		fio_blkio_log_err(blkio_set_int);
 		goto err_blkio_destroy;
 	}
@@ -277,7 +296,10 @@ static int fio_blkio_init(struct thread_data *td)
 
 	/* get queue */
 
-	data->q = blkio_get_queue(data->b, 0);
+	if (options->hipri)
+		data->q = blkio_get_poll_queue(data->b, 0);
+	else
+		data->q = blkio_get_queue(data->b, 0);
 
 	/* Set data only here so cleanup() does nothing if init() fails. */
 	td->io_ops_data = data;
diff --git a/fio.1 b/fio.1
index 12eeb013..7c1b315a 100644
--- a/fio.1
+++ b/fio.1
@@ -2620,6 +2620,9 @@ A colon-separated list of libblkio properties to be set after connecting but
 before starting the \fBstruct blkio\fR. Each property must have the format
 \fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
 the engine sets any other properties, so those can be overriden.
+.TP
+.BI (libblkio)hipri \fR=\fPbool
+Use poll queues.
 .SS "I/O depth"
 .TP
 .BI iodepth \fR=\fPint
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 05/10] engines/libblkio: Add option libblkio_vectored
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (3 preceding siblings ...)
  2022-11-21 18:28 ` [PATCH 04/10] engines/libblkio: Add support for poll queues Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-12-01 17:11   ` Vincent Fu
  2022-11-21 18:28 ` [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim Alberto Faria
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

When enabled, read and write requests are submitted as vectored requests
using blkioq_{readv,writev}(), instead of using blkioq_{read,write}().

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst          |  4 ++++
 engines/libblkio.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
 fio.1              |  3 +++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index cdea3258..b9c7c8df 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2875,6 +2875,10 @@ with the caveat that when used on the command line, they must come after the
 	set after the engine sets any other properties, so those can be
 	overriden.
 
+.. option:: libblkio_vectored=bool : [libblkio]
+
+	Submit vectored read and write requests. Default is 0.
+
 I/O depth
 ~~~~~~~~~
 
diff --git a/engines/libblkio.c b/engines/libblkio.c
index d2ade3f1..dcf701ad 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -28,6 +28,7 @@ struct fio_blkio_options {
 	char *pre_start_props;
 
 	unsigned int hipri;
+	unsigned int vectored;
 };
 
 static struct fio_option options[] = {
@@ -68,6 +69,15 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
+	{
+		.name	= "libblkio_vectored",
+		.lname	= "Use blkioq_{readv,writev}()",
+		.type	= FIO_OPT_STR_SET,
+		.off1	= offsetof(struct fio_blkio_options, vectored),
+		.help	= "Use blkioq_{readv,writev}() instead of blkioq_{read,write}()",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
 
 	{
 		.name = NULL,
@@ -251,6 +261,7 @@ struct fio_blkio_data {
 	bool has_mem_region; /* whether mem_region is valid */
 	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
 
+	struct iovec *iovecs; /* for vectored requests */
 	struct blkio_completion *completions;
 };
 
@@ -267,8 +278,9 @@ static int fio_blkio_init(struct thread_data *td)
 		goto err_free;
 	}
 
+	data->iovecs = calloc(td->o.iodepth, sizeof(data->iovecs[0]));
 	data->completions = calloc(td->o.iodepth, sizeof(data->completions[0]));
-	if (!data->completions) {
+	if (!data->iovecs || !data->completions) {
 		log_err("fio: calloc() failed\n");
 		goto err_free;
 	}
@@ -310,6 +322,7 @@ err_blkio_destroy:
 	blkio_destroy(&data->b);
 err_free:
 	free(data->completions);
+	free(data->iovecs);
 	free(data);
 	return 1;
 }
@@ -362,6 +375,7 @@ static void fio_blkio_cleanup(struct thread_data *td)
 	if (data) {
 		blkio_destroy(&data->b);
 		free(data->completions);
+		free(data->iovecs);
 		free(data);
 	}
 }
@@ -432,19 +446,41 @@ static int fio_blkio_open_file(struct thread_data *td, struct fio_file *f)
 static enum fio_q_status fio_blkio_queue(struct thread_data *td,
 					 struct io_u *io_u)
 {
+	const struct fio_blkio_options *options = td->eo;
 	struct fio_blkio_data *data = td->io_ops_data;
 
 	fio_ro_check(td, io_u);
 
 	switch (io_u->ddir) {
 		case DDIR_READ:
-			blkioq_read(data->q, io_u->offset, io_u->xfer_buf,
-				    (size_t)io_u->xfer_buflen, io_u, 0);
+			if (options->vectored) {
+				struct iovec *iov = &data->iovecs[io_u->index];
+				iov->iov_base = io_u->xfer_buf;
+				iov->iov_len = (size_t)io_u->xfer_buflen;
+
+				blkioq_readv(data->q, io_u->offset, iov, 1,
+					     io_u, 0);
+			} else {
+				blkioq_read(data->q, io_u->offset,
+					    io_u->xfer_buf,
+					    (size_t)io_u->xfer_buflen, io_u, 0);
+			}
 			break;
 
 		case DDIR_WRITE:
-			blkioq_write(data->q, io_u->offset, io_u->xfer_buf,
-				     (size_t)io_u->xfer_buflen, io_u, 0);
+			if (options->vectored) {
+				struct iovec *iov = &data->iovecs[io_u->index];
+				iov->iov_base = io_u->xfer_buf;
+				iov->iov_len = (size_t)io_u->xfer_buflen;
+
+				blkioq_writev(data->q, io_u->offset, iov, 1,
+					      io_u, 0);
+			} else {
+				blkioq_write(data->q, io_u->offset,
+					     io_u->xfer_buf,
+					     (size_t)io_u->xfer_buflen, io_u,
+					     0);
+			}
 			break;
 
 		case DDIR_TRIM:
diff --git a/fio.1 b/fio.1
index 7c1b315a..a403b415 100644
--- a/fio.1
+++ b/fio.1
@@ -2623,6 +2623,9 @@ the engine sets any other properties, so those can be overriden.
 .TP
 .BI (libblkio)hipri \fR=\fPbool
 Use poll queues.
+.TP
+.BI (libblkio)libblkio_vectored \fR=\fPbool
+Submit vectored read and write requests. Default is 0.
 .SS "I/O depth"
 .TP
 .BI iodepth \fR=\fPint
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (4 preceding siblings ...)
  2022-11-21 18:28 ` [PATCH 05/10] engines/libblkio: Add option libblkio_vectored Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-12-01 17:13   ` Vincent Fu
  2022-11-21 18:28 ` [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode Alberto Faria
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

When set, trim IOs will be submitted as blkioq_write_zeroes() requests
instead of blkioq_discard() requests.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst          |  5 +++++
 engines/libblkio.c | 20 ++++++++++++++++++--
 fio.1              |  4 ++++
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index b9c7c8df..7155add6 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2879,6 +2879,11 @@ with the caveat that when used on the command line, they must come after the
 
 	Submit vectored read and write requests. Default is 0.
 
+.. option:: libblkio_write_zeroes_on_trim=bool : [libblkio]
+
+	Submit trims as "write zeroes" requests instead of discard requests.
+	Default is 0.
+
 I/O depth
 ~~~~~~~~~
 
diff --git a/engines/libblkio.c b/engines/libblkio.c
index dcf701ad..79af3aa7 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -29,6 +29,7 @@ struct fio_blkio_options {
 
 	unsigned int hipri;
 	unsigned int vectored;
+	unsigned int write_zeroes_on_trim;
 };
 
 static struct fio_option options[] = {
@@ -78,6 +79,16 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
+	{
+		.name	= "libblkio_write_zeroes_on_trim",
+		.lname	= "Use blkioq_write_zeroes() for TRIM",
+		.type	= FIO_OPT_STR_SET,
+		.off1	= offsetof(struct fio_blkio_options,
+				   write_zeroes_on_trim),
+		.help	= "Use blkioq_write_zeroes() for TRIM instead of blkioq_discard()",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
 
 	{
 		.name = NULL,
@@ -484,8 +495,13 @@ static enum fio_q_status fio_blkio_queue(struct thread_data *td,
 			break;
 
 		case DDIR_TRIM:
-			blkioq_discard(data->q, io_u->offset, io_u->xfer_buflen,
-				       io_u, 0);
+			if (options->write_zeroes_on_trim) {
+				blkioq_write_zeroes(data->q, io_u->offset,
+						    io_u->xfer_buflen, io_u, 0);
+			} else {
+				blkioq_discard(data->q, io_u->offset,
+					       io_u->xfer_buflen, io_u, 0);
+			}
 		        break;
 
 		case DDIR_SYNC:
diff --git a/fio.1 b/fio.1
index a403b415..053c2eda 100644
--- a/fio.1
+++ b/fio.1
@@ -2626,6 +2626,10 @@ Use poll queues.
 .TP
 .BI (libblkio)libblkio_vectored \fR=\fPbool
 Submit vectored read and write requests. Default is 0.
+.TP
+.BI (libblkio)libblkio_write_zeroes_on_trim \fR=\fPbool
+Submit trims as "write zeroes" requests instead of discard requests. Default is
+0.
 .SS "I/O depth"
 .TP
 .BI iodepth \fR=\fPint
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (5 preceding siblings ...)
  2022-11-21 18:28 ` [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim Alberto Faria
@ 2022-11-21 18:28 ` Alberto Faria
  2022-12-01 17:21   ` Vincent Fu
  2022-11-21 18:29 ` [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd Alberto Faria
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:28 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

It allows configuring how the engine waits for request completions,
instead of always using a blocking blkioq_do_io() call.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst          |  14 ++++-
 engines/libblkio.c | 132 ++++++++++++++++++++++++++++++++++++++++++---
 fio.1              |  16 +++++-
 3 files changed, 154 insertions(+), 8 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 7155add6..4b059cb2 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2332,7 +2332,8 @@ with the caveat that when used on the command line, they must come after the
 
    [libblkio]
 
-	Use poll queues.
+	Use poll queues. This is incompatible with
+	:option:`libblkio_wait_mode=eventfd <libblkio_wait_mode>`.
 
    [pvsync2]
 
@@ -2884,6 +2885,17 @@ with the caveat that when used on the command line, they must come after the
 	Submit trims as "write zeroes" requests instead of discard requests.
 	Default is 0.
 
+.. option:: libblkio_wait_mode=str : [libblkio]
+
+	How to wait for completions:
+
+	**block** (default)
+		Use a blocking call to ``blkioq_do_io()``.
+	**eventfd**
+		Use a blocking call to ``read()`` on the completion eventfd.
+	**loop**
+		Use a busy loop with a non-blocking call to ``blkioq_do_io()``.
+
 I/O depth
 ~~~~~~~~~
 
diff --git a/engines/libblkio.c b/engines/libblkio.c
index 79af3aa7..d7666f69 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -20,6 +20,12 @@
 #include "../options.h"
 #include "../parse.h"
 
+enum fio_blkio_wait_mode {
+	FIO_BLKIO_WAIT_MODE_BLOCK,
+	FIO_BLKIO_WAIT_MODE_EVENTFD,
+	FIO_BLKIO_WAIT_MODE_LOOP,
+};
+
 struct fio_blkio_options {
 	void *pad; /* option fields must not have offset 0 */
 
@@ -30,6 +36,7 @@ struct fio_blkio_options {
 	unsigned int hipri;
 	unsigned int vectored;
 	unsigned int write_zeroes_on_trim;
+	enum fio_blkio_wait_mode wait_mode;
 };
 
 static struct fio_option options[] = {
@@ -89,6 +96,30 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
+	{
+		.name	= "libblkio_wait_mode",
+		.lname	= "How to wait for completions",
+		.type	= FIO_OPT_STR,
+		.off1	= offsetof(struct fio_blkio_options, wait_mode),
+		.help	= "How to wait for completions",
+		.def	= "block",
+		.posval = {
+			  { .ival = "block",
+			    .oval = FIO_BLKIO_WAIT_MODE_BLOCK,
+			    .help = "Blocking blkioq_do_io()",
+			  },
+			  { .ival = "eventfd",
+			    .oval = FIO_BLKIO_WAIT_MODE_EVENTFD,
+			    .help = "Blocking read() on the completion eventfd",
+			  },
+			  { .ival = "loop",
+			    .oval = FIO_BLKIO_WAIT_MODE_LOOP,
+			    .help = "Busy loop with non-blocking blkioq_do_io()",
+			  },
+		},
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
 
 	{
 		.name = NULL,
@@ -237,12 +268,21 @@ static int fio_blkio_setup(struct thread_data *td)
 	 * again in the init() callback.
 	 */
 
+	const struct fio_blkio_options *options = td->eo;
 	struct blkio *b;
 	int ret = 0;
 	uint64_t capacity;
 
 	assert(td->files_index == 1);
 
+	/* validate options */
+
+	if (options->hipri &&
+		options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
+		log_err("fio: option hipri is incompatible with option libblkio_wait_mode=eventfd\n");
+		return 1;
+	}
+
 	/* get target size */
 
 	if (fio_blkio_create_and_connect(td, &b) != 0)
@@ -268,6 +308,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 */
 
 	bool has_mem_region; /* whether mem_region is valid */
 	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
@@ -280,6 +321,7 @@ static int fio_blkio_init(struct thread_data *td)
 {
 	const struct fio_blkio_options *options = td->eo;
 	struct fio_blkio_data *data;
+	int flags;
 
 	/* allocate per-thread data struct */
 
@@ -324,6 +366,31 @@ static int fio_blkio_init(struct thread_data *td)
 	else
 		data->q = blkio_get_queue(data->b, 0);
 
+	/* enable completion fd and make it blocking */
+
+	if (options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
+		blkioq_set_completion_fd_enabled(data->q, true);
+
+		data->completion_fd = blkioq_get_completion_fd(data->q);
+
+		flags = fcntl(data->completion_fd, F_GETFL);
+		if (flags < 0) {
+			log_err("fio: fcntl(F_GETFL) failed: %s\n",
+				strerror(errno));
+			goto err_blkio_destroy;
+		}
+
+		flags &= ~O_NONBLOCK;
+
+		if (fcntl(data->completion_fd, F_SETFL, flags) != 0) {
+			log_err("fio: fcntl(F_SETFL) failed: %s\n",
+				strerror(errno));
+			goto err_blkio_destroy;
+		}
+	} else {
+		data->completion_fd = -1;
+	}
+
 	/* Set data only here so cleanup() does nothing if init() fails. */
 	td->io_ops_data = data;
 
@@ -521,16 +588,69 @@ static enum fio_q_status fio_blkio_queue(struct thread_data *td,
 static int fio_blkio_getevents(struct thread_data *td, unsigned int min,
 			       unsigned int max, const struct timespec *t)
 {
+	const struct fio_blkio_options *options = td->eo;
 	struct fio_blkio_data *data = td->io_ops_data;
-	int n;
+	int ret, n;
+	uint64_t event;
 
-	n = blkioq_do_io(data->q, data->completions, (int)min, (int)max, NULL);
-	if (n < 0) {
-		fio_blkio_log_err(blkioq_do_io);
+	switch (options->wait_mode) {
+	case FIO_BLKIO_WAIT_MODE_BLOCK:
+
+		n = blkioq_do_io(data->q, data->completions, (int)min, (int)max,
+				 NULL);
+		if (n < 0) {
+			fio_blkio_log_err(blkioq_do_io);
+			return -1;
+		}
+
+		return n;
+
+	case FIO_BLKIO_WAIT_MODE_EVENTFD:
+
+		n = blkioq_do_io(data->q, data->completions, 0, (int)max, NULL);
+		if (n < 0) {
+			fio_blkio_log_err(blkioq_do_io);
+			return -1;
+		}
+
+		while (n < (int)min) {
+			ret = read(data->completion_fd, &event, sizeof(event));
+			if (ret != sizeof(event)) {
+				log_err("fio: read() on the completion fd returned %d\n",
+					ret);
+				return -1;
+			}
+
+			ret = blkioq_do_io(data->q, data->completions + n, 0,
+					   (int)max - n, NULL);
+			if (ret < 0) {
+				fio_blkio_log_err(blkioq_do_io);
+				return -1;
+			}
+
+			n += ret;
+		}
+
+		return n;
+
+	case FIO_BLKIO_WAIT_MODE_LOOP:
+
+		for (n = 0; n < (int)min; ) {
+			ret = blkioq_do_io(data->q, data->completions + n, 0,
+					   (int)max - n, NULL);
+			if (ret < 0) {
+				fio_blkio_log_err(blkioq_do_io);
+				return -1;
+			}
+
+			n += ret;
+		}
+
+		return n;
+
+	default:
 		return -1;
 	}
-
-	return n;
 }
 
 static struct io_u *fio_blkio_event(struct thread_data *td, int event)
diff --git a/fio.1 b/fio.1
index 053c2eda..88bb232f 100644
--- a/fio.1
+++ b/fio.1
@@ -2622,7 +2622,7 @@ 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.
+Use poll queues. This is incompatible with \fBlibblkio_wait_mode=eventfd\fR.
 .TP
 .BI (libblkio)libblkio_vectored \fR=\fPbool
 Submit vectored read and write requests. Default is 0.
@@ -2630,6 +2630,20 @@ Submit vectored read and write requests. Default is 0.
 .BI (libblkio)libblkio_write_zeroes_on_trim \fR=\fPbool
 Submit trims as "write zeroes" requests instead of discard requests. Default is
 0.
+.TP
+.BI (libblkio)libblkio_wait_mode \fR=\fPstr
+How to wait for completions:
+.RS
+.RS
+.TP
+.B block \fR(default)
+Use a blocking call to \fBblkioq_do_io()\fR.
+.TP
+.B eventfd
+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.
 .SS "I/O depth"
 .TP
 .BI iodepth \fR=\fPint
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (6 preceding siblings ...)
  2022-11-21 18:28 ` [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode Alberto Faria
@ 2022-11-21 18:29 ` Alberto Faria
  2022-12-01 17:23   ` Vincent Fu
  2022-11-21 18:29 ` [PATCH 09/10] engines/libblkio: Add options for some driver-specific properties Alberto Faria
  2022-11-21 18:29 ` [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process Alberto Faria
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:29 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

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]
+
+	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
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 09/10] engines/libblkio: Add options for some driver-specific properties
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (7 preceding siblings ...)
  2022-11-21 18:29 ` [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd Alberto Faria
@ 2022-11-21 18:29 ` Alberto Faria
  2022-11-21 18:29 ` [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process Alberto Faria
  9 siblings, 0 replies; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:29 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

The properties are either common to several drivers or particularly
relevant for benchmarking, so this should help write cleaner workload
files.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst                                 | 39 ++++++++++----
 engines/libblkio.c                        | 64 ++++++++++++++++++++++-
 examples/libblkio-io_uring.fio            |  5 +-
 examples/libblkio-virtio-blk-vfio-pci.fio |  5 +-
 fio.1                                     | 32 +++++++++---
 5 files changed, 119 insertions(+), 26 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 7b3a3ffc..763f4f51 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2861,21 +2861,40 @@ with the caveat that when used on the command line, they must come after the
 
 	The driver to be used by libblkio (e.g. **virtio-blk-vfio-pci**).
 
+.. option:: libblkio_path=str : [libblkio]
+
+	Sets the value of the driver-specific "path" property prior to calling
+	``blkio_connect()``, which identifies the target device or file on which
+	to perform I/O. Its exact semantics are driver-dependent and not all
+	drivers may support it.
+
 .. option:: libblkio_pre_connect_props=str : [libblkio]
 
-	A colon-separated list of libblkio properties to be set after creating
-	but before connecting the ``struct blkio``. Each property must have the
-	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
-	set after the engine sets any other properties, so those can be
-	overriden.
+	A colon-separated list of additional libblkio properties to be set after
+	creating but before connecting the ``struct blkio``. Each property must
+	have the format ``<name>=<value>``. Colons can be escaped as ``\:``.
+	These are set after the engine sets any other properties, so those can
+	be overriden.
+
+.. option:: libblkio_num_entries=int : [libblkio]
+
+	Sets the value of the driver-specific "num-entries" property prior to
+	calling ``blkio_start()``. Its exact semantics are driver-dependent and
+	not all drivers may support it.
+
+.. option:: libblkio_queue_size=int : [libblkio]
+
+	Sets the value of the driver-specific "queue-size" property prior to
+	calling ``blkio_start()``. Its exact semantics are driver-dependent and
+	not all drivers may support it.
 
 .. option:: libblkio_pre_start_props=str : [libblkio]
 
-	A colon-separated list of libblkio properties to be set after connecting
-	but before starting the ``struct blkio``. Each property must have the
-	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
-	set after the engine sets any other properties, so those can be
-	overriden.
+	A colon-separated list of additional libblkio properties to be set after
+	connecting but before starting the ``struct blkio``. Each property must
+	have the format ``<name>=<value>``. Colons can be escaped as ``\:``.
+	These are set after the engine sets any other properties, so those can
+	be overriden.
 
 .. option:: libblkio_vectored=bool : [libblkio]
 
diff --git a/engines/libblkio.c b/engines/libblkio.c
index 3ff87e6d..a80be66f 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -30,7 +30,12 @@ struct fio_blkio_options {
 	void *pad; /* option fields must not have offset 0 */
 
 	char *driver;
+
+	char *path;
 	char *pre_connect_props;
+
+	int num_entries;
+	int queue_size;
 	char *pre_start_props;
 
 	unsigned int hipri;
@@ -50,18 +55,51 @@ static struct fio_option options[] = {
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
+
+	{
+		.name	= "libblkio_path",
+		.lname	= "libblkio \"path\" property",
+		.type	= FIO_OPT_STR_STORE,
+		.off1	= offsetof(struct fio_blkio_options, path),
+		.help	= "Value to set the \"path\" property to",
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
 	{
 		.name	= "libblkio_pre_connect_props",
-		.lname	= "Properties to be set before blkio_connect()",
+		.lname	= "Additional properties to be set before blkio_connect()",
 		.type	= FIO_OPT_STR_STORE,
 		.off1	= offsetof(struct fio_blkio_options, pre_connect_props),
 		.help	= "",
 		.category = FIO_OPT_C_ENGINE,
 		.group	= FIO_OPT_G_LIBBLKIO,
 	},
+
+	{
+		.name	= "libblkio_num_entries",
+		.lname	= "libblkio \"num-entries\" property",
+		.type	= FIO_OPT_INT,
+		.off1	= offsetof(struct fio_blkio_options, num_entries),
+		.help	= "Value to set the \"num-entries\" property to",
+		.minval	= 1,
+		.interval = 1,
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
+	{
+		.name	= "libblkio_queue_size",
+		.lname	= "libblkio \"queue-size\" property",
+		.type	= FIO_OPT_INT,
+		.off1	= offsetof(struct fio_blkio_options, queue_size),
+		.help	= "Value to set the \"queue-size\" property to",
+		.minval	= 1,
+		.interval = 1,
+		.category = FIO_OPT_C_ENGINE,
+		.group	= FIO_OPT_G_LIBBLKIO,
+	},
 	{
 		.name	= "libblkio_pre_start_props",
-		.lname	= "Properties to be set before blkio_start()",
+		.lname	= "Additional properties to be set before blkio_start()",
 		.type	= FIO_OPT_STR_STORE,
 		.off1	= offsetof(struct fio_blkio_options, pre_start_props),
 		.help	= "",
@@ -244,6 +282,13 @@ static int fio_blkio_create_and_connect(struct thread_data *td,
 		goto err_blkio_destroy;
 	}
 
+	if (options->path) {
+		if (blkio_set_str(b, "path", options->path) != 0) {
+			fio_blkio_log_err(blkio_set_str);
+			goto err_blkio_destroy;
+		}
+	}
+
 	if (fio_blkio_set_props_from_str(b, "libblkio_pre_connect_props",
 					 options->pre_connect_props) != 0)
 		goto err_blkio_destroy;
@@ -257,6 +302,21 @@ static int fio_blkio_create_and_connect(struct thread_data *td,
 
 	/* set pre-start properties */
 
+	if (options->num_entries != 0) {
+		if (blkio_set_int(b, "num-entries",
+				  options->num_entries) != 0) {
+			fio_blkio_log_err(blkio_set_int);
+			goto err_blkio_destroy;
+		}
+	}
+
+	if (options->queue_size != 0) {
+		if (blkio_set_int(b, "queue-size", options->queue_size) != 0) {
+			fio_blkio_log_err(blkio_set_int);
+			goto err_blkio_destroy;
+		}
+	}
+
 	if (fio_blkio_set_props_from_str(b, "libblkio_pre_start_props",
 					 options->pre_start_props) != 0)
 		goto err_blkio_destroy;
diff --git a/examples/libblkio-io_uring.fio b/examples/libblkio-io_uring.fio
index d2700e51..e5885094 100644
--- a/examples/libblkio-io_uring.fio
+++ b/examples/libblkio-io_uring.fio
@@ -1,15 +1,14 @@
 ; Benchmark accessing a regular file or block device using libblkio.
 ;
 ; Replace "libblkio_path" below with the path to your file or device, or
-; override it by passing the '--libblkio_pre_connect_props=path=...' flag to
-; fio.
+; override it by passing the '--libblkio_path=...' flag to fio.
 ;
 ; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
 
 [global]
 ioengine=libblkio
 libblkio_driver=io_uring
-libblkio_pre_connect_props=path=/dev/nvme0n1  ; REPLACE THIS WITH THE RIGHT PATH
+libblkio_path=/dev/nvme0n1  ; REPLACE THIS WITH THE RIGHT PATH
 rw=randread
 blocksize=4k
 direct=1
diff --git a/examples/libblkio-virtio-blk-vfio-pci.fio b/examples/libblkio-virtio-blk-vfio-pci.fio
index 79d6228d..ba89f7ef 100644
--- a/examples/libblkio-virtio-blk-vfio-pci.fio
+++ b/examples/libblkio-virtio-blk-vfio-pci.fio
@@ -1,15 +1,14 @@
 ; Benchmark accessing a PCI virtio-blk device using libblkio.
 ;
 ; Replace "libblkio_path" below with the path to your device's sysfs directory,
-; or override it by passing the '--libblkio_pre_connect_props=path=...' flag to
-; fio. Note that colons in the path must be escaped with a backslash.
+; or override it by passing the '--libblkio_path=...' flag to fio.
 ;
 ; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
 
 [global]
 ioengine=libblkio
 libblkio_driver=virtio-blk-vfio-pci
-libblkio_pre_connect_props=path=/sys/bus/pci/devices/0000\:00\:01.0  ; REPLACE THIS WITH THE RIGHT PATH
+libblkio_path=/sys/bus/pci/devices/0000:00:01.0  ; REPLACE THIS WITH THE RIGHT PATH
 rw=randread
 blocksize=4k
 time_based=1
diff --git a/fio.1 b/fio.1
index e2f92bd1..f20a4164 100644
--- a/fio.1
+++ b/fio.1
@@ -2609,17 +2609,33 @@ If this option is set, xnvme will use vectored read/write commands.
 .BI (libblkio)libblkio_driver \fR=\fPstr
 The driver to be used by libblkio (e.g. \fBvirtio-blk-vfio-pci\fR).
 .TP
+.BI (libblkio)libblkio_path \fR=\fPstr
+Sets the value of the driver-specific "path" property prior to calling
+\fBblkio_connect()\fR, which identifies the target device or file on which to
+perform I/O. Its exact semantics are driver-dependent and not all drivers may
+support it.
+.TP
 .BI (libblkio)libblkio_pre_connect_props \fR=\fPstr
-A colon-separated list of libblkio properties to be set after creating but
-before connecting the \fBstruct blkio\fR. Each property must have the format
-\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
-the engine sets any other properties, so those can be overriden.
+A colon-separated list of additional libblkio properties to be set after
+creating but before connecting the \fBstruct blkio\fR. Each property must have
+the format \fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are
+set after the engine sets any other properties, so those can be overriden.
+.TP
+.BI (libblkio)libblkio_num_entries \fR=\fPint
+Sets the value of the driver-specific "num-entries" property prior to calling
+\fBblkio_start()\fR. Its exact semantics are driver-dependent and not all
+drivers may support it.
+.TP
+.BI (libblkio)libblkio_queue_size \fR=\fPint
+Sets the value of the driver-specific "queue-size" property prior to calling
+\fBblkio_start()\fR. Its exact semantics are driver-dependent and not all
+drivers may support it.
 .TP
 .BI (libblkio)libblkio_pre_start_props \fR=\fPstr
-A colon-separated list of libblkio properties to be set after connecting but
-before starting the \fBstruct blkio\fR. Each property must have the format
-\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
-the engine sets any other properties, so those can be overriden.
+A colon-separated list of additional libblkio properties to be set after
+connecting but before starting the \fBstruct blkio\fR. Each property must have
+the format \fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are
+set after 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 and
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process
  2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
                   ` (8 preceding siblings ...)
  2022-11-21 18:29 ` [PATCH 09/10] engines/libblkio: Add options for some driver-specific properties Alberto Faria
@ 2022-11-21 18:29 ` Alberto Faria
  2022-12-01 17:29   ` Vincent Fu
  9 siblings, 1 reply; 20+ messages in thread
From: Alberto Faria @ 2022-11-21 18:29 UTC (permalink / raw)
  To: fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella, Alberto Faria

fio groups all subjobs that set option 'thread' into a single process.
Have them all share a single `struct blkio` instance, with one `struct
blkioq` per thread/subjob. This allows benchmarking multi-queue setups.

Note that `struct blkio` instances cannot be shared across different
processes.

Signed-off-by: Alberto Faria <afaria@redhat.com>
---
 HOWTO.rst                                 |   8 +-
 engines/libblkio.c                        | 250 +++++++++++++++++++---
 examples/libblkio-io_uring.fio            |  13 +-
 examples/libblkio-virtio-blk-vfio-pci.fio |  13 +-
 fio.1                                     |   7 +-
 5 files changed, 257 insertions(+), 34 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 763f4f51..4e69abfc 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -2199,7 +2199,13 @@ I/O engine
 			:option:`libblkio_driver`. If
 			:option:`mem`/:option:`iomem` is not specified, memory
 			allocation is delegated to libblkio (and so is
-			guaranteed to work with the selected *driver*).
+			guaranteed to work with the selected *driver*). One
+			``struct blkio`` instance is used per process, so all
+			jobs setting option :option:`thread` will share a single
+			``struct blkio`` (with one queue per thread) and must
+			specify compatible options. Note that some drivers don't
+			allow several processes to access the same device or
+			file simultaneously, but allow it for threads.
 
 I/O engine specific parameters
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/engines/libblkio.c b/engines/libblkio.c
index a80be66f..987dea4f 100644
--- a/engines/libblkio.c
+++ b/engines/libblkio.c
@@ -249,6 +249,105 @@ static int fio_blkio_set_props_from_str(struct blkio *b, const char *opt_name,
 			blkio_get_error_msg()); \
 	})
 
+static bool possibly_null_strs_equal(const char *a, const char *b)
+{
+	return (!a && !b) || (a && b && strcmp(a, b) == 0);
+}
+
+/*
+ * Returns the total number of subjobs using option 'thread' in the entire
+ * workload that have the given value for the 'hipri' option.
+ */
+static int total_threaded_subjobs(bool hipri)
+{
+	struct thread_data *td;
+	unsigned int i;
+	int count = 0;
+
+	for_each_td(td, i) {
+		const struct fio_blkio_options *options = td->eo;
+		if (td->o.use_thread && (bool)options->hipri == hipri)
+			++count;
+	}
+
+	return count;
+}
+
+static struct {
+	bool set_up;
+	bool direct;
+	struct fio_blkio_options opts;
+} first_threaded_subjob = { 0 };
+
+static void fio_blkio_log_opt_compat_err(const char *option_name)
+{
+	log_err("fio: jobs using engine libblkio and sharing a process must agree on the %s option\n",
+		option_name);
+}
+
+/*
+ * If td represents a subjob with option 'thread', check if its options are
+ * compatible with those of other threaded subjobs that were already set up.
+ */
+static int fio_blkio_check_opt_compat(struct thread_data *td)
+{
+	const struct fio_blkio_options *options = td->eo, *prev_options;
+
+	if (!td->o.use_thread)
+		return 0; /* subjob doesn't use 'thread' */
+
+	if (!first_threaded_subjob.set_up) {
+		/* first subjob using 'thread', store options for later */
+		first_threaded_subjob.set_up	= true;
+		first_threaded_subjob.direct	= td->o.odirect;
+		first_threaded_subjob.opts	= *options;
+		return 0;
+	}
+
+	/* not first subjob using 'thread', check option compatibility */
+
+	prev_options = &first_threaded_subjob.opts;
+
+	if (td->o.odirect != first_threaded_subjob.direct) {
+		fio_blkio_log_opt_compat_err("direct/buffered");
+		return 1;
+	}
+
+	if (strcmp(options->driver, prev_options->driver) != 0) {
+		fio_blkio_log_opt_compat_err("libblkio_driver");
+		return 1;
+	}
+
+	if (!possibly_null_strs_equal(options->path, prev_options->path)) {
+		fio_blkio_log_opt_compat_err("libblkio_path");
+		return 1;
+	}
+
+	if (!possibly_null_strs_equal(options->pre_connect_props,
+				      prev_options->pre_connect_props)) {
+		fio_blkio_log_opt_compat_err("libblkio_pre_connect_props");
+		return 1;
+	}
+
+	if (options->num_entries != prev_options->num_entries) {
+		fio_blkio_log_opt_compat_err("libblkio_num_entries");
+		return 1;
+	}
+
+	if (options->queue_size != prev_options->queue_size) {
+		fio_blkio_log_opt_compat_err("libblkio_queue_size");
+		return 1;
+	}
+
+	if (!possibly_null_strs_equal(options->pre_start_props,
+				      prev_options->pre_start_props)) {
+		fio_blkio_log_opt_compat_err("libblkio_pre_start_props");
+		return 1;
+	}
+
+	return 0;
+}
+
 static int fio_blkio_create_and_connect(struct thread_data *td,
 					struct blkio **out_blkio)
 {
@@ -329,6 +428,8 @@ err_blkio_destroy:
 	return 1;
 }
 
+static bool incompatible_threaded_subjob_options = false;
+
 static int fio_blkio_setup(struct thread_data *td)
 {
 	/*
@@ -348,6 +449,11 @@ static int fio_blkio_setup(struct thread_data *td)
 
 	/* validate options */
 
+	if (fio_blkio_check_opt_compat(td) != 0) {
+		incompatible_threaded_subjob_options = true;
+		return 1;
+	}
+
 	if (options->hipri &&
 		options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
 		log_err("fio: option hipri is incompatible with option libblkio_wait_mode=eventfd\n");
@@ -380,9 +486,16 @@ out_blkio_destroy:
 	return ret;
 }
 
+/* per-process state */
+static struct {
+	pthread_mutex_t mutex;
+	int initted_threads;
+	int initted_hipri_threads;
+	struct blkio *b;
+} proc_state = { PTHREAD_MUTEX_INITIALIZER, 0, 0, NULL };
+
 /* per-thread state */
 struct fio_blkio_data {
-	struct blkio *b;
 	struct blkioq *q;
 	int completion_fd; /* may be -1 if not FIO_BLKIO_WAIT_MODE_EVENTFD */
 
@@ -393,12 +506,33 @@ struct fio_blkio_data {
 	struct blkio_completion *completions;
 };
 
+static void fio_blkio_proc_lock(void) {
+	int ret;
+	ret = pthread_mutex_lock(&proc_state.mutex);
+	assert(ret == 0);
+}
+
+static void fio_blkio_proc_unlock(void) {
+	int ret;
+	ret = pthread_mutex_unlock(&proc_state.mutex);
+	assert(ret == 0);
+}
+
 static int fio_blkio_init(struct thread_data *td)
 {
 	const struct fio_blkio_options *options = td->eo;
 	struct fio_blkio_data *data;
 	int flags;
 
+	if (td->o.use_thread && incompatible_threaded_subjob_options) {
+		/*
+		 * Different subjobs using option 'thread' specified
+		 * incompatible options. We don't know which configuration
+		 * should win, so we just fail all such subjobs.
+		 */
+		return 1;
+	}
+
 	/* allocate per-thread data struct */
 
 	data = calloc(1, sizeof(*data));
@@ -414,33 +548,54 @@ static int fio_blkio_init(struct thread_data *td)
 		goto err_free;
 	}
 
-	/* create, connect, and start blkio */
+	/* initialize per-process blkio */
 
-	if (fio_blkio_create_and_connect(td, &data->b) != 0)
-		goto err_free;
+	fio_blkio_proc_lock();
 
-	if (blkio_set_int(data->b, "num-queues", options->hipri ? 0 : 1) != 0) {
-		fio_blkio_log_err(blkio_set_int);
-		goto err_blkio_destroy;
-	}
+	if (proc_state.initted_threads == 0) {
+		int num_queues, num_poll_queues;
 
-	if (blkio_set_int(data->b, "num-poll-queues",
-			  options->hipri ? 1 : 0) != 0) {
-		fio_blkio_log_err(blkio_set_int);
-		goto err_blkio_destroy;
-	}
+		if (td->o.use_thread) {
+			num_queues 	= total_threaded_subjobs(false);
+			num_poll_queues = total_threaded_subjobs(true);
+		} else {
+			num_queues 	= options->hipri ? 0 : 1;
+			num_poll_queues = options->hipri ? 1 : 0;
+		}
 
-	if (blkio_start(data->b) != 0) {
-		fio_blkio_log_err(blkio_start);
-		goto err_blkio_destroy;
+		/* create, connect, and start blkio */
+
+		if (fio_blkio_create_and_connect(td, &proc_state.b) != 0)
+			goto err_unlock;
+
+		if (blkio_set_int(proc_state.b, "num-queues",
+				  num_queues) != 0) {
+			fio_blkio_log_err(blkio_set_int);
+			goto err_blkio_destroy;
+		}
+
+		if (blkio_set_int(proc_state.b, "num-poll-queues",
+				  num_poll_queues) != 0) {
+			fio_blkio_log_err(blkio_set_int);
+			goto err_blkio_destroy;
+		}
+
+		if (blkio_start(proc_state.b) != 0) {
+			fio_blkio_log_err(blkio_start);
+			goto err_blkio_destroy;
+		}
 	}
 
-	/* get queue */
+	/* get per-thread queue */
 
-	if (options->hipri)
-		data->q = blkio_get_poll_queue(data->b, 0);
-	else
-		data->q = blkio_get_queue(data->b, 0);
+	if (options->hipri) {
+		int i = proc_state.initted_hipri_threads;
+		data->q = blkio_get_poll_queue(proc_state.b, i);
+	} else {
+		int i = proc_state.initted_threads -
+				proc_state.initted_hipri_threads;
+		data->q = blkio_get_queue(proc_state.b, i);
+	}
 
 	/* enable completion fd and make it blocking */
 
@@ -468,13 +623,25 @@ static int fio_blkio_init(struct thread_data *td)
 		data->completion_fd = -1;
 	}
 
+	++proc_state.initted_threads;
+
+	if (options->hipri)
+		++proc_state.initted_hipri_threads;
+
 	/* Set data only here so cleanup() does nothing if init() fails. */
 	td->io_ops_data = data;
 
+	fio_blkio_proc_unlock();
+
 	return 0;
 
 err_blkio_destroy:
-	blkio_destroy(&data->b);
+	if (proc_state.initted_threads == 0)
+		blkio_destroy(&proc_state.b);
+err_unlock:
+	if (proc_state.initted_threads == 0)
+		proc_state.b = NULL;
+	fio_blkio_proc_unlock();
 err_free:
 	free(data->completions);
 	free(data->iovecs);
@@ -514,7 +681,7 @@ static int fio_blkio_post_init(struct thread_data *td)
 			.fd	= -1,
 		};
 
-		if (blkio_map_mem_region(data->b, &region) != 0) {
+		if (blkio_map_mem_region(proc_state.b, &region) != 0) {
 			fio_blkio_log_err(blkio_map_mem_region);
 			return 1;
 		}
@@ -527,11 +694,27 @@ static void fio_blkio_cleanup(struct thread_data *td)
 {
 	struct fio_blkio_data *data = td->io_ops_data;
 
+	/*
+	 * Subjobs from different jobs can be terminated at different times, so
+	 * this callback may be invoked for one subjob while another is still
+	 * doing I/O. Those subjobs may share the process, so we must wait until
+	 * the last subjob in the process wants to clean up to actually destroy
+	 * the blkio.
+	 */
+
 	if (data) {
-		blkio_destroy(&data->b);
 		free(data->completions);
 		free(data->iovecs);
 		free(data);
+
+		fio_blkio_proc_lock();
+
+		if (--proc_state.initted_threads == 0) {
+			blkio_destroy(&proc_state.b);
+			proc_state.b = NULL;
+		}
+
+		fio_blkio_proc_unlock();
 	}
 }
 
@@ -546,7 +729,7 @@ static int fio_blkio_iomem_alloc(struct thread_data *td, size_t size)
 
 	/* round up size to satisfy mem-region-alignment */
 
-	if (blkio_get_uint64(data->b, "mem-region-alignment",
+	if (blkio_get_uint64(proc_state.b, "mem-region-alignment",
 			     &mem_region_alignment) != 0) {
 		fio_blkio_log_err(blkio_get_uint64);
 		return 1;
@@ -556,14 +739,16 @@ static int fio_blkio_iomem_alloc(struct thread_data *td, size_t size)
 
 	/* allocate memory region */
 
-	if (blkio_alloc_mem_region(data->b, &data->mem_region,
+	fio_blkio_proc_lock();
+
+	if (blkio_alloc_mem_region(proc_state.b, &data->mem_region,
 				   aligned_size) != 0) {
 		fio_blkio_log_err(blkio_alloc_mem_region);
 		ret = 1;
 		goto out;
 	}
 
-	if (blkio_map_mem_region(data->b, &data->mem_region) != 0) {
+	if (blkio_map_mem_region(proc_state.b, &data->mem_region) != 0) {
 		fio_blkio_log_err(blkio_map_mem_region);
 		ret = 1;
 		goto out_free;
@@ -576,8 +761,9 @@ static int fio_blkio_iomem_alloc(struct thread_data *td, size_t size)
 	goto out;
 
 out_free:
-	blkio_free_mem_region(data->b, &data->mem_region);
+	blkio_free_mem_region(proc_state.b, &data->mem_region);
 out:
+	fio_blkio_proc_unlock();
 	return ret;
 }
 
@@ -586,8 +772,12 @@ static void fio_blkio_iomem_free(struct thread_data *td)
 	struct fio_blkio_data *data = td->io_ops_data;
 
 	if (data && data->has_mem_region) {
-		blkio_unmap_mem_region(data->b, &data->mem_region);
-		blkio_free_mem_region(data->b, &data->mem_region);
+		fio_blkio_proc_lock();
+
+		blkio_unmap_mem_region(proc_state.b, &data->mem_region);
+		blkio_free_mem_region(proc_state.b, &data->mem_region);
+
+		fio_blkio_proc_unlock();
 
 		data->has_mem_region = false;
 	}
diff --git a/examples/libblkio-io_uring.fio b/examples/libblkio-io_uring.fio
index e5885094..99a79dbe 100644
--- a/examples/libblkio-io_uring.fio
+++ b/examples/libblkio-io_uring.fio
@@ -3,6 +3,10 @@
 ; Replace "libblkio_path" below with the path to your file or device, or
 ; override it by passing the '--libblkio_path=...' flag to fio.
 ;
+; In the example below, the two subjobs of "job-B" *and* the single subjob of
+; "job-C" will share a single `struct blkio` instance, and "job-A" will use a
+; separate `struct blkio` instance.
+;
 ; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
 
 [global]
@@ -15,4 +19,11 @@ direct=1
 time_based=1
 runtime=10s
 
-[job]
+[job-A]
+
+[job-B]
+numjobs=2  ; run two copies of this job simultaneously
+thread=1   ; have each copy run as a separate thread in the *same* process
+
+[job-C]
+thread=1  ; have the job run as a thread in the *same* process as "job-B"
diff --git a/examples/libblkio-virtio-blk-vfio-pci.fio b/examples/libblkio-virtio-blk-vfio-pci.fio
index ba89f7ef..f78e5714 100644
--- a/examples/libblkio-virtio-blk-vfio-pci.fio
+++ b/examples/libblkio-virtio-blk-vfio-pci.fio
@@ -3,6 +3,10 @@
 ; Replace "libblkio_path" below with the path to your device's sysfs directory,
 ; or override it by passing the '--libblkio_path=...' flag to fio.
 ;
+; In the example below, the two subjobs of "job-B" *and* the single subjob of
+; "job-C" will share a single `struct blkio` instance, and "job-A" will use a
+; separate `struct blkio` instance.
+;
 ; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
 
 [global]
@@ -14,4 +18,11 @@ blocksize=4k
 time_based=1
 runtime=10s
 
-[job]
+[job-A]
+
+[job-B]
+numjobs=2  ; run two copies of this job simultaneously
+thread=1   ; have each copy run as a separate thread in the *same* process
+
+[job-C]
+thread=1  ; have the job run as a thread in the *same* process as "job-B"
diff --git a/fio.1 b/fio.1
index f20a4164..cc743174 100644
--- a/fio.1
+++ b/fio.1
@@ -1994,7 +1994,12 @@ engine specific options. (See \fIhttps://xnvme.io/\fR).
 Use the libblkio library (\fIhttps://gitlab.com/libblkio/libblkio\fR). The
 specific \fBdriver\fR to use must be set using \fBlibblkio_driver\fR. If
 \fBmem\fR/\fBiomem\fR is not specified, memory allocation is delegated to
-libblkio (and so is guaranteed to work with the selected \fBdriver\fR).
+libblkio (and so is guaranteed to work with the selected \fBdriver\fR). One
+\fBstruct blkio\fR instance is used per process, so all jobs setting option
+\fBthread\fR will share a single \fBstruct blkio\fR (with one queue per thread)
+and must specify compatible options. Note that some drivers don't allow several
+processes to access the same device or file simultaneously, but allow it for
+threads.
 .SS "I/O engine specific parameters"
 In addition, there are some parameters which are only valid when a specific
 \fBioengine\fR is in use. These are used identically to normal parameters,
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/10] Add a libblkio engine
  2022-11-21 18:28 ` [PATCH 01/10] " Alberto Faria
@ 2022-11-22  9:17   ` Damien Le Moal
  2022-12-01 22:10     ` Alberto Faria
  0 siblings, 1 reply; 20+ messages in thread
From: Damien Le Moal @ 2022-11-22  9:17 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/22/22 03:28, Alberto Faria wrote:
> The libblkio library provides a unified API for efficiently accessing
> block devices using modern high-performance block I/O interfaces like
> io_uring and vhost-user-blk. Using libblkio reduces the amount of code
> needed for interfacing with storage devices and allows developers to
> focus on their applcations.
> 
> Add a libblkio engine that uses libblkio to perform I/O. This is useful
> to benchmark the library itself, and also adds support for storage
> interfaces and devices otherwise not supported by fio, such as
> virtio-blk PCI, vhost-user, and vhost-vDPA devices.
> 
> See the libblkio documentation [2] or KVM Forum 2022 [3] presentation
> for more information on the library itself.
> 
> [1] https://gitlab.com/libblkio/libblkio
> [2] https://libblkio.gitlab.io/libblkio/index.html
> [3] https://static.sched.com/hosted_files/kvmforum2022/8c/libblkio-kvm-forum-2022.pdf
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>  HOWTO.rst                                 |  26 ++
>  Makefile                                  |   6 +
>  configure                                 |  25 ++
>  engines/libblkio.c                        | 463 ++++++++++++++++++++++
>  examples/libblkio-io_uring.fio            |  19 +
>  examples/libblkio-virtio-blk-vfio-pci.fio |  18 +
>  fio.1                                     |  19 +
>  optgroup.h                                |   2 +
>  8 files changed, 578 insertions(+)
>  create mode 100644 engines/libblkio.c
>  create mode 100644 examples/libblkio-io_uring.fio
>  create mode 100644 examples/libblkio-virtio-blk-vfio-pci.fio
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index e796f961..d5a2749c 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2192,6 +2192,12 @@ I/O engine
>  			the SPDK NVMe driver, or your own custom NVMe driver. The xnvme engine includes
>  			engine specific options. (See https://xnvme.io).
>  
> +		**libblkio**
> +			Use the libblkio library
> +			(https://gitlab.com/libblkio/libblkio). The specific
> +			*driver* to use must be set using
> +			:option:`libblkio_driver`.
> +
>  I/O engine specific parameters
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
> @@ -2842,6 +2848,26 @@ with the caveat that when used on the command line, they must come after the
>  
>  	If this option is set. xnvme will use vectored read/write commands.
>  
> +.. option:: libblkio_driver=str : [libblkio]
> +
> +	The driver to be used by libblkio (e.g. **virtio-blk-vfio-pci**).

It would be nice to list the possible values and their meaning here.

> +
> +.. option:: libblkio_pre_connect_props=str : [libblkio]
> +
> +	A colon-separated list of libblkio properties to be set after creating
> +	but before connecting the ``struct blkio``. Each property must have the
> +	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
> +	set after the engine sets any other properties, so those can be
> +	overriden.

"struct blkio" has no meaning whatsoever for the fio command line
interface. So could this be reworded without using struct names ? E.g.
"without connecting the device accessed by libblkio" ?

And we need to list the possible properties or have at least a link to
some documentation listing the possible properties. Otherwise, how can
the user find that out ?

> +
> +.. option:: libblkio_pre_start_props=str : [libblkio]
> +
> +	A colon-separated list of libblkio properties to be set after connecting
> +	but before starting the ``struct blkio``. Each property must have the
> +	format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
> +	set after the engine sets any other properties, so those can be
> +	overriden.

Same comment as above.

> +
>  I/O depth
>  ~~~~~~~~~
>  
> diff --git a/Makefile b/Makefile
> index 7bd572d7..9fd8f59b 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -237,6 +237,12 @@ ifdef CONFIG_LIBXNVME
>    xnvme_CFLAGS = $(LIBXNVME_CFLAGS)
>    ENGINES += xnvme
>  endif
> +ifdef CONFIG_LIBBLKIO
> +  libblkio_SRCS = engines/libblkio.c
> +  libblkio_LIBS = $(LIBBLKIO_LIBS)
> +  libblkio_CFLAGS = $(LIBBLKIO_CFLAGS)
> +  ENGINES += libblkio
> +endif
>  ifeq ($(CONFIG_TARGET_OS), Linux)
>    SOURCE += diskutil.c fifo.c blktrace.c cgroup.c trim.c engines/sg.c \
>  		oslib/linux-dev-lookup.c engines/io_uring.c engines/nvme.c
> diff --git a/configure b/configure
> index 1b12d268..6d8e3a87 100755
> --- a/configure
> +++ b/configure
> @@ -176,6 +176,7 @@ libiscsi="no"
>  libnbd="no"
>  libnfs=""
>  xnvme=""
> +libblkio=""
>  libzbc=""
>  dfs=""
>  seed_buckets=""
> @@ -248,6 +249,8 @@ for opt do
>    ;;
>    --disable-xnvme) xnvme="no"
>    ;;
> +  --disable-libblkio) libblkio="no"
> +  ;;
>    --disable-tcmalloc) disable_tcmalloc="yes"
>    ;;
>    --disable-libnfs) libnfs="no"
> @@ -304,6 +307,7 @@ if test "$show_help" = "yes" ; then
>    echo "--enable-libiscsi       Enable iscsi support"
>    echo "--enable-libnbd         Enable libnbd (NBD engine) support"
>    echo "--disable-xnvme         Disable xnvme support even if found"
> +  echo "--disable-libblkio      Disable libblkio support even if found"
>    echo "--disable-libzbc        Disable libzbc even if found"
>    echo "--disable-tcmalloc      Disable tcmalloc support"
>    echo "--dynamic-libengines    Lib-based ioengines as dynamic libraries"
> @@ -2663,6 +2667,22 @@ if test "$xnvme" != "no" ; then
>  fi
>  print_config "xnvme engine" "$xnvme"
>  
> +##########################################
> +# Check if we have libblkio
> +if test "$libblkio" != "no" ; then
> +  if check_min_lib_version blkio 1.0.0; then
> +    libblkio="yes"
> +    libblkio_cflags=$(pkg-config --cflags blkio)
> +    libblkio_libs=$(pkg-config --libs blkio)
> +  else
> +    if test "$libblkio" = "yes" ; then
> +      feature_not_found "libblkio" "libblkio-dev or libblkio-devel"
> +    fi
> +    libblkio="no"
> +  fi
> +fi
> +print_config "libblkio engine" "$libblkio"
> +
>  ##########################################
>  # check march=armv8-a+crc+crypto
>  if test "$march_armv8_a_crc_crypto" != "yes" ; then
> @@ -3276,6 +3296,11 @@ if test "$xnvme" = "yes" ; then
>    echo "LIBXNVME_CFLAGS=$xnvme_cflags" >> $config_host_mak
>    echo "LIBXNVME_LIBS=$xnvme_libs" >> $config_host_mak
>  fi
> +if test "$libblkio" = "yes" ; then
> +  output_sym "CONFIG_LIBBLKIO"
> +  echo "LIBBLKIO_CFLAGS=$libblkio_cflags" >> $config_host_mak
> +  echo "LIBBLKIO_LIBS=$libblkio_libs" >> $config_host_mak
> +fi
>  if test "$dynamic_engines" = "yes" ; then
>    output_sym "CONFIG_DYNAMIC_ENGINES"
>  fi
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> new file mode 100644
> index 00000000..bd9a5c84
> --- /dev/null
> +++ b/engines/libblkio.c
> @@ -0,0 +1,463 @@
> +/*
> + * libblkio engine
> + *
> + * IO engine using libblkio to access various block I/O interfaces:
> + * https://gitlab.com/libblkio/libblkio
> + */
> +
> +#include <assert.h>
> +#include <errno.h>
> +#include <stdbool.h>
> +#include <stddef.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include <blkio.h>
> +
> +#include "../fio.h"
> +#include "../optgroup.h"
> +#include "../options.h"
> +#include "../parse.h"
> +
> +struct fio_blkio_options {
> +	void *pad; /* option fields must not have offset 0 */
> +
> +	char *driver;
> +	char *pre_connect_props;
> +	char *pre_start_props;
> +};
> +
> +static struct fio_option options[] = {
> +	{
> +		.name	= "libblkio_driver",
> +		.lname	= "libblkio driver name",
> +		.type	= FIO_OPT_STR_STORE,
> +		.off1	= offsetof(struct fio_blkio_options, driver),
> +		.help	= "Name of the driver to be used by libblkio",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
> +	{
> +		.name	= "libblkio_pre_connect_props",
> +		.lname	= "Properties to be set before blkio_connect()",
> +		.type	= FIO_OPT_STR_STORE,
> +		.off1	= offsetof(struct fio_blkio_options, pre_connect_props),
> +		.help	= "",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
> +	{
> +		.name	= "libblkio_pre_start_props",
> +		.lname	= "Properties to be set before blkio_start()",
> +		.type	= FIO_OPT_STR_STORE,
> +		.off1	= offsetof(struct fio_blkio_options, pre_start_props),
> +		.help	= "",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
> +
> +	{
> +		.name = NULL,
> +	},
> +};
> +
> +static int fio_blkio_set_props_from_str(struct blkio *b, const char *opt_name,
> +					const char *str) {
> +	int ret = 0;
> +	char *new_str, *name, *value;
> +
> +	if (!str)
> +		return 0;
> +
> +	/* copy string */
> +
> +	new_str = strdup(str);
> +

No need for the blank lines around the above line.

> +	if (!new_str) {
> +		log_err("fio: strdup() failed\n");
> +		return 1;
> +	}
> +
> +	/* iterate over property name-value pairs */
> +

No need for the blank line here either.

> +	while ((name = get_next_str(&new_str))) {
> +		/* split into property name and value */
> +
> +		value = strchr(name, '=');
> +

Same comment.

> +		if (!value) {
> +			log_err("fio: missing '=' in option %s\n", opt_name);
> +			ret = 1;
> +			break;
> +		}
> +
> +		*value = '\0';
> +		++value;
> +
> +		/* strip whitespace from property name and value */
> +
> +		strip_blank_front(&name);
> +		strip_blank_end(name);
> +

same.

> +		if (name[0] == '\0') {
> +			log_err("fio: empty property name in option %s\n",
> +				opt_name);
> +			ret = 1;
> +			break;
> +		}
> +
> +		strip_blank_front(&value);
> +		strip_blank_end(value);
> +
> +		/* set property */
> +

again here too.

> +		if (blkio_set_str(b, name, value) != 0) {
> +			log_err("fio: error setting property '%s' to '%s': %s\n",
> +				name, value, blkio_get_error_msg());
> +			ret = 1;
> +			break;
> +		}
> +	}
> +
> +	free(new_str);
> +	return ret;
> +}
> +
> +/*
> + * Log the failure of a libblkio function.
> + *
> + * `(void)func` is to ensure `func` exists and prevent typos
> + */
> +#define fio_blkio_log_err(func) \
> +	({ \
> +		(void)func; \
> +		log_err("fio: %s() failed: %s\n", #func, \
> +			blkio_get_error_msg()); \
> +	})
> +
> +static int fio_blkio_create_and_connect(struct thread_data *td,
> +					struct blkio **out_blkio)
> +{
> +	const struct fio_blkio_options *options = td->eo;
> +	struct blkio *b;
> +	int ret;
> +
> +	/* create blkio */
> +

Useless blank line.

> +	if (!options->driver) {
> +		log_err("fio: engine libblkio requires option libblkio_driver to be set\n");
> +		return 1;
> +	}
> +
> +	if (blkio_create(options->driver, &b) != 0) {
> +		fio_blkio_log_err(blkio_create);
> +		return 1;
> +	}
> +
> +	/* set pre-connect properties */
> +
> +	/* don't fail if driver doesn't have a "direct" property */

Use multi-line comment ?

> +	ret = blkio_set_bool(b, "direct", td->o.odirect);
> +	if (ret != 0 && ret != -ENOENT) {
> +		fio_blkio_log_err(blkio_set_bool);
> +		goto err_blkio_destroy;
> +	}
> +
> +	if (blkio_set_bool(b, "read-only", read_only) != 0) {
> +		fio_blkio_log_err(blkio_set_bool);
> +		goto err_blkio_destroy;
> +	}
> +
> +	if (fio_blkio_set_props_from_str(b, "libblkio_pre_connect_props",
> +					 options->pre_connect_props) != 0)
> +		goto err_blkio_destroy;
> +
> +	/* connect blkio */
> +

No need for this blnak line either I think... Stopping here about blank
lines. Matter of taste I guess, but I think you have too many of them,
making the code somewhat unpleasant to read.

> +	if (blkio_connect(b) != 0) {
> +		fio_blkio_log_err(blkio_connect);
> +		goto err_blkio_destroy;
> +	}
> +
> +	/* set pre-start properties */
> +
> +	if (fio_blkio_set_props_from_str(b, "libblkio_pre_start_props",
> +					 options->pre_start_props) != 0)
> +		goto err_blkio_destroy;
> +
> +	*out_blkio = b;
> +	return 0;
> +
> +err_blkio_destroy:
> +	blkio_destroy(&b);
> +	return 1;
> +}
> +
> +static int fio_blkio_setup(struct thread_data *td)
> +{
> +	/*
> +	 * We have to determine device/file size here, so we create and connect
> +	 * a blkio instance. But this callback is called from the main thread in
> +	 * the original fio process, not from the processes in which jobs will
> +	 * actually run. We thus subsequently destroy the blkio and create it
> +	 * again in the init() callback.
> +	 */

Unusual place for a comment. Move this above the function declaration to
explain what the function does.

> +
> +	struct blkio *b;
> +	int ret = 0;
> +	uint64_t capacity;
> +
> +	assert(td->files_index == 1);
> +
> +	/* get target size */
> +
> +	if (fio_blkio_create_and_connect(td, &b) != 0)
> +		return 1;
> +
> +	if (blkio_get_uint64(b, "capacity", &capacity) != 0) {
> +		fio_blkio_log_err(blkio_get_uint64);
> +		ret = 1;
> +		goto out_blkio_destroy;
> +	}
> +
> +	/* set file size for fio */
> +
> +	td->files[0]->real_file_size = capacity;
> +	fio_file_set_size_known(td->files[0]);
> +
> +out_blkio_destroy:
> +	blkio_destroy(&b);
> +	return ret;
> +}
> +
> +/* per-thread state */
> +struct fio_blkio_data {
> +	struct blkio *b;
> +	struct blkioq *q;
> +
> +	bool has_mem_region; /* whether mem_region is valid */
> +	struct blkio_mem_region mem_region;
> +
> +	struct blkio_completion *completions;
> +};

Why not have this declaration at the beginning of the file ?

> +
> +static int fio_blkio_init(struct thread_data *td)
> +{
> +	struct fio_blkio_data *data;
> +
> +	/* allocate per-thread data struct */
> +
> +	data = calloc(1, sizeof(*data));
> +	if (!data) {
> +		log_err("fio: calloc() failed\n");
> +		goto err_free;

This will seg-fault as err_free first does "free(data->completions)".
Return here.

> +	}
> +
> +	data->completions = calloc(td->o.iodepth, sizeof(data->completions[0]));
> +	if (!data->completions) {
> +		log_err("fio: calloc() failed\n");
> +		goto err_free;
> +	}
> +
> +	/* create, connect, and start blkio */
> +
> +	if (fio_blkio_create_and_connect(td, &data->b) != 0)
> +		goto err_free;
> +
> +	if (blkio_set_int(data->b, "num-queues", 1) != 0) {
> +		fio_blkio_log_err(blkio_set_int);
> +		goto err_blkio_destroy;
> +	}
> +
> +	if (blkio_start(data->b) != 0) {
> +		fio_blkio_log_err(blkio_start);
> +		goto err_blkio_destroy;
> +	}
> +
> +	/* get queue */

Useless comment.

> +
> +	data->q = blkio_get_queue(data->b, 0);
> +
> +	/* Set data only here so cleanup() does nothing if init() fails. */
> +	td->io_ops_data = data;
> +
> +	return 0;
> +
> +err_blkio_destroy:
> +	blkio_destroy(&data->b);
> +err_free:
> +	free(data->completions);
> +	free(data);
> +	return 1;
> +}
> +
> +static void fio_blkio_cleanup(struct thread_data *td)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +
> +	if (data) {
> +		blkio_destroy(&data->b);
> +		free(data->completions);
> +		free(data);
> +	}
> +}
> +
> +#define align_up(x, y) ((((x) + (y) - 1) / (y)) * (y))
> +
> +static int fio_blkio_iomem_alloc(struct thread_data *td, size_t size)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +	int ret;
> +	uint64_t mem_region_alignment;
> +	size_t aligned_size;
> +
> +	/* round up size to satisfy mem-region-alignment */
> +
> +	if (blkio_get_uint64(data->b, "mem-region-alignment",
> +			     &mem_region_alignment) != 0) {
> +		fio_blkio_log_err(blkio_get_uint64);
> +		return 1;
> +	}
> +
> +	aligned_size = align_up(size, (size_t)mem_region_alignment);
> +
> +	/* allocate memory region */
> +
> +	if (blkio_alloc_mem_region(data->b, &data->mem_region,
> +				   aligned_size) != 0) {
> +		fio_blkio_log_err(blkio_alloc_mem_region);
> +		ret = 1;
> +		goto out;
> +	}
> +
> +	if (blkio_map_mem_region(data->b, &data->mem_region) != 0) {
> +		fio_blkio_log_err(blkio_map_mem_region);
> +		ret = 1;
> +		goto out_free;
> +	}
> +
> +	td->orig_buffer = data->mem_region.addr;
> +	data->has_mem_region = true;
> +
> +	ret = 0;
> +	goto out;
> +
> +out_free:
> +	blkio_free_mem_region(data->b, &data->mem_region);
> +out:
> +	return ret;
> +}
> +
> +static void fio_blkio_iomem_free(struct thread_data *td)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +
> +	if (data && data->has_mem_region) {
> +		blkio_unmap_mem_region(data->b, &data->mem_region);
> +		blkio_free_mem_region(data->b, &data->mem_region);
> +
> +		data->has_mem_region = false;
> +	}
> +}
> +
> +static int fio_blkio_open_file(struct thread_data *td, struct fio_file *f)
> +{
> +	return 0;
> +}
> +
> +static enum fio_q_status fio_blkio_queue(struct thread_data *td,
> +					 struct io_u *io_u)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +
> +	fio_ro_check(td, io_u);
> +
> +	switch (io_u->ddir) {
> +		case DDIR_READ:
> +			blkioq_read(data->q, io_u->offset, io_u->xfer_buf,
> +				    (size_t)io_u->xfer_buflen, io_u, 0);
> +			break;
> +
> +		case DDIR_WRITE:
> +			blkioq_write(data->q, io_u->offset, io_u->xfer_buf,
> +				     (size_t)io_u->xfer_buflen, io_u, 0);
> +			break;
> +
> +		case DDIR_TRIM:
> +			blkioq_discard(data->q, io_u->offset, io_u->xfer_buflen,
> +				       io_u, 0);
> +		        break;
> +
> +		case DDIR_SYNC:
> +		case DDIR_DATASYNC:
> +			blkioq_flush(data->q, io_u, 0);
> +			break;
> +
> +		default:
> +			io_u->error = ENOTSUP;
> +			io_u_log_error(td, io_u);
> +			return FIO_Q_COMPLETED;
> +	}
> +
> +	return FIO_Q_QUEUED;
> +}
> +
> +static int fio_blkio_getevents(struct thread_data *td, unsigned int min,
> +			       unsigned int max, const struct timespec *t)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +	int n;
> +
> +	n = blkioq_do_io(data->q, data->completions, (int)min, (int)max, NULL);
> +	if (n < 0) {
> +		fio_blkio_log_err(blkioq_do_io);
> +		return -1;
> +	}
> +
> +	return n;
> +}
> +
> +static struct io_u *fio_blkio_event(struct thread_data *td, int event)
> +{
> +	struct fio_blkio_data *data = td->io_ops_data;
> +	struct blkio_completion *completion = &data->completions[event];
> +	struct io_u *io_u = completion->user_data;
> +
> +	io_u->error = -completion->ret;
> +
> +	return io_u;
> +}
> +
> +FIO_STATIC struct ioengine_ops ioengine = {
> +	.name			= "libblkio",
> +	.version		= FIO_IOOPS_VERSION,
> +	.flags			= FIO_DISKLESSIO | FIO_NOEXTEND |
> +				  FIO_NO_OFFLOAD,
> +
> +	.setup			= fio_blkio_setup,
> +	.init			= fio_blkio_init,
> +	.cleanup		= fio_blkio_cleanup,
> +
> +	.iomem_alloc		= fio_blkio_iomem_alloc,
> +	.iomem_free		= fio_blkio_iomem_free,
> +
> +	.open_file		= fio_blkio_open_file,
> +
> +	.queue			= fio_blkio_queue,
> +	.getevents		= fio_blkio_getevents,
> +	.event			= fio_blkio_event,
> +
> +	.options		= options,
> +	.option_struct_size	= sizeof(struct fio_blkio_options),
> +};
> +
> +static void fio_init fio_blkio_register(void)
> +{
> +	register_ioengine(&ioengine);
> +}
> +
> +static void fio_exit fio_blkio_unregister(void)
> +{
> +	unregister_ioengine(&ioengine);
> +}
> diff --git a/examples/libblkio-io_uring.fio b/examples/libblkio-io_uring.fio
> new file mode 100644
> index 00000000..d2700e51
> --- /dev/null
> +++ b/examples/libblkio-io_uring.fio
> @@ -0,0 +1,19 @@
> +; Benchmark accessing a regular file or block device using libblkio.
> +;
> +; Replace "libblkio_path" below with the path to your file or device, or
> +; override it by passing the '--libblkio_pre_connect_props=path=...' flag to
> +; fio.
> +;
> +; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
> +
> +[global]
> +ioengine=libblkio
> +libblkio_driver=io_uring
> +libblkio_pre_connect_props=path=/dev/nvme0n1  ; REPLACE THIS WITH THE RIGHT PATH
> +rw=randread
> +blocksize=4k
> +direct=1
> +time_based=1
> +runtime=10s
> +
> +[job]
> diff --git a/examples/libblkio-virtio-blk-vfio-pci.fio b/examples/libblkio-virtio-blk-vfio-pci.fio
> new file mode 100644
> index 00000000..79d6228d
> --- /dev/null
> +++ b/examples/libblkio-virtio-blk-vfio-pci.fio
> @@ -0,0 +1,18 @@
> +; Benchmark accessing a PCI virtio-blk device using libblkio.
> +;
> +; Replace "libblkio_path" below with the path to your device's sysfs directory,
> +; or override it by passing the '--libblkio_pre_connect_props=path=...' flag to
> +; fio. Note that colons in the path must be escaped with a backslash.
> +;
> +; For information on libblkio, see: https://gitlab.com/libblkio/libblkio
> +
> +[global]
> +ioengine=libblkio
> +libblkio_driver=virtio-blk-vfio-pci
> +libblkio_pre_connect_props=path=/sys/bus/pci/devices/0000\:00\:01.0  ; REPLACE THIS WITH THE RIGHT PATH
> +rw=randread
> +blocksize=4k
> +time_based=1
> +runtime=10s
> +
> +[job]
> diff --git a/fio.1 b/fio.1
> index 9e33c9e1..64a774f5 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -1989,6 +1989,10 @@ I/O engine using the xNVMe C API, for NVMe devices. The xnvme engine provides
>  flexibility to access GNU/Linux Kernel NVMe driver via libaio, IOCTLs, io_uring,
>  the SPDK NVMe driver, or your own custom NVMe driver. The xnvme engine includes
>  engine specific options. (See \fIhttps://xnvme.io/\fR).
> +.TP
> +.B libblkio
> +Use the libblkio library (\fIhttps://gitlab.com/libblkio/libblkio\fR). The
> +specific \fBdriver\fR to use must be set using \fBlibblkio_driver\fR.
>  .SS "I/O engine specific parameters"
>  In addition, there are some parameters which are only valid when a specific
>  \fBioengine\fR is in use. These are used identically to normal parameters,
> @@ -2599,6 +2603,21 @@ xnvme namespace identifier for userspace NVMe driver such as SPDK.
>  .TP
>  .BI (xnvme)xnvme_iovec
>  If this option is set, xnvme will use vectored read/write commands.
> +.TP
> +.BI (libblkio)libblkio_driver \fR=\fPstr
> +The driver to be used by libblkio (e.g. \fBvirtio-blk-vfio-pci\fR).
> +.TP
> +.BI (libblkio)libblkio_pre_connect_props \fR=\fPstr
> +A colon-separated list of libblkio properties to be set after creating but
> +before connecting the \fBstruct blkio\fR. Each property must have the format
> +\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
> +the engine sets any other properties, so those can be overriden.
> +.TP
> +.BI (libblkio)libblkio_pre_start_props \fR=\fPstr
> +A colon-separated list of libblkio properties to be set after connecting but
> +before starting the \fBstruct blkio\fR. Each property must have the format
> +\fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
> +the engine sets any other properties, so those can be overriden.
>  .SS "I/O depth"
>  .TP
>  .BI iodepth \fR=\fPint
> diff --git a/optgroup.h b/optgroup.h
> index dc73c8f3..024b902f 100644
> --- a/optgroup.h
> +++ b/optgroup.h
> @@ -73,6 +73,7 @@ enum opt_category_group {
>  	__FIO_OPT_G_NFS,
>  	__FIO_OPT_G_WINDOWSAIO,
>  	__FIO_OPT_G_XNVME,
> +	__FIO_OPT_G_LIBBLKIO,
>  
>  	FIO_OPT_G_RATE		= (1ULL << __FIO_OPT_G_RATE),
>  	FIO_OPT_G_ZONE		= (1ULL << __FIO_OPT_G_ZONE),
> @@ -120,6 +121,7 @@ enum opt_category_group {
>  	FIO_OPT_G_DFS		= (1ULL << __FIO_OPT_G_DFS),
>  	FIO_OPT_G_WINDOWSAIO	= (1ULL << __FIO_OPT_G_WINDOWSAIO),
>  	FIO_OPT_G_XNVME         = (1ULL << __FIO_OPT_G_XNVME),
> +	FIO_OPT_G_LIBBLKIO	= (1ULL << __FIO_OPT_G_LIBBLKIO),
>  };
>  
>  extern const struct opt_group *opt_group_from_mask(uint64_t *mask);

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 04/10] engines/libblkio: Add support for poll queues
  2022-11-21 18:28 ` [PATCH 04/10] engines/libblkio: Add support for poll queues Alberto Faria
@ 2022-12-01 17:01   ` Vincent Fu
  0 siblings, 0 replies; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:01 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/21/22 13:28, Alberto Faria wrote:
> Configure a poll queue instead of a "regular" queue when option hipri is
> set.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst          |  4 ++++
>   engines/libblkio.c | 26 ++++++++++++++++++++++++--
>   fio.1              |  3 +++
>   3 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index fb27d3d3..cdea3258 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2330,6 +2330,10 @@ with the caveat that when used on the command line, they must come after the
>           by the application. The benefits are more efficient IO for high IOPS
>           scenarios, and lower latencies for low queue depth IO.
>   
> +   [libblkio]
> +
> +	Use poll queues.
> +
>      [pvsync2]
>   
>   	Set RWF_HIPRI on I/O, indicating to the kernel that it's of higher priority
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index 54fd9ff6..d2ade3f1 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -26,6 +26,8 @@ struct fio_blkio_options {
>   	char *driver;
>   	char *pre_connect_props;
>   	char *pre_start_props;
> +
> +	unsigned int hipri;
>   };
>   
>   static struct fio_option options[] = {
> @@ -57,6 +59,16 @@ static struct fio_option options[] = {
>   		.group	= FIO_OPT_G_LIBBLKIO,
>   	},
>   
> +	{
> +		.name	= "hipri",
> +		.lname	= "Use poll queues",
> +		.type	= FIO_OPT_STR_SET,
> +		.off1	= offsetof(struct fio_blkio_options, hipri),
> +		.help	= "Use poll queues",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
> +

Some of these elements have blank lines before or after the { } and 
others do not. For consistency I would eliminate the blank lines before 
and after the { and }.

>   	{
>   		.name = NULL,
>   	},
> @@ -244,6 +256,7 @@ struct fio_blkio_data {
>   
>   static int fio_blkio_init(struct thread_data *td)
>   {
> +	const struct fio_blkio_options *options = td->eo;
>   	struct fio_blkio_data *data;
>   
>   	/* allocate per-thread data struct */
> @@ -265,7 +278,13 @@ static int fio_blkio_init(struct thread_data *td)
>   	if (fio_blkio_create_and_connect(td, &data->b) != 0)
>   		goto err_free;
>   
> -	if (blkio_set_int(data->b, "num-queues", 1) != 0) {
> +	if (blkio_set_int(data->b, "num-queues", options->hipri ? 0 : 1) != 0) {
> +		fio_blkio_log_err(blkio_set_int);
> +		goto err_blkio_destroy;
> +	}
> +
> +	if (blkio_set_int(data->b, "num-poll-queues",
> +			  options->hipri ? 1 : 0) != 0) {
>   		fio_blkio_log_err(blkio_set_int);
>   		goto err_blkio_destroy;
>   	}
> @@ -277,7 +296,10 @@ static int fio_blkio_init(struct thread_data *td)
>   
>   	/* get queue */
>   
> -	data->q = blkio_get_queue(data->b, 0);
> +	if (options->hipri)
> +		data->q = blkio_get_poll_queue(data->b, 0);
> +	else
> +		data->q = blkio_get_queue(data->b, 0);
>   
>   	/* Set data only here so cleanup() does nothing if init() fails. */
>   	td->io_ops_data = data;
> diff --git a/fio.1 b/fio.1
> index 12eeb013..7c1b315a 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2620,6 +2620,9 @@ A colon-separated list of libblkio properties to be set after connecting but
>   before starting the \fBstruct blkio\fR. Each property must have the format
>   \fB<name>=<value>\fR. Colons can be escaped as \fB\\:\fR. These are set after
>   the engine sets any other properties, so those can be overriden.
> +.TP
> +.BI (libblkio)hipri \fR=\fPbool

The type for hipri is FIO_OPT_STR_SET which is different from a boolean. 
If it were a boolean fio would accept only hipri=0 or hipri=1. For 
FIO_OPT_STR_SET fio accepts hipri=<any number>, or just hipri with no 
value (same meaning as hipri=1), with 0 representing false.

For FIO_OPT_STR_SET options (e.g., thread, time_based) the convention in 
the man page is to not specify a type.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 05/10] engines/libblkio: Add option libblkio_vectored
  2022-11-21 18:28 ` [PATCH 05/10] engines/libblkio: Add option libblkio_vectored Alberto Faria
@ 2022-12-01 17:11   ` Vincent Fu
  2022-12-01 22:13     ` Alberto Faria
  0 siblings, 1 reply; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:11 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/21/22 13:28, Alberto Faria wrote:
> When enabled, read and write requests are submitted as vectored requests
> using blkioq_{readv,writev}(), instead of using blkioq_{read,write}().
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst          |  4 ++++
>   engines/libblkio.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
>   fio.1              |  3 +++
>   3 files changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index cdea3258..b9c7c8df 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2875,6 +2875,10 @@ with the caveat that when used on the command line, they must come after the
>   	set after the engine sets any other properties, so those can be
>   	overriden.
>   
> +.. option:: libblkio_vectored=bool : [libblkio]

See my response to the hipri patch regarding the difference between 
booleans and FIO_OPT_STR_SET. I don't have strong feelings regarding 
whether this should be a boolean or FIO_OPT_STR_SET but if the type 
listed in the HOWTO and manpage is boolean then the type in the options 
struct should be FIO_OPT_BOOL. No type should be listed when it's 
FIO_OPT_STR_SET.

> +
> +	Submit vectored read and write requests. Default is 0.
> +
>   I/O depth
>   ~~~~~~~~~
>   
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index d2ade3f1..dcf701ad 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -28,6 +28,7 @@ struct fio_blkio_options {
>   	char *pre_start_props;
>   
>   	unsigned int hipri;
> +	unsigned int vectored;
>   };
>   
>   static struct fio_option options[] = {
> @@ -68,6 +69,15 @@ static struct fio_option options[] = {
>   		.category = FIO_OPT_C_ENGINE,
>   		.group	= FIO_OPT_G_LIBBLKIO,
>   	},
> +	{
> +		.name	= "libblkio_vectored",
> +		.lname	= "Use blkioq_{readv,writev}()",
> +		.type	= FIO_OPT_STR_SET,
> +		.off1	= offsetof(struct fio_blkio_options, vectored),
> +		.help	= "Use blkioq_{readv,writev}() instead of blkioq_{read,write}()",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
>   
>   	{
>   		.name = NULL,
> @@ -251,6 +261,7 @@ struct fio_blkio_data {
>   	bool has_mem_region; /* whether mem_region is valid */
>   	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
>   
> +	struct iovec *iovecs; /* for vectored requests */
>   	struct blkio_completion *completions;
>   };
>   
> @@ -267,8 +278,9 @@ static int fio_blkio_init(struct thread_data *td)
>   		goto err_free;
>   	}
>   
> +	data->iovecs = calloc(td->o.iodepth, sizeof(data->iovecs[0]));
>   	data->completions = calloc(td->o.iodepth, sizeof(data->completions[0]));
> -	if (!data->completions) {
> +	if (!data->iovecs || !data->completions) {
>   		log_err("fio: calloc() failed\n");
>   		goto err_free;
>   	}
> @@ -310,6 +322,7 @@ err_blkio_destroy:
>   	blkio_destroy(&data->b);
>   err_free:
>   	free(data->completions);
> +	free(data->iovecs);
>   	free(data);
>   	return 1;
>   }
> @@ -362,6 +375,7 @@ static void fio_blkio_cleanup(struct thread_data *td)
>   	if (data) {
>   		blkio_destroy(&data->b);
>   		free(data->completions);
> +		free(data->iovecs);
>   		free(data);
>   	}
>   }
> @@ -432,19 +446,41 @@ static int fio_blkio_open_file(struct thread_data *td, struct fio_file *f)
>   static enum fio_q_status fio_blkio_queue(struct thread_data *td,
>   					 struct io_u *io_u)
>   {
> +	const struct fio_blkio_options *options = td->eo;
>   	struct fio_blkio_data *data = td->io_ops_data;
>   
>   	fio_ro_check(td, io_u);
>   
>   	switch (io_u->ddir) {
>   		case DDIR_READ:
> -			blkioq_read(data->q, io_u->offset, io_u->xfer_buf,
> -				    (size_t)io_u->xfer_buflen, io_u, 0);
> +			if (options->vectored) {
> +				struct iovec *iov = &data->iovecs[io_u->index];
> +				iov->iov_base = io_u->xfer_buf;
> +				iov->iov_len = (size_t)io_u->xfer_buflen;
> +
> +				blkioq_readv(data->q, io_u->offset, iov, 1,
> +					     io_u, 0);
> +			} else {
> +				blkioq_read(data->q, io_u->offset,
> +					    io_u->xfer_buf,
> +					    (size_t)io_u->xfer_buflen, io_u, 0);
> +			}
>   			break;
>   
>   		case DDIR_WRITE:
> -			blkioq_write(data->q, io_u->offset, io_u->xfer_buf,
> -				     (size_t)io_u->xfer_buflen, io_u, 0);
> +			if (options->vectored) {
> +				struct iovec *iov = &data->iovecs[io_u->index];
> +				iov->iov_base = io_u->xfer_buf;
> +				iov->iov_len = (size_t)io_u->xfer_buflen;
> +
> +				blkioq_writev(data->q, io_u->offset, iov, 1,
> +					      io_u, 0);
> +			} else {
> +				blkioq_write(data->q, io_u->offset,
> +					     io_u->xfer_buf,
> +					     (size_t)io_u->xfer_buflen, io_u,
> +					     0);
> +			}
>   			break;
>   
>   		case DDIR_TRIM:
> diff --git a/fio.1 b/fio.1
> index 7c1b315a..a403b415 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2623,6 +2623,9 @@ the engine sets any other properties, so those can be overriden.
>   .TP
>   .BI (libblkio)hipri \fR=\fPbool
>   Use poll queues.
> +.TP
> +.BI (libblkio)libblkio_vectored \fR=\fPbool
> +Submit vectored read and write requests. Default is 0.
>   .SS "I/O depth"
>   .TP
>   .BI iodepth \fR=\fPint


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim
  2022-11-21 18:28 ` [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim Alberto Faria
@ 2022-12-01 17:13   ` Vincent Fu
  0 siblings, 0 replies; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:13 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/21/22 13:28, Alberto Faria wrote:
> When set, trim IOs will be submitted as blkioq_write_zeroes() requests
> instead of blkioq_discard() requests.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst          |  5 +++++
>   engines/libblkio.c | 20 ++++++++++++++++++--
>   fio.1              |  4 ++++
>   3 files changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index b9c7c8df..7155add6 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2879,6 +2879,11 @@ with the caveat that when used on the command line, they must come after the
>   
>   	Submit vectored read and write requests. Default is 0.
>   
> +.. option:: libblkio_write_zeroes_on_trim=bool : [libblkio]

Same comment here as for libblkio_vectored regarding option types.

> +
> +	Submit trims as "write zeroes" requests instead of discard requests.
> +	Default is 0.
> +
>   I/O depth
>   ~~~~~~~~~
>   
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index dcf701ad..79af3aa7 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -29,6 +29,7 @@ struct fio_blkio_options {
>   
>   	unsigned int hipri;
>   	unsigned int vectored;
> +	unsigned int write_zeroes_on_trim;
>   };
>   
>   static struct fio_option options[] = {
> @@ -78,6 +79,16 @@ static struct fio_option options[] = {
>   		.category = FIO_OPT_C_ENGINE,
>   		.group	= FIO_OPT_G_LIBBLKIO,
>   	},
> +	{
> +		.name	= "libblkio_write_zeroes_on_trim",
> +		.lname	= "Use blkioq_write_zeroes() for TRIM",
> +		.type	= FIO_OPT_STR_SET,
> +		.off1	= offsetof(struct fio_blkio_options,
> +				   write_zeroes_on_trim),
> +		.help	= "Use blkioq_write_zeroes() for TRIM instead of blkioq_discard()",
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
>   
>   	{
>   		.name = NULL,
> @@ -484,8 +495,13 @@ static enum fio_q_status fio_blkio_queue(struct thread_data *td,
>   			break;
>   
>   		case DDIR_TRIM:
> -			blkioq_discard(data->q, io_u->offset, io_u->xfer_buflen,
> -				       io_u, 0);
> +			if (options->write_zeroes_on_trim) {
> +				blkioq_write_zeroes(data->q, io_u->offset,
> +						    io_u->xfer_buflen, io_u, 0);
> +			} else {
> +				blkioq_discard(data->q, io_u->offset,
> +					       io_u->xfer_buflen, io_u, 0);
> +			}
>   		        break;
>   
>   		case DDIR_SYNC:
> diff --git a/fio.1 b/fio.1
> index a403b415..053c2eda 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2626,6 +2626,10 @@ Use poll queues.
>   .TP
>   .BI (libblkio)libblkio_vectored \fR=\fPbool
>   Submit vectored read and write requests. Default is 0.
> +.TP
> +.BI (libblkio)libblkio_write_zeroes_on_trim \fR=\fPbool
> +Submit trims as "write zeroes" requests instead of discard requests. Default is
> +0.
>   .SS "I/O depth"
>   .TP
>   .BI iodepth \fR=\fPint


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode
  2022-11-21 18:28 ` [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode Alberto Faria
@ 2022-12-01 17:21   ` Vincent Fu
  0 siblings, 0 replies; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:21 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/21/22 13:28, Alberto Faria wrote:
> It allows configuring how the engine waits for request completions,
> instead of always using a blocking blkioq_do_io() call.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst          |  14 ++++-
>   engines/libblkio.c | 132 ++++++++++++++++++++++++++++++++++++++++++---
>   fio.1              |  16 +++++-
>   3 files changed, 154 insertions(+), 8 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 7155add6..4b059cb2 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2332,7 +2332,8 @@ with the caveat that when used on the command line, they must come after the
>   
>      [libblkio]
>   
> -	Use poll queues.
> +	Use poll queues. This is incompatible with
> +	:option:`libblkio_wait_mode=eventfd <libblkio_wait_mode>`.
>   
>      [pvsync2]
>   
> @@ -2884,6 +2885,17 @@ with the caveat that when used on the command line, they must come after the
>   	Submit trims as "write zeroes" requests instead of discard requests.
>   	Default is 0.
>   
> +.. option:: libblkio_wait_mode=str : [libblkio]
> +
> +	How to wait for completions:
> +
> +	**block** (default)
> +		Use a blocking call to ``blkioq_do_io()``.
> +	**eventfd**
> +		Use a blocking call to ``read()`` on the completion eventfd.
> +	**loop**
> +		Use a busy loop with a non-blocking call to ``blkioq_do_io()``.
> +
>   I/O depth
>   ~~~~~~~~~
>   
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index 79af3aa7..d7666f69 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -20,6 +20,12 @@
>   #include "../options.h"
>   #include "../parse.h"
>   
> +enum fio_blkio_wait_mode {
> +	FIO_BLKIO_WAIT_MODE_BLOCK,
> +	FIO_BLKIO_WAIT_MODE_EVENTFD,
> +	FIO_BLKIO_WAIT_MODE_LOOP,
> +};
> +
>   struct fio_blkio_options {
>   	void *pad; /* option fields must not have offset 0 */
>   
> @@ -30,6 +36,7 @@ struct fio_blkio_options {
>   	unsigned int hipri;
>   	unsigned int vectored;
>   	unsigned int write_zeroes_on_trim;
> +	enum fio_blkio_wait_mode wait_mode;
>   };
>   
>   static struct fio_option options[] = {
> @@ -89,6 +96,30 @@ static struct fio_option options[] = {
>   		.category = FIO_OPT_C_ENGINE,
>   		.group	= FIO_OPT_G_LIBBLKIO,
>   	},
> +	{
> +		.name	= "libblkio_wait_mode",
> +		.lname	= "How to wait for completions",
> +		.type	= FIO_OPT_STR,
> +		.off1	= offsetof(struct fio_blkio_options, wait_mode),
> +		.help	= "How to wait for completions",
> +		.def	= "block",
> +		.posval = {
> +			  { .ival = "block",
> +			    .oval = FIO_BLKIO_WAIT_MODE_BLOCK,
> +			    .help = "Blocking blkioq_do_io()",
> +			  },
> +			  { .ival = "eventfd",
> +			    .oval = FIO_BLKIO_WAIT_MODE_EVENTFD,
> +			    .help = "Blocking read() on the completion eventfd",
> +			  },
> +			  { .ival = "loop",
> +			    .oval = FIO_BLKIO_WAIT_MODE_LOOP,
> +			    .help = "Busy loop with non-blocking blkioq_do_io()",
> +			  },
> +		},
> +		.category = FIO_OPT_C_ENGINE,
> +		.group	= FIO_OPT_G_LIBBLKIO,
> +	},
>   
>   	{
>   		.name = NULL,
> @@ -237,12 +268,21 @@ static int fio_blkio_setup(struct thread_data *td)
>   	 * again in the init() callback.
>   	 */
>   
> +	const struct fio_blkio_options *options = td->eo;
>   	struct blkio *b;
>   	int ret = 0;
>   	uint64_t capacity;
>   
>   	assert(td->files_index == 1);
>   
> +	/* validate options */
> +
> +	if (options->hipri &&
> +		options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
> +		log_err("fio: option hipri is incompatible with option libblkio_wait_mode=eventfd\n");
> +		return 1;
> +	}
> +
>   	/* get target size */
>   
>   	if (fio_blkio_create_and_connect(td, &b) != 0)
> @@ -268,6 +308,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 */
>   
>   	bool has_mem_region; /* whether mem_region is valid */
>   	struct blkio_mem_region mem_region; /* only if allocated by libblkio */
> @@ -280,6 +321,7 @@ static int fio_blkio_init(struct thread_data *td)
>   {
>   	const struct fio_blkio_options *options = td->eo;
>   	struct fio_blkio_data *data;
> +	int flags;
>   
>   	/* allocate per-thread data struct */
>   
> @@ -324,6 +366,31 @@ static int fio_blkio_init(struct thread_data *td)
>   	else
>   		data->q = blkio_get_queue(data->b, 0);
>   
> +	/* enable completion fd and make it blocking */
> +
> +	if (options->wait_mode == FIO_BLKIO_WAIT_MODE_EVENTFD) {
> +		blkioq_set_completion_fd_enabled(data->q, true);
> +
> +		data->completion_fd = blkioq_get_completion_fd(data->q);
> +
> +		flags = fcntl(data->completion_fd, F_GETFL);
> +		if (flags < 0) {
> +			log_err("fio: fcntl(F_GETFL) failed: %s\n",
> +				strerror(errno));
> +			goto err_blkio_destroy;
> +		}
> +
> +		flags &= ~O_NONBLOCK;
> +
> +		if (fcntl(data->completion_fd, F_SETFL, flags) != 0) {
> +			log_err("fio: fcntl(F_SETFL) failed: %s\n",
> +				strerror(errno));
> +			goto err_blkio_destroy;
> +		}
> +	} else {
> +		data->completion_fd = -1;
> +	}
> +
>   	/* Set data only here so cleanup() does nothing if init() fails. */
>   	td->io_ops_data = data;
>   
> @@ -521,16 +588,69 @@ static enum fio_q_status fio_blkio_queue(struct thread_data *td,
>   static int fio_blkio_getevents(struct thread_data *td, unsigned int min,
>   			       unsigned int max, const struct timespec *t)
>   {
> +	const struct fio_blkio_options *options = td->eo;
>   	struct fio_blkio_data *data = td->io_ops_data;
> -	int n;
> +	int ret, n;
> +	uint64_t event;
>   
> -	n = blkioq_do_io(data->q, data->completions, (int)min, (int)max, NULL);
> -	if (n < 0) {
> -		fio_blkio_log_err(blkioq_do_io);
> +	switch (options->wait_mode) {
> +	case FIO_BLKIO_WAIT_MODE_BLOCK:
> +

Because we already have indentation here the blank lines before and 
after each "case" don't improve readability much. To be consistent with 
the rest of the code base and conserve vertical screen real estate I 
would drop the blank lines.

> +		n = blkioq_do_io(data->q, data->completions, (int)min, (int)max,
> +				 NULL);
> +		if (n < 0) {
> +			fio_blkio_log_err(blkioq_do_io);
> +			return -1;
> +		}
> +
> +		return n;
> +
> +	case FIO_BLKIO_WAIT_MODE_EVENTFD:
> +
> +		n = blkioq_do_io(data->q, data->completions, 0, (int)max, NULL);
> +		if (n < 0) {
> +			fio_blkio_log_err(blkioq_do_io);
> +			return -1;
> +		}
> +
> +		while (n < (int)min) {
> +			ret = read(data->completion_fd, &event, sizeof(event));
> +			if (ret != sizeof(event)) {
> +				log_err("fio: read() on the completion fd returned %d\n",
> +					ret);
> +				return -1;
> +			}
> +
> +			ret = blkioq_do_io(data->q, data->completions + n, 0,
> +					   (int)max - n, NULL);
> +			if (ret < 0) {
> +				fio_blkio_log_err(blkioq_do_io);
> +				return -1;
> +			}
> +
> +			n += ret;
> +		}
> +
> +		return n;
> +
> +	case FIO_BLKIO_WAIT_MODE_LOOP:
> +
> +		for (n = 0; n < (int)min; ) {
> +			ret = blkioq_do_io(data->q, data->completions + n, 0,
> +					   (int)max - n, NULL);
> +			if (ret < 0) {
> +				fio_blkio_log_err(blkioq_do_io);
> +				return -1;
> +			}
> +
> +			n += ret;
> +		}
> +
> +		return n;
> +
> +	default:
>   		return -1;
>   	}
> -
> -	return n;
>   }
>   
>   static struct io_u *fio_blkio_event(struct thread_data *td, int event)
> diff --git a/fio.1 b/fio.1
> index 053c2eda..88bb232f 100644
> --- a/fio.1
> +++ b/fio.1
> @@ -2622,7 +2622,7 @@ 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.
> +Use poll queues. This is incompatible with \fBlibblkio_wait_mode=eventfd\fR.
>   .TP
>   .BI (libblkio)libblkio_vectored \fR=\fPbool
>   Submit vectored read and write requests. Default is 0.
> @@ -2630,6 +2630,20 @@ Submit vectored read and write requests. Default is 0.
>   .BI (libblkio)libblkio_write_zeroes_on_trim \fR=\fPbool
>   Submit trims as "write zeroes" requests instead of discard requests. Default is
>   0.
> +.TP
> +.BI (libblkio)libblkio_wait_mode \fR=\fPstr
> +How to wait for completions:
> +.RS
> +.RS
> +.TP
> +.B block \fR(default)
> +Use a blocking call to \fBblkioq_do_io()\fR.
> +.TP
> +.B eventfd
> +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.
>   .SS "I/O depth"
>   .TP
>   .BI iodepth \fR=\fPint


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd
  2022-11-21 18:29 ` [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd Alberto Faria
@ 2022-12-01 17:23   ` Vincent Fu
  0 siblings, 0 replies; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:23 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process
  2022-11-21 18:29 ` [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process Alberto Faria
@ 2022-12-01 17:29   ` Vincent Fu
  0 siblings, 0 replies; 20+ messages in thread
From: Vincent Fu @ 2022-12-01 17:29 UTC (permalink / raw)
  To: Alberto Faria, fio; +Cc: Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On 11/21/22 13:29, Alberto Faria wrote:
> fio groups all subjobs that set option 'thread' into a single process.
> Have them all share a single `struct blkio` instance, with one `struct
> blkioq` per thread/subjob. This allows benchmarking multi-queue setups.
> 
> Note that `struct blkio` instances cannot be shared across different
> processes.
> 
> Signed-off-by: Alberto Faria <afaria@redhat.com>
> ---
>   HOWTO.rst                                 |   8 +-
>   engines/libblkio.c                        | 250 +++++++++++++++++++---
>   examples/libblkio-io_uring.fio            |  13 +-
>   examples/libblkio-virtio-blk-vfio-pci.fio |  13 +-
>   fio.1                                     |   7 +-
>   5 files changed, 257 insertions(+), 34 deletions(-)
> 
> diff --git a/HOWTO.rst b/HOWTO.rst
> index 763f4f51..4e69abfc 100644
> --- a/HOWTO.rst
> +++ b/HOWTO.rst
> @@ -2199,7 +2199,13 @@ I/O engine
>   			:option:`libblkio_driver`. If
>   			:option:`mem`/:option:`iomem` is not specified, memory
>   			allocation is delegated to libblkio (and so is
> -			guaranteed to work with the selected *driver*).
> +			guaranteed to work with the selected *driver*). One
> +			``struct blkio`` instance is used per process, so all
> +			jobs setting option :option:`thread` will share a single
> +			``struct blkio`` (with one queue per thread) and must
> +			specify compatible options. Note that some drivers don't
> +			allow several processes to access the same device or
> +			file simultaneously, but allow it for threads.
>   
>   I/O engine specific parameters
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> diff --git a/engines/libblkio.c b/engines/libblkio.c
> index a80be66f..987dea4f 100644
> --- a/engines/libblkio.c
> +++ b/engines/libblkio.c
> @@ -249,6 +249,105 @@ static int fio_blkio_set_props_from_str(struct blkio *b, const char *opt_name,
>   			blkio_get_error_msg()); \
>   	})
>   
> +static bool possibly_null_strs_equal(const char *a, const char *b)
> +{
> +	return (!a && !b) || (a && b && strcmp(a, b) == 0);
> +}
> +
> +/*
> + * Returns the total number of subjobs using option 'thread' in the entire
> + * workload that have the given value for the 'hipri' option.
> + */
> +static int total_threaded_subjobs(bool hipri)
> +{
> +	struct thread_data *td;
> +	unsigned int i;
> +	int count = 0;
> +
> +	for_each_td(td, i) {
> +		const struct fio_blkio_options *options = td->eo;
> +		if (td->o.use_thread && (bool)options->hipri == hipri)
> +			++count;
> +	}
> +
> +	return count;
> +}

The loop should skip jobs not using the libblkio ioengine because, for 
example, some ioengines don't have any ioengine options.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 01/10] Add a libblkio engine
  2022-11-22  9:17   ` Damien Le Moal
@ 2022-12-01 22:10     ` Alberto Faria
  0 siblings, 0 replies; 20+ messages in thread
From: Alberto Faria @ 2022-12-01 22:10 UTC (permalink / raw)
  To: Damien Le Moal; +Cc: fio, Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On Tue, Nov 22, 2022 at 9:17 AM Damien Le Moal
<damien.lemoal@opensource.wdc.com> wrote:
> On 11/22/22 03:28, Alberto Faria wrote:
> > The libblkio library provides a unified API for efficiently accessing
> > block devices using modern high-performance block I/O interfaces like
> > io_uring and vhost-user-blk. Using libblkio reduces the amount of code
> > needed for interfacing with storage devices and allows developers to
> > focus on their applcations.
> >
> > Add a libblkio engine that uses libblkio to perform I/O. This is useful
> > to benchmark the library itself, and also adds support for storage
> > interfaces and devices otherwise not supported by fio, such as
> > virtio-blk PCI, vhost-user, and vhost-vDPA devices.
> >
> > See the libblkio documentation [2] or KVM Forum 2022 [3] presentation
> > for more information on the library itself.
> >
> > [1] https://gitlab.com/libblkio/libblkio
> > [2] https://libblkio.gitlab.io/libblkio/index.html
> > [3] https://static.sched.com/hosted_files/kvmforum2022/8c/libblkio-kvm-forum-2022.pdf
> >
> > Signed-off-by: Alberto Faria <afaria@redhat.com>
> > ---
> >  HOWTO.rst                                 |  26 ++
> >  Makefile                                  |   6 +
> >  configure                                 |  25 ++
> >  engines/libblkio.c                        | 463 ++++++++++++++++++++++
> >  examples/libblkio-io_uring.fio            |  19 +
> >  examples/libblkio-virtio-blk-vfio-pci.fio |  18 +
> >  fio.1                                     |  19 +
> >  optgroup.h                                |   2 +
> >  8 files changed, 578 insertions(+)
> >  create mode 100644 engines/libblkio.c
> >  create mode 100644 examples/libblkio-io_uring.fio
> >  create mode 100644 examples/libblkio-virtio-blk-vfio-pci.fio
> >
> > diff --git a/HOWTO.rst b/HOWTO.rst
> > index e796f961..d5a2749c 100644
> > --- a/HOWTO.rst
> > +++ b/HOWTO.rst
> > @@ -2192,6 +2192,12 @@ I/O engine
> >                       the SPDK NVMe driver, or your own custom NVMe driver. The xnvme engine includes
> >                       engine specific options. (See https://xnvme.io).
> >
> > +             **libblkio**
> > +                     Use the libblkio library
> > +                     (https://gitlab.com/libblkio/libblkio). The specific
> > +                     *driver* to use must be set using
> > +                     :option:`libblkio_driver`.
> > +
> >  I/O engine specific parameters
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > @@ -2842,6 +2848,26 @@ with the caveat that when used on the command line, they must come after the
> >
> >       If this option is set. xnvme will use vectored read/write commands.
> >
> > +.. option:: libblkio_driver=str : [libblkio]
> > +
> > +     The driver to be used by libblkio (e.g. **virtio-blk-vfio-pci**).
>
> It would be nice to list the possible values and their meaning here.
>
> > +
> > +.. option:: libblkio_pre_connect_props=str : [libblkio]
> > +
> > +     A colon-separated list of libblkio properties to be set after creating
> > +     but before connecting the ``struct blkio``. Each property must have the
> > +     format ``<name>=<value>``. Colons can be escaped as ``\:``. These are
> > +     set after the engine sets any other properties, so those can be
> > +     overriden.
>
> "struct blkio" has no meaning whatsoever for the fio command line
> interface. So could this be reworded without using struct names ? E.g.
> "without connecting the device accessed by libblkio" ?
>
> And we need to list the possible properties or have at least a link to
> some documentation listing the possible properties. Otherwise, how can
> the user find that out ?

In v2 I ended up linking to the libblkio docs. I avoided adding lists
of available drivers and properties since those would become
incomplete rather quickly and also depend on the installed version of
libblkio.

Thanks,
Alberto


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH 05/10] engines/libblkio: Add option libblkio_vectored
  2022-12-01 17:11   ` Vincent Fu
@ 2022-12-01 22:13     ` Alberto Faria
  0 siblings, 0 replies; 20+ messages in thread
From: Alberto Faria @ 2022-12-01 22:13 UTC (permalink / raw)
  To: Vincent Fu; +Cc: fio, Kevin Wolf, Stefan Hajnoczi, Stefano Garzarella

On Thu, Dec 1, 2022 at 5:11 PM Vincent Fu <vincentfu@gmail.com> wrote:
> On 11/21/22 13:28, Alberto Faria wrote:
> > When enabled, read and write requests are submitted as vectored requests
> > using blkioq_{readv,writev}(), instead of using blkioq_{read,write}().
> >
> > Signed-off-by: Alberto Faria <afaria@redhat.com>
> > ---
> >   HOWTO.rst          |  4 ++++
> >   engines/libblkio.c | 46 +++++++++++++++++++++++++++++++++++++++++-----
> >   fio.1              |  3 +++
> >   3 files changed, 48 insertions(+), 5 deletions(-)
> >
> > diff --git a/HOWTO.rst b/HOWTO.rst
> > index cdea3258..b9c7c8df 100644
> > --- a/HOWTO.rst
> > +++ b/HOWTO.rst
> > @@ -2875,6 +2875,10 @@ with the caveat that when used on the command line, they must come after the
> >       set after the engine sets any other properties, so those can be
> >       overriden.
> >
> > +.. option:: libblkio_vectored=bool : [libblkio]
>
> See my response to the hipri patch regarding the difference between
> booleans and FIO_OPT_STR_SET. I don't have strong feelings regarding
> whether this should be a boolean or FIO_OPT_STR_SET but if the type
> listed in the HOWTO and manpage is boolean then the type in the options
> struct should be FIO_OPT_BOOL. No type should be listed when it's
> FIO_OPT_STR_SET.

Thanks, I went with keeping FIO_OPT_STR_SET and not listing the type
in the docs in v2.

Alberto


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2022-12-01 22:14 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-21 18:28 [PATCH 00/10] Add a libblkio engine Alberto Faria
2022-11-21 18:28 ` [PATCH 01/10] " Alberto Faria
2022-11-22  9:17   ` Damien Le Moal
2022-12-01 22:10     ` Alberto Faria
2022-11-21 18:28 ` [PATCH 02/10] Add engine flag FIO_SKIPPABLE_IOMEM_ALLOC Alberto Faria
2022-11-21 18:28 ` [PATCH 03/10] engines/libblkio: Allow setting option mem/iomem Alberto Faria
2022-11-21 18:28 ` [PATCH 04/10] engines/libblkio: Add support for poll queues Alberto Faria
2022-12-01 17:01   ` Vincent Fu
2022-11-21 18:28 ` [PATCH 05/10] engines/libblkio: Add option libblkio_vectored Alberto Faria
2022-12-01 17:11   ` Vincent Fu
2022-12-01 22:13     ` Alberto Faria
2022-11-21 18:28 ` [PATCH 06/10] engines/libblkio: Add option libblkio_write_zeroes_on_trim Alberto Faria
2022-12-01 17:13   ` Vincent Fu
2022-11-21 18:28 ` [PATCH 07/10] engines/libblkio: Add option libblkio_wait_mode Alberto Faria
2022-12-01 17:21   ` Vincent Fu
2022-11-21 18:29 ` [PATCH 08/10] engines/libblkio: Add option libblkio_force_enable_completion_eventfd Alberto Faria
2022-12-01 17:23   ` Vincent Fu
2022-11-21 18:29 ` [PATCH 09/10] engines/libblkio: Add options for some driver-specific properties Alberto Faria
2022-11-21 18:29 ` [PATCH 10/10] engines/libblkio: Share a single blkio instance among threads in same process Alberto Faria
2022-12-01 17:29   ` Vincent Fu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).