Re: [PATCH 8/8] block, block-backend: write some hot coroutine wrappers by hand

[Eric Blake]

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?

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