Re: [PATCH] Use io_uring_register_ring_fd() to skip fd operations

[Damien Le Moal]

On 2022/04/18 16:52, Sam Li 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 | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/io_uring.c b/block/io_uring.c
> index 782afdb433..2942967126 100644
> --- a/block/io_uring.c
> +++ b/block/io_uring.c
> @@ -435,6 +435,9 @@ LuringState *luring_init(Error **errp)
>      }
>  
>      ioq_init(&s->io_q);
> +    if (io_uring_register_ring_fd(&s->ring) < 0) {
> +        error_setg_errno(errp, errno, "failed to register linux
>  io_uring ring file descriptor");

This line seems broken. Probably your mailer wrapped the line. If you can,
please use "git send-email" to avoid such problems.

The line is very long anyway. So it probably is better to split it after errno:

        error_setg_errno(errp, errno,
                "failed to register linux io_uring ring file descriptor");

Also, if I understand this correctly, you ignore the error here and this will
naturally fallback to the non-optimized iouring operation which will do the
fdget()/fdput(), right ? If yes, it would be good to add a comment before the
error_setg_errno() call to make it clear that the error being ignored is not an
oversight.

> +    }

Personally, I like a blank line before return.

>      return s;
>  

And while at it, I would remove this useless blank line. Completely optional :)

>  }


-- 
Damien Le Moal
Western Digital Research