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 <stefanha@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@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?