block/io_uring.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
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 | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/block/io_uring.c b/block/io_uring.c
index 782afdb433..51f4834b69 100644
--- a/block/io_uring.c
+++ b/block/io_uring.c
@@ -435,8 +435,16 @@ LuringState *luring_init(Error **errp)
}
ioq_init(&s->io_q);
- return s;
+ if (io_uring_register_ring_fd(&s->ring) < 0) {
+ /*
+ * Only warn about this error: we 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;
}
void luring_cleanup(LuringState *s)
--
2.35.1
On 2022-04-19 07:33, 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 | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/block/io_uring.c b/block/io_uring.c > index 782afdb433..51f4834b69 100644 > --- a/block/io_uring.c > +++ b/block/io_uring.c > @@ -435,8 +435,16 @@ LuringState *luring_init(Error **errp) > } > > ioq_init(&s->io_q); > - return s; > + if (io_uring_register_ring_fd(&s->ring) < 0) { > + /* > + * Only warn about this error: we 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; As a general convention, I don't think the errp is going to get proper handling by the callers, if non-NULL is returned like here. IOW a matching error_free is never called and this is memory leak? I guess error_report is better? Fam > } > > void luring_cleanup(LuringState *s) > -- > 2.35.1 > >
Hi Fam, I've missed out freeing error object and error_report would be a better case indeed. Thanks for pointing out the problem. I'll fix that in no time. Best regards, Sam Fam Zheng <fam@euphon.net> 于2022年4月21日周四 21:36写道: > On 2022-04-19 07:33, 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 | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/block/io_uring.c b/block/io_uring.c > > index 782afdb433..51f4834b69 100644 > > --- a/block/io_uring.c > > +++ b/block/io_uring.c > > @@ -435,8 +435,16 @@ LuringState *luring_init(Error **errp) > > } > > > > ioq_init(&s->io_q); > > - return s; > > + if (io_uring_register_ring_fd(&s->ring) < 0) { > > + /* > > + * Only warn about this error: we 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; > > As a general convention, I don't think the errp is going to get proper > handling > by the callers, if non-NULL is returned like here. IOW a matching > error_free is > never called and this is memory leak? > > I guess error_report is better? > > Fam > > > } > > > > void luring_cleanup(LuringState *s) > > -- > > 2.35.1 > > > > >
© 2016 - 2024 Red Hat, Inc.