Re: [RFC PATCH 5/6] block: Allow bdrv_get_allocated_file_size to run in bdrv context

[Kevin Wolf <kwolf@redhat.com>]

Am 23.05.2023 um 23:39 hat Fabiano Rosas geschrieben:
> We're about to move calls to 'fstat' into the thread-pool to avoid
> blocking VCPU threads should the system call take too long.
> 
> To achieve that we first need to make sure none of its callers is
> holding the aio_context lock, otherwise yielding before scheduling the
> aiocb handler would result in a deadlock when the qemu_global_mutex is
> released and another thread tries to acquire the aio_context.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  block/qapi.c | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/block/qapi.c b/block/qapi.c
> index ae6cd1c2ff..cd197abf1f 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -222,6 +222,26 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
>      return 0;
>  }
>  
> +static int64_t bdrv_get_actual_size(BlockDriverState *bs)
> +{
> +    int64_t size;
> +    AioContext *old_ctx = NULL;
> +
> +    if (qemu_in_coroutine()) {

Hm. Why can't we make sure that it always runs in a coroutine?

Callers:

* bdrv_query_block_node_info(). This functions seems to be completely
  unused, we can remove it.

* bdrv_query_image_info(). Called by bdrv_block_device_info() and itself.
  bdrv_block_device_info() could become a co_wrapper after swapping the
  first two parameters so that it runs in the AioContext of @bs.

* bdrv_query_block_graph_info(). Only called by qemu-img. Could become a
  co_wrapper_bdrv_rdlock.

> +        aio_context_release(bdrv_get_aio_context(bs));
> +        old_ctx = bdrv_co_enter(bs);

I think this is the wrong function to do this. The caller should already
make sure that it's in the right AioContext.

> +    }
> +
> +    size = bdrv_get_allocated_file_size(bs);
> +
> +    if (qemu_in_coroutine() && old_ctx) {
> +        bdrv_co_leave(bs, old_ctx);
> +        aio_context_acquire(bdrv_get_aio_context(bs));
> +    }
> +
> +    return size;
> +}

Kevin