Il ven 7 apr 2023, 22:04 Eric Blake <ebl...@redhat.com> ha scritto:
> On Fri, Apr 07, 2023 at 05:33:03PM +0200, Paolo Bonzini wrote:
> > The introduction of the graph lock is causing blk_get_geometry, a hot
> function
> > used in the I/O path, to create a coroutine. However, the only part
> that really
> > needs to run in coroutine context is the call to
> bdrv_co_refresh_total_sectors,
> > which in turn only happens in the rare case of host CD-ROM devices.
> >
> > So, write by hand the three wrappers on the path from
> blk_co_get_geometry to
> > bdrv_co_refresh_total_sectors, so that the coroutine wrapper is only
> created
> > if bdrv_nb_sectors actually calls bdrv_refresh_total_sectors.
> >
> > Reported-by: Stefan Hajnoczi <stefa...@redhat.com>
> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> > ---
> > block.c | 22 ++++++++++++++++++++++
> > block/block-backend.c | 27 +++++++++++++++++++++++++++
> >
> > include/sysemu/block-backend-io.h | 5 ++---
> > 4 files changed, 52 insertions(+), 4 deletions(-)
> >
> > diff --git a/block.c b/block.c
> > index dbbc8de30c24..3390efd18cf6 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -5859,6 +5859,28 @@ int64_t coroutine_fn
> bdrv_co_nb_sectors(BlockDriverState *bs)
> > return bs->total_sectors;
> > }
> >
> > +/*
> > + * This wrapper is written by hand because this function is in the hot
> I/O path,
> > + * via blk_get_geometry.
> > + */
> > +int64_t coroutine_mixed_fn bdrv_nb_sectors(BlockDriverState *bs)
> > +{
> > + BlockDriver *drv = bs->drv;
> > + IO_CODE();
> > +
> > + if (!drv)
> > + return -ENOMEDIUM;
> > +
> > + if (!bs->bl.has_variable_length) {
> > + int ret = bdrv_refresh_total_sectors(bs, bs->total_sectors);
>
> Is this logic backwards? Why are we only refreshing the total sectors
> when we don't have variable length?
>
Yes, it is backwards.
Paolo
> > + if (ret < 0) {
> > + return ret;
> > + }
> > + }
> > +
> > + return bs->total_sectors;
> > +}
> > +
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
>
>
