Re: [PATCH] block/io_uring: revert "Use io_uring_register_ring_fd() to skip fd operations"

[Stefano Garzarella]

On Sat, Sep 24, 2022 at 10:48:15PM +0800, Sam Li wrote:
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1193
>
> The commit "Use io_uring_register_ring_fd() to skip fd operations" broke
> when booting a guest with iothread and io_uring. That is because the
> io_uring_register_ring_fd() call is made from the main thread instead of
> IOThread where io_uring_submit() is called. It can not be guaranteed
> to register the ring fd in the correct thread or unregister the same ring
> fd if the IOThread is disabled. This optimization is not critical so we
> will revert previous commit.

Right, maybe we can call it on the first submit.

> This reverts commit e2848bc574fe2715c694bf8fe9a1ba7f78a1125a
> and 77e3f038af1764983087e3551a0fde9951952c4d.

Yep, better to revert for now:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


Should we queue this for stable?

Thanks,
Stefano

> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
> block/io_uring.c | 13 +------------
> meson.build      |  1 -
> 2 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/block/io_uring.c b/block/io_uring.c
> index a1760152e0..973e15d876 100644
> --- a/block/io_uring.c
> +++ b/block/io_uring.c
> @@ -11,7 +11,6 @@
> #include "qemu/osdep.h"
> #include <liburing.h>
> #include "block/aio.h"
> -#include "qemu/error-report.h"
> #include "qemu/queue.h"
> #include "block/block.h"
> #include "block/raw-aio.h"
> @@ -19,7 +18,6 @@
> #include "qapi/error.h"
> #include "trace.h"
>
> -
> /* io_uring ring size */
> #define MAX_ENTRIES 128
>
> @@ -432,17 +430,8 @@ LuringState *luring_init(Error **errp)
>     }
>
>     ioq_init(&s->io_q);
> -#ifdef CONFIG_LIBURING_REGISTER_RING_FD
> -    if (io_uring_register_ring_fd(&s->ring) < 0) {
> -        /*
> -         * Only warn about this error: we will fallback to the non-optimized
> -         * io_uring operations.
> -         */
> -        warn_report("failed to register linux io_uring ring file descriptor");
> -    }
> -#endif
> -
>     return s;
> +
> }
>
> void luring_cleanup(LuringState *s)
> diff --git a/meson.build b/meson.build
> index 3885fc1076..63cfb844cf 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1793,7 +1793,6 @@ 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>', 
> dependencies:linux_io_uring))
> config_host_data.set('CONFIG_LIBPMEM', libpmem.found())
> config_host_data.set('CONFIG_NUMA', numa.found())
> config_host_data.set('CONFIG_OPENGL', opengl.found())
> --
> 2.37.3