On 2022-04-22 09:52, Daniel P. Berrangé wrote:
> On Fri, Apr 22, 2022 at 09:34:28AM +0100, Fam Zheng wrote:
> > On 2022-04-22 00:36, 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 | 10 +++++++++-
> > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/block/io_uring.c b/block/io_uring.c
> > > index 782afdb433..5247fb79e2 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_reportf_err(*errp,
> > > + "failed to register linux io_uring ring file
> > > descriptor");
> >
> > IIUC errp can be NULL, so let's not dereference it without checking. So,
> > just
> > use error_report?
>
> Plenty of people will be running kernels that lack the new feature,
> so this "failure" will be an expected scenario. We shouldn't be
> spamming the logs with any error or warning message. Assuming QEMU
> remains fully functional, merely not as optimized, we should be
> totally silent.
Functionally, that's a very valid point. But performance wise, is it good to
have some visibility of this? Since people use io_uring instead of other
options almost certainly for performance, and here the issue does matter quite
a bit.
Fam
>
> At most stick in a 'trace' point so we can record whether the
> optimization is present.
>
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>
>
