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? > + 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