[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5] Use io_uring_register_ring_fd() to skip fd operations
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v5] Use io_uring_register_ring_fd() to skip fd operations |
Date: |
Tue, 31 May 2022 11:25:55 +0100 |
On Tue, 31 May 2022 at 09:49, Sam Li <faithilikerun@gmail.com> wrote:
>
> Linux recently added a new io_uring(7) optimization API that QEMU
> doesn't take advantage of yet. The liburing library that QEMU uses
> has added a corresponding new API calling io_uring_register_ring_fd().
> When this API is called after creating the ring, the io_uring_submit()
> library function passes a flag to the io_uring_enter(2) syscall
> allowing it to skip the ring file descriptor fdget()/fdput()
> operations. This saves some CPU cycles.
>
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
> block/io_uring.c | 13 ++++++++++++-
> meson.build | 1 +
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/block/io_uring.c b/block/io_uring.c
> index 0b401512b9..4d691d8373 100644
> --- a/block/io_uring.c
> +++ b/block/io_uring.c
> @@ -17,6 +17,9 @@
> #include "qemu/coroutine.h"
> #include "qapi/error.h"
> #include "trace.h"
> +#ifdef CONFIG_LIBURING_REGISTER_RING_FD
> + io_uring_register_ring_fd(&s->ring);
> +#endif
When CONFIG_LIBURING_REGISTER_RING_FD is defined the compiler will
report an error here since it is not possible to call a function at
the top level of a C source file.
There is already a io_uring_register_ring_fd() call inside
luring_init(), so this one can be removed from the patch. Please move
the #ifdef into luring_init() so io_uring_register_ring_fd() is only
called when the API is available.
>
> /* io_uring ring size */
> #define MAX_ENTRIES 128
> @@ -434,8 +437,16 @@ LuringState *luring_init(Error **errp)
> }
>
> ioq_init(&s->io_q);
> - return s;
> + if (io_uring_register_ring_fd(&s->ring) < 0) {
> + /*
> + * Only warn about this error: we will fallback to the non-optimized
> + * io_uring operations.
> + */
> + error_reportf_err(*errp,
> + "failed to register linux io_uring ring file
> descriptor");
Returning a non-NULL LuringState while also setting errp is likely to
cause problems. QEMU's error_set() functions have assert(*errp ==
NULL) so the caller will abort later on if another error occurs after
luring_init() set errp. The Error API expects errp to be set zero or
one times, not more. Please use warn_report() instead, then errp won't
be involved.
> + }
>
> + return s;
> }
>
> void luring_cleanup(LuringState *s)
> diff --git a/meson.build b/meson.build
> index 9ebc00f032..927e6ec1a4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1733,6 +1733,7 @@ config_host_data.set('CONFIG_LIBNFS', libnfs.found())
> config_host_data.set('CONFIG_LIBSSH', libssh.found())
> config_host_data.set('CONFIG_LINUX_AIO', libaio.found())
> config_host_data.set('CONFIG_LINUX_IO_URING', linux_io_uring.found())
> +config_host_data.set('CONFIG_LIBURING_REGISTER_RING_FD',
> cc.has_function('io_uring_register_ring_fd', prefix: '#include <liburing.h>'))
I checked build/meson-logs/meson-log.txt after compiling to see if
io_uring_register_ring_fd() was detected correctly. Detection always
fails because meson is compiling the test program without liburing
(there is a linker error because the io_uring_register_ring_fd symbol
cannot be found). This can be solved by adding a dependency on
linux_io_uring:
config_host_data.set('CONFIG_LIBURING_REGISTER_RING_FD',
cc.has_function('io_uring_register_ring_fd', prefix: '#include
<liburing.h>', dependencies: linux_io_uring))
Stefan