Re: [PATCH v2 1/3] linux-aio: use LinuxAioState from the running thread

[Paolo Bonzini <pbonzini@redhat.com>]

On 10/28/22 09:16, Emanuele Giuseppe Esposito wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Remove usage of aio_context_acquire by always submitting asynchronous
> AIO to the current thread's LinuxAioState.
> 
> In order to prevent mistakes from the caller side, avoid passing LinuxAioState
> in laio_io_{plug/unplug} and laio_co_submit.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
>   block/file-posix.c      | 10 +++-------
>   block/linux-aio.c       | 34 ++++++++++++++++++----------------
>   include/block/aio.h     |  4 ----
>   include/block/raw-aio.h | 10 ++++------
>   4 files changed, 25 insertions(+), 33 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 23acffb9a4..23fe98eb3e 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2099,10 +2099,8 @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset,
>   #endif
>   #ifdef CONFIG_LINUX_AIO
>       } else if (s->use_linux_aio) {
> -        LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
>           assert(qiov->size == bytes);
> -        return laio_co_submit(bs, aio, s->fd, offset, qiov, type,
> -                              s->aio_max_batch);
> +        return laio_co_submit(s->fd, offset, qiov, type, s->aio_max_batch);
>   #endif
>       }
>   
> @@ -2142,8 +2140,7 @@ static void raw_aio_plug(BlockDriverState *bs)
>       BDRVRawState __attribute__((unused)) *s = bs->opaque;
>   #ifdef CONFIG_LINUX_AIO
>       if (s->use_linux_aio) {
> -        LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
> -        laio_io_plug(bs, aio);
> +        laio_io_plug();
>       }
>   #endif
>   #ifdef CONFIG_LINUX_IO_URING
> @@ -2159,8 +2156,7 @@ static void raw_aio_unplug(BlockDriverState *bs)
>       BDRVRawState __attribute__((unused)) *s = bs->opaque;
>   #ifdef CONFIG_LINUX_AIO
>       if (s->use_linux_aio) {
> -        LinuxAioState *aio = aio_get_linux_aio(bdrv_get_aio_context(bs));
> -        laio_io_unplug(bs, aio, s->aio_max_batch);
> +        laio_io_unplug(s->aio_max_batch);
>       }
>   #endif
>   #ifdef CONFIG_LINUX_IO_URING
> diff --git a/block/linux-aio.c b/block/linux-aio.c
> index d2cfb7f523..c806d3bb91 100644
> --- a/block/linux-aio.c
> +++ b/block/linux-aio.c
> @@ -16,6 +16,9 @@
>   #include "qemu/coroutine.h"
>   #include "qapi/error.h"
>   
> +/* Only used for assertions.  */
> +#include "qemu/coroutine_int.h"
> +
>   #include <libaio.h>
>   
>   /*
> @@ -56,10 +59,8 @@ struct LinuxAioState {
>       io_context_t ctx;
>       EventNotifier e;
>   
> -    /* io queue for submit at batch.  Protected by AioContext lock. */
> +    /* All data is only used in one I/O thread.  */
>       LaioQueue io_q;
> -
> -    /* I/O completion processing.  Only runs in I/O thread.  */
>       QEMUBH *completion_bh;
>       int event_idx;
>       int event_max;
> @@ -102,9 +103,8 @@ static void qemu_laio_process_completion(struct qemu_laiocb *laiocb)
>        * later.  Coroutines cannot be entered recursively so avoid doing
>        * that!
>        */
> -    if (!qemu_coroutine_entered(laiocb->co)) {
> -        aio_co_wake(laiocb->co);
> -    }
> +    assert(laiocb->co->ctx == laiocb->ctx->aio_context);
> +    qemu_coroutine_enter_if_inactive(laiocb->co);

This is wrong, it misses the aio_context_acquire/aio_context_release 
pair in aio_co_enter() (which is called by aio_co_wake()).

Likewise in patch 2.

Paolo