Re: [Qemu-devel] [Qemu-block] [PATCH 09/17] mirror: Lock AioContext in mirror_co_perform()

[Paolo Bonzini <pbonzini@redhat.com>]

On 13/08/2018 04:19, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Locking AioContext should not be needed anywhere here.  mirror_run is
called via aio_co_enter or aio_co_wake, so the lock is actually already
taken every time you call aio_context_acquire.

It was needed only because AIO callbacks do *not* take the lock, but
it's not needed anymore since the conversion to coroutines.

Paolo

>  block/mirror.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 85b08086cc..6330269156 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -196,7 +196,6 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret)
>  {
>      MirrorBlockJob *s = op->s;
>  
> -    aio_context_acquire(blk_get_aio_context(s->common.blk));
>      if (ret < 0) {
>          BlockErrorAction action;
>  
> @@ -207,14 +206,12 @@ static void coroutine_fn mirror_write_complete(MirrorOp *op, int ret)
>          }
>      }
>      mirror_iteration_done(op, ret);
> -    aio_context_release(blk_get_aio_context(s->common.blk));
>  }
>  
>  static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret)
>  {
>      MirrorBlockJob *s = op->s;
>  
> -    aio_context_acquire(blk_get_aio_context(s->common.blk));
>      if (ret < 0) {
>          BlockErrorAction action;
>  
> @@ -230,7 +227,6 @@ static void coroutine_fn mirror_read_complete(MirrorOp *op, int ret)
>                               op->qiov.size, &op->qiov, 0);
>          mirror_write_complete(op, ret);
>      }
> -    aio_context_release(blk_get_aio_context(s->common.blk));
>  }
>  
>  /* Clip bytes relative to offset to not exceed end-of-file */
> @@ -387,12 +383,16 @@ static void coroutine_fn mirror_co_perform(void *opaque)
>  {
>      MirrorOp *op = opaque;
>      MirrorBlockJob *s = op->s;
> +    AioContext *aio_context;
>      int ret;
>  
> +    aio_context = blk_get_aio_context(s->common.blk);
> +    aio_context_acquire(aio_context);
> +
>      switch (op->mirror_method) {
>      case MIRROR_METHOD_COPY:
>          mirror_co_read(opaque);
> -        return;
> +        goto done;
>      case MIRROR_METHOD_ZERO:
>          ret = mirror_co_zero(s, op->offset, op->bytes);
>          break;
> @@ -404,6 +404,9 @@ static void coroutine_fn mirror_co_perform(void *opaque)
>      }
>  
>      mirror_write_complete(op, ret);
> +
> +done:
> +    aio_context_release(aio_context);
>  }
>  
>  /* If mirror_method == MIRROR_METHOD_COPY, *offset and *bytes will be
>