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 <faithilike...@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
