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

[Paolo Bonzini]

On 10/28/22 09:16, Emanuele Giuseppe Esposito wrote:
From: Paolo Bonzini <pbonz...@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 <pbonz...@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eespo...@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