[PATCH 0/2] block-backend: process I/O in the current AioContext

[PATCH 0/2] block-backend: process I/O in the current AioContext

[Stefan Hajnoczi]

Switch blk_aio_*() APIs over to multi-queue by using
qemu_get_current_aio_context() instead of blk_get_aio_context(). This change
will allow devices to process I/O in multiple IOThreads in the future.

Stefan Hajnoczi (2):
  block-backend: process I/O in the current AioContext
  block-backend: process zoned requests in the current AioContext

 block/block-backend.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

-- 
2.41.0

[PATCH 1/2] block-backend: process I/O in the current AioContext

[Stefan Hajnoczi]

Switch blk_aio_*() APIs over to multi-queue by using
qemu_get_current_aio_context() instead of blk_get_aio_context(). This
change will allow devices to process I/O in multiple IOThreads in the
future.

I audited existing blk_aio_*() callers:
- migration/block.c: blk_mig_lock() protects the data accessed by the
  completion callback.
- The remaining emulated devices and exports run with
  qemu_get_aio_context() == blk_get_aio_context().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-backend.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 4009ed5fed..39d969c919 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1533,7 +1533,7 @@ BlockAIOCB *blk_abort_aio_request(BlockBackend *blk,
     acb->blk = blk;
     acb->ret = ret;
 
-    replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+    replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
                                      error_callback_bh, acb);
     return &acb->common;
 }
@@ -1595,11 +1595,11 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, int64_t offset,
     acb->has_returned = false;
 
     co = qemu_coroutine_create(co_entry, acb);
-    aio_co_enter(blk_get_aio_context(blk), co);
+    aio_co_enter(qemu_get_current_aio_context(), co);
 
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+        replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
                                          blk_aio_complete_bh, acb);
     }
 
-- 
2.41.0

[PATCH 2/2] block-backend: process zoned requests in the current AioContext

[Stefan Hajnoczi]

Process zoned requests in the current thread's AioContext instead of in
the BlockBackend's AioContext.

There is no need to use the BlockBackend's AioContext thanks to CoMutex
bs->wps->colock, which protects zone metadata.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/block-backend.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index 39d969c919..fae058913d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1901,11 +1901,11 @@ BlockAIOCB *blk_aio_zone_report(BlockBackend *blk, int64_t offset,
     acb->has_returned = false;
 
     co = qemu_coroutine_create(blk_aio_zone_report_entry, acb);
-    aio_co_enter(blk_get_aio_context(blk), co);
+    aio_co_enter(qemu_get_current_aio_context(), co);
 
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+        replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
                                          blk_aio_complete_bh, acb);
     }
 
@@ -1942,11 +1942,11 @@ BlockAIOCB *blk_aio_zone_mgmt(BlockBackend *blk, BlockZoneOp op,
     acb->has_returned = false;
 
     co = qemu_coroutine_create(blk_aio_zone_mgmt_entry, acb);
-    aio_co_enter(blk_get_aio_context(blk), co);
+    aio_co_enter(qemu_get_current_aio_context(), co);
 
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+        replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
                                          blk_aio_complete_bh, acb);
     }
 
@@ -1982,10 +1982,10 @@ BlockAIOCB *blk_aio_zone_append(BlockBackend *blk, int64_t *offset,
     acb->has_returned = false;
 
     co = qemu_coroutine_create(blk_aio_zone_append_entry, acb);
-    aio_co_enter(blk_get_aio_context(blk), co);
+    aio_co_enter(qemu_get_current_aio_context(), co);
     acb->has_returned = true;
     if (acb->rwco.ret != NOT_DONE) {
-        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
+        replay_bh_schedule_oneshot_event(qemu_get_current_aio_context(),
                                          blk_aio_complete_bh, acb);
     }
 
-- 
2.41.0

Re: [PATCH 0/2] block-backend: process I/O in the current AioContext

[Kevin Wolf]

Am 15.08.2023 um 18:05 hat Stefan Hajnoczi geschrieben:
> Switch blk_aio_*() APIs over to multi-queue by using
> qemu_get_current_aio_context() instead of blk_get_aio_context(). This change
> will allow devices to process I/O in multiple IOThreads in the future.

Both code paths still use blk_aio_em_aiocb_info, which is:

    static AioContext *blk_aio_em_aiocb_get_aio_context(BlockAIOCB *acb_)
    {
        BlkAioEmAIOCB *acb = container_of(acb_, BlkAioEmAIOCB, common);

        return blk_get_aio_context(acb->rwco.blk);
    }

    static const AIOCBInfo blk_aio_em_aiocb_info = {
        .aiocb_size         = sizeof(BlkAioEmAIOCB),
        .get_aio_context    = blk_aio_em_aiocb_get_aio_context,
    };

.get_aio_context() is called by bdrv_aio_cancel(), which already looks
wrong before this patch because in theory it can end up polling the
AioContext of a different thread. After this patch, .get_aio_context()
doesn't even necessarily return the AioContext that runs the request any
more.

The only thing that might save us is that I can't find any device that
both supports iothreads and calls bdrv_aio_cancel(). But we shouldn't
rely on that.

Maybe the solution is to just remove .get_aio_context altogether and use
AIO_WAIT_WHILE(NULL, ...) in bdrv_aio_cancel().

Kevin

Re: [PATCH 0/2] block-backend: process I/O in the current AioContext

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]

On Fri, Aug 18, 2023 at 05:24:22PM +0200, Kevin Wolf wrote:
> Am 15.08.2023 um 18:05 hat Stefan Hajnoczi geschrieben:
> > Switch blk_aio_*() APIs over to multi-queue by using
> > qemu_get_current_aio_context() instead of blk_get_aio_context(). This change
> > will allow devices to process I/O in multiple IOThreads in the future.
> 
> Both code paths still use blk_aio_em_aiocb_info, which is:
> 
>     static AioContext *blk_aio_em_aiocb_get_aio_context(BlockAIOCB *acb_)
>     {
>         BlkAioEmAIOCB *acb = container_of(acb_, BlkAioEmAIOCB, common);
> 
>         return blk_get_aio_context(acb->rwco.blk);
>     }
> 
>     static const AIOCBInfo blk_aio_em_aiocb_info = {
>         .aiocb_size         = sizeof(BlkAioEmAIOCB),
>         .get_aio_context    = blk_aio_em_aiocb_get_aio_context,
>     };
> 
> .get_aio_context() is called by bdrv_aio_cancel(), which already looks
> wrong before this patch because in theory it can end up polling the
> AioContext of a different thread. After this patch, .get_aio_context()
> doesn't even necessarily return the AioContext that runs the request any
> more.
> 
> The only thing that might save us is that I can't find any device that
> both supports iothreads and calls bdrv_aio_cancel(). But we shouldn't
> rely on that.
> 
> Maybe the solution is to just remove .get_aio_context altogether and use
> AIO_WAIT_WHILE(NULL, ...) in bdrv_aio_cancel().

I will remove AIOCBInfo.get_aio_context in v2.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]