[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 01/32] block-coroutine-wrapper: Take AioContext lock in no_co_wrap
From: |
Kevin Wolf |
Subject: |
[PULL 01/32] block-coroutine-wrapper: Take AioContext lock in no_co_wrappers |
Date: |
Tue, 30 May 2023 18:32:08 +0200 |
All of the functions that currently take a BlockDriverState, BdrvChild
or BlockBackend as their first parameter expect the associated
AioContext to be locked when they are called. In the case of
no_co_wrappers, they are called from bottom halves directly in the main
loop, so no other caller can be expected to take the lock for them. This
can result in assertion failures because a lock that isn't taken is
released in nested event loops.
Looking at the first parameter is already done by co_wrappers to decide
where the coroutine should run, so doing the same in no_co_wrappers is
only consistent. Take the lock in the generated bottom halves to fix the
problem.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20230525124713.401149-2-kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block-common.h | 3 +++
block/block-backend.c | 7 ++++++-
scripts/block-coroutine-wrapper.py | 25 +++++++++++++++----------
3 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/block/block-common.h b/include/block/block-common.h
index 93196229ac..e15395f2cb 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -65,6 +65,9 @@
* scheduling a BH in the bottom half that runs the respective non-coroutine
* function. The coroutine yields after scheduling the BH and is reentered when
* the wrapped function returns.
+ *
+ * If the first parameter of the function is a BlockDriverState, BdrvChild or
+ * BlockBackend pointer, the AioContext lock for it is taken in the wrapper.
*/
#define no_co_wrapper
diff --git a/block/block-backend.c b/block/block-backend.c
index ca537cd0ad..26447664ab 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2394,9 +2394,14 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason)
AioContext *blk_get_aio_context(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
+ BlockDriverState *bs;
IO_CODE();
+ if (!blk) {
+ return qemu_get_aio_context();
+ }
+
+ bs = blk_bs(blk);
if (bs) {
AioContext *ctx = bdrv_get_aio_context(blk_bs(blk));
assert(ctx == blk->ctx);
diff --git a/scripts/block-coroutine-wrapper.py
b/scripts/block-coroutine-wrapper.py
index 60e9b3107c..d4a183db61 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -88,16 +88,7 @@ def __init__(self, wrapper_type: str, return_type: str,
name: str,
raise ValueError(f"no_co function can't be rdlock:
{self.name}")
self.target_name = f'{subsystem}_{subname}'
- t = self.args[0].type
- if t == 'BlockDriverState *':
- ctx = 'bdrv_get_aio_context(bs)'
- elif t == 'BdrvChild *':
- ctx = 'bdrv_get_aio_context(child->bs)'
- elif t == 'BlockBackend *':
- ctx = 'blk_get_aio_context(blk)'
- else:
- ctx = 'qemu_get_aio_context()'
- self.ctx = ctx
+ self.ctx = self.gen_ctx()
self.get_result = 's->ret = '
self.ret = 'return s.ret;'
@@ -109,6 +100,17 @@ def __init__(self, wrapper_type: str, return_type: str,
name: str,
self.co_ret = ''
self.return_field = ''
+ def gen_ctx(self, prefix: str = '') -> str:
+ t = self.args[0].type
+ if t == 'BlockDriverState *':
+ return f'bdrv_get_aio_context({prefix}bs)'
+ elif t == 'BdrvChild *':
+ return f'bdrv_get_aio_context({prefix}child->bs)'
+ elif t == 'BlockBackend *':
+ return f'blk_get_aio_context({prefix}blk)'
+ else:
+ return 'qemu_get_aio_context()'
+
def gen_list(self, format: str) -> str:
return ', '.join(format.format_map(arg.__dict__) for arg in self.args)
@@ -262,8 +264,11 @@ def gen_no_co_wrapper(func: FuncDecl) -> str:
static void {name}_bh(void *opaque)
{{
{struct_name} *s = opaque;
+ AioContext *ctx = {func.gen_ctx('s->')};
+ aio_context_acquire(ctx);
{func.get_result}{name}({ func.gen_list('s->{name}') });
+ aio_context_release(ctx);
aio_co_wake(s->co);
}}
--
2.40.1
- [PULL 00/32] Block layer patches, Kevin Wolf, 2023/05/30
- [PULL 06/32] qcow2: Fix open with 'file' in iothread, Kevin Wolf, 2023/05/30
- [PULL 05/32] mirror: Hold main AioContext lock for calling bdrv_open_backing_file(), Kevin Wolf, 2023/05/30
- [PULL 08/32] copy-before-write: Fix open with child in iothread, Kevin Wolf, 2023/05/30
- [PULL 03/32] block: Take main AioContext lock when calling bdrv_open(), Kevin Wolf, 2023/05/30
- [PULL 02/32] block: Clarify locking rules for bdrv_open(_inherit)(), Kevin Wolf, 2023/05/30
- [PULL 01/32] block-coroutine-wrapper: Take AioContext lock in no_co_wrappers,
Kevin Wolf <=
- [PULL 04/32] block-backend: Fix blk_new_open() for iothreads, Kevin Wolf, 2023/05/30
- [PULL 19/32] block/export: stop using is_external in vhost-user-blk server, Kevin Wolf, 2023/05/30
- [PULL 16/32] virtio-scsi: stop using aio_disable_external() during unplug, Kevin Wolf, 2023/05/30
- [PULL 15/32] virtio-scsi: avoid race between unplug and transport event, Kevin Wolf, 2023/05/30
- [PULL 17/32] util/vhost-user-server: rename refcount to in_flight counter, Kevin Wolf, 2023/05/30
- [PULL 18/32] block/export: wait for vhost-user-blk requests when draining, Kevin Wolf, 2023/05/30
- [PULL 24/32] hw/xen: do not set is_external=true on evtchn fds, Kevin Wolf, 2023/05/30
- [PULL 26/32] block/export: don't require AioContext lock around blk_exp_ref/unref(), Kevin Wolf, 2023/05/30
- [PULL 22/32] block: drain from main loop thread in bdrv_co_yield_to_drain(), Kevin Wolf, 2023/05/30
- [PULL 30/32] virtio-scsi: implement BlockDevOps->drained_begin(), Kevin Wolf, 2023/05/30