From: Kevin Wolf <kwolf@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, stefanha@redhat.com, hreitz@redhat.com,
eblake@redhat.com, qemu-devel@nongnu.org
Subject: [PATCH 01/12] block-coroutine-wrapper: Take AioContext lock in no_co_wrappers
Date: Thu, 25 May 2023 14:47:02 +0200 [thread overview]
Message-ID: <20230525124713.401149-2-kwolf@redhat.com> (raw)
In-Reply-To: <20230525124713.401149-1-kwolf@redhat.com>
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>
---
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
next prev parent reply other threads:[~2023-05-25 12:49 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-25 12:47 [PATCH 00/12] block: Fix blockdev-create with iothreads Kevin Wolf
2023-05-25 12:47 ` Kevin Wolf [this message]
2023-05-25 18:15 ` [PATCH 01/12] block-coroutine-wrapper: Take AioContext lock in no_co_wrappers Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 02/12] block: Clarify locking rules for bdrv_open(_inherit)() Kevin Wolf
2023-05-25 18:16 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 03/12] block: Take main AioContext lock when calling bdrv_open() Kevin Wolf
2023-05-25 18:20 ` Stefan Hajnoczi
2023-05-26 8:27 ` Kevin Wolf
2023-05-25 12:47 ` [PATCH 04/12] block-backend: Fix blk_new_open() for iothreads Kevin Wolf
2023-05-25 18:37 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 05/12] mirror: Hold main AioContext lock for calling bdrv_open_backing_file() Kevin Wolf
2023-05-25 18:40 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 06/12] qcow2: Fix open with 'file' in iothread Kevin Wolf
2023-05-25 18:51 ` Stefan Hajnoczi
2023-05-26 8:40 ` Kevin Wolf
2023-05-26 9:04 ` Kevin Wolf
2023-05-25 12:47 ` [PATCH 07/12] raw-format: " Kevin Wolf
2023-05-25 18:52 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 08/12] copy-before-write: Fix open with child " Kevin Wolf
2023-05-25 18:55 ` Stefan Hajnoczi
2023-05-26 14:30 ` Eric Blake
2023-05-25 12:47 ` [PATCH 09/12] block: Take AioContext lock in bdrv_open_driver() Kevin Wolf
2023-05-25 18:56 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 10/12] block: Fix AioContext locking in bdrv_insert_node() Kevin Wolf
2023-05-25 18:58 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 11/12] iotests: Make verify_virtio_scsi_pci_or_ccw() public Kevin Wolf
2023-05-25 18:59 ` Stefan Hajnoczi
2023-05-25 12:47 ` [PATCH 12/12] iotests: Test blockdev-create in iothread Kevin Wolf
2023-05-25 19:01 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230525124713.401149-2-kwolf@redhat.com \
--to=kwolf@redhat.com \
--cc=eblake@redhat.com \
--cc=hreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).