diff mbox series

[2/2] Use io_ring_register_ring_fd() to skip fd operations

Message ID 20220418090504.50107-1-faithilikerun@gmail.com
State New
Headers show
Series None | expand

Commit Message

Sam Li April 18, 2022, 9:05 a.m. UTC
fix code style issue.

Signed-off-by: Sam Li <faithilikerun@gmail.com>
---
 block/io_uring.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Damien Le Moal April 18, 2022, 10:24 p.m. UTC | #1
On 2022/04/18 18:05, Sam Li wrote:
> fix code style issue.

This patch must be squashed into the previous one.

> 
> Signed-off-by: Sam Li <faithilikerun@gmail.com>
> ---
>  block/io_uring.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/block/io_uring.c b/block/io_uring.c
> index 2942967126..57745ecfa1 100644
> --- a/block/io_uring.c
> +++ b/block/io_uring.c
> @@ -436,10 +436,15 @@ 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");
> +        /*
> +         * If the function fails, it will fallback to the non-optimized io_uring
> +         * operations.
> +         */

The comment wording is a little odd: given that the comment is inside the "if",
meaning that we are in the case "the function failed", saying "if the function
fails..." is strange. You could simply state something like:

	/*
	 * Only warn about this error: we will fall back to the non-optimized
	 * io_uring operations.
	 */

> +        error_setg_errno(errp, errno,
> +                         "failed to register linux io_uring ring file descriptor");
>      }
> -    return s;
>  
> +    return s;
>  }
>  
>  void luring_cleanup(LuringState *s)
Sam Li April 18, 2022, 11:36 p.m. UTC | #2
Thanks for noticing the problem. I've done that.

Sam

Damien Le Moal <Damien.LeMoal@wdc.com> 于2022年4月19日周二 06:24写道:

> On 2022/04/18 18:05, Sam Li wrote:
> > fix code style issue.
>
> This patch must be squashed into the previous one.
>
> >
> > Signed-off-by: Sam Li <faithilikerun@gmail.com>
> > ---
> >  block/io_uring.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/io_uring.c b/block/io_uring.c
> > index 2942967126..57745ecfa1 100644
> > --- a/block/io_uring.c
> > +++ b/block/io_uring.c
> > @@ -436,10 +436,15 @@ 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");
> > +        /*
> > +         * If the function fails, it will fallback to the non-optimized
> io_uring
> > +         * operations.
> > +         */
>
> The comment wording is a little odd: given that the comment is inside the
> "if",
> meaning that we are in the case "the function failed", saying "if the
> function
> fails..." is strange. You could simply state something like:
>
>         /*
>          * Only warn about this error: we will fall back to the
> non-optimized
>          * io_uring operations.
>          */
>
> > +        error_setg_errno(errp, errno,
> > +                         "failed to register linux io_uring ring file
> descriptor");
> >      }
> > -    return s;
> >
> > +    return s;
> >  }
> >
> >  void luring_cleanup(LuringState *s)
>
>
> --
> Damien Le Moal
> Western Digital Research
diff mbox series

Patch

diff --git a/block/io_uring.c b/block/io_uring.c
index 2942967126..57745ecfa1 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -436,10 +436,15 @@  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");
+        /*
+         * If the function fails, it will fallback to the non-optimized io_uring
+         * operations.
+         */
+        error_setg_errno(errp, errno,
+                         "failed to register linux io_uring ring file descriptor");
     }
-    return s;
 
+    return s;
 }
 
 void luring_cleanup(LuringState *s)