On 6/2/25 4:20 PM, Stefan Hajnoczi wrote:
On Mon, Jun 02, 2025 at 08:26:39AM -0400, Brian wrote:
On 5/28/25 3:09 PM, Stefan Hajnoczi wrote:
io_uring may not be available at runtime due to system policies (e.g.
the io_uring_disabled sysctl) or creation could fail due to file
descriptor resource limits.
Handle failure scenarios as follows:
If another AioContext already has io_uring, then fail AioContext
creation so that the aio_add_sqe() API is available uniformly from all
QEMU threads. Otherwise fall back to epoll(7) if io_uring is
unavailable.
Notes:
- Update the comment about selecting the fastest fdmon implementation.
At this point it's not about speed anymore, it's about aio_add_sqe()
API availability.
- Uppercase the error message when converting from error_report() to
error_setg_errno() for consistency (but there are instances of
lowercase in the codebase).
- It's easier to move the #ifdefs from aio-posix.h to aio-posix.c.
Signed-off-by: Stefan Hajnoczi<stefa...@redhat.com>
---
util/aio-posix.h | 12 ++----------
util/aio-posix.c | 29 ++++++++++++++++++++++++++---
util/fdmon-io_uring.c | 8 ++++----
3 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/util/aio-posix.h b/util/aio-posix.h
index f9994ed79e..6f9d97d866 100644
--- a/util/aio-posix.h
+++ b/util/aio-posix.h
@@ -18,6 +18,7 @@
#define AIO_POSIX_H
#include "block/aio.h"
+#include "qapi/error.h" struct AioHandler { GPollFD pfd; @@ -72,17
+73,8 @@ static inline void fdmon_epoll_disable(AioContext *ctx) #endif
/* !CONFIG_EPOLL_CREATE1 */ #ifdef CONFIG_LINUX_IO_URING -bool
fdmon_io_uring_setup(AioContext *ctx); +void
fdmon_io_uring_setup(AioContext *ctx, Error **errp); void
fdmon_io_uring_destroy(AioContext *ctx); -#else -static inline bool
fdmon_io_uring_setup(AioContext *ctx) -{ - return false; -} - -static
inline void fdmon_io_uring_destroy(AioContext *ctx) -{ -} #endif /*
!CONFIG_LINUX_IO_URING */ #endif /* AIO_POSIX_H */ diff --git
a/util/aio-posix.c b/util/aio-posix.c index fa047fc7ad..44b3df61f9
100644 --- a/util/aio-posix.c +++ b/util/aio-posix.c @@ -16,6 +16,7 @@
#include "qemu/osdep.h"
#include "block/block.h"
#include "block/thread-pool.h"
+#include "qapi/error.h"
#include "qemu/main-loop.h"
#include "qemu/lockcnt.h"
#include "qemu/rcu.h"
@@ -717,17 +718,39 @@ void aio_context_setup(AioContext *ctx, Error **errp)
ctx->epollfd = -1;
ctx->epollfd_tag = NULL;
- /* Use the fastest fd monitoring implementation if available */
- if (fdmon_io_uring_setup(ctx)) {
- return;
+#ifdef CONFIG_LINUX_IO_URING
+ {
+ static bool need_io_uring;
+ Error *local_err = NULL; /* ERRP_GUARD() doesn't handle error_abort */
+
+ /* io_uring takes precedence because it provides aio_add_sqe() support
*/
+ fdmon_io_uring_setup(ctx, &local_err);
+ if (!local_err) {
+ /*
+ * If one AioContext gets io_uring, then all AioContexts need
io_uring
+ * so that aio_add_sqe() support is available across all threads.
+ */
+ need_io_uring = true;
+ return;
+ }
+ if (need_io_uring) {
+ error_propagate(errp, local_err);
+ return;
+ }
+
+ warn_report_err_once(local_err); /* frees local_err */
+ local_err = NULL;
}
+#endif /* CONFIG_LINUX_IO_URING */
Is there a problem with the logic of this code snippet?
If we fail at fdmon_io_uring_setup, specifically at io_uring_queue_init,
local_err (or errp) will be set to a non-NULL error value. In that case,
need_io_uring will be set to true, but the function will return immediately.
If local_err is non-NULL then this conditional is not taken:
if (!local_err) {
/*
* If one AioContext gets io_uring, then all AioContexts need io_uring
* so that aio_add_sqe() support is available across all threads.
*/
need_io_uring = true;
return;
}
If the logic you described is correct, please rephrase it. I don't see
how what you've written can happen.
Sorry, I didn’t notice that need_io_uring is a static var. Was confused
about when if (need_io_block) gets executed
As a result, the later if (need_io_uring) block will never be executed
fdmon_epoll_setup(ctx);
}
void aio_context_destroy(AioContext *ctx)
{
+#ifdef CONFIG_LINUX_IO_URING
fdmon_io_uring_destroy(ctx);
+#endif
qemu_lockcnt_lock(&ctx->list_lock);
fdmon_epoll_disable(ctx);
diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c
index 2092d08d24..ef1a866a03 100644
--- a/util/fdmon-io_uring.c
+++ b/util/fdmon-io_uring.c
@@ -45,6 +45,7 @@
#include "qemu/osdep.h"
#include <poll.h>
+#include "qapi/error.h"
#include "qemu/rcu_queue.h"
#include "aio-posix.h"
@@ -361,7 +362,7 @@ static const FDMonOps fdmon_io_uring_ops = {
.gsource_dispatch = fdmon_io_uring_gsource_dispatch,
};
-bool fdmon_io_uring_setup(AioContext *ctx)
+void fdmon_io_uring_setup(AioContext *ctx, Error **errp)
{
int ret;
@@ -369,15 +370,14 @@ bool fdmon_io_uring_setup(AioContext *ctx)
ret = io_uring_queue_init(FDMON_IO_URING_ENTRIES, &ctx->fdmon_io_uring,
0);
if (ret != 0) {
- return false;
+ error_setg_errno(errp, -ret, "Failed to initialize io_uring");
+ return;
}
QSLIST_INIT(&ctx->submit_list);
ctx->fdmon_ops = &fdmon_io_uring_ops;
ctx->io_uring_fd_tag = g_source_add_unix_fd(&ctx->source,
ctx->fdmon_io_uring.ring_fd, G_IO_IN);
-
- return true;
}
void fdmon_io_uring_destroy(AioContext *ctx)
