Re: [PATCH qemu 1/1] Add IOURING_SETUP_SINGLE_ISSUER flag to improve iouring performance

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

On Tue, Mar 25, 2025 at 09:49:38PM +0100, ~h0lyalg0rithm wrote:
> From: Suraj Shirvankar <surajshirvankar@gmail.com>
> 

Please include the rationale for this change in the commit description.
This way anyone reading the git log will be able to understand the
intent behind this change. Something like:

  IORING_SETUP_SINGLE_ISSUER enables optimizations in the kernel for
  applications that only access the io_uring context from one thread.
  QEMU calls io_uring_enter(2) from one AioContext, so it is safe to
  enable this flag.

> Signed-off-by: Suraj Shirvankar <surajshirvankar@gmail.com>
> ---
>  util/fdmon-io_uring.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

`make check` fails with this patch applied because aio_context_new()
(which calls fdmon_io_uring_setup()) is called before the thread is
created. When fdmon_io_uring_wait() is called the io_uring context is
now being used by another thread:

  qemu-system-x86_64: ../util/fdmon-io_uring.c:330: fdmon_io_uring_wait: Assertion `ret >= 0' failed.

Once this hurdle is overcome it should be possible to use
IORING_SETUP_SINGLE_ISSUER. Two ideas:

1. Modify aio_context_new() callers so they create the AioContext inside
   the thread.

2. Defer io_uring context creation until it is needed. It's probably
   still a good idea to create a temporary io-uring context early during
   startup to check that io_uring is available (and then destroy it
   right away).

I slightly prefer the first option.

> 
> diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c
> index b0d68bdc44..235837abcb 100644
> --- a/util/fdmon-io_uring.c
> +++ b/util/fdmon-io_uring.c
> @@ -324,8 +324,14 @@ static const FDMonOps fdmon_io_uring_ops = {
>  bool fdmon_io_uring_setup(AioContext *ctx)
>  {
>      int ret;
> +    unsigned int flags = 0;
>  
> -    ret = io_uring_queue_init(FDMON_IO_URING_ENTRIES, &ctx->fdmon_io_uring, 0);
> +    /* This improves performance but can be skipped on old hosts */
> +#ifdef IORING_SETUP_SINGLE_ISSUER
> +    flags |= IORING_SETUP_SINGLE_ISSUER

The semicolon is missing at the end of the line.

> +#endif
> +
> +    ret = io_uring_queue_init(FDMON_IO_URING_ENTRIES, &ctx->fdmon_io_uring, flags);
>      if (ret != 0) {
>          return false;
>      }
> -- 
> 2.45.3
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]