Re: [PATCH 7/8] virtio-blk: don't acquire AioContext in virtio_blk_handle_vq()

[Emanuele Giuseppe Esposito]

Am 08/11/2022 um 22:19 schrieb Stefan Hajnoczi:
> There is no need to acquire AioContext in virtio_blk_handle_vq() because
> no APIs used in the function require it and nothing else in the
> virtio-blk code requires mutual exclusion anymore.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  hw/block/virtio-blk.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index faea045178..771d87cfbe 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -784,7 +784,6 @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
>  
>      IO_CODE();
>  
> -    aio_context_acquire(blk_get_aio_context(s->blk));
>      blk_io_plug(s->blk);
>  
>      do {
> @@ -810,7 +809,6 @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
>      }
>  
>      blk_io_unplug(s->blk);
> -    aio_context_release(blk_get_aio_context(s->blk));
>  }
>  
>  static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> 

As already discussed offline, this might be problematic with the work I
am done.

Basically I am trying to replace the AioContext lock with a rwlock for
graph modifications, and in order to use it we must convert all
BlockDriver IO functions in coroutines, because they traverse the graph
and must take the read lock (defined as coroutine_fn).

This implies that for now we need to implement blk_* and bdrv_*
functions in a similar fashion as generated_co_wrapper, therefore
creating a coroutine and polling waiting for it.
And polling uses AIO_WAIT_WHILE, which assumes the AioContext lock to be
taken.

In the future, we will use AIO_WAIT_WHILE_UNLOCKED, as you did in patch
1, but right now it's definitely too early to do it for a g_c_w function.

For this specific case, I see that blk_ioplug/unplug is not called in a
lot of places:
- here, and it's ok
- virtio-scsi, and I think you are going to cover this too soon
- xen-block

So if you manage to make all callers aiocontext-free, then I can just
rebase on top of your series and use AIO_WAIT_WHILE_POLL for
blk_ioplug/unplug when I convert it in coroutine.

Thank you,
Emanuele