From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
John Snow <jsnow@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>,
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>,
Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>,
Eric Blake <eblake@redhat.com>, Cleber Rosa <crosa@redhat.com>,
qemu-devel@nongnu.org,
Emanuele Giuseppe Esposito <eesposit@redhat.com>
Subject: [PATCH 08/20] block-coroutine-wrapper.py: take the graph rdlock in bdrv_* functions
Date: Wed, 16 Nov 2022 08:48:38 -0500 [thread overview]
Message-ID: <20221116134850.3051419-9-eesposit@redhat.com> (raw)
In-Reply-To: <20221116134850.3051419-1-eesposit@redhat.com>
All generated_co_wrapper functions create a coroutine when
called from non-coroutine context.
The format can be one of the two:
bdrv_something()
if(qemu_in_coroutine()):
bdrv_co_something();
else:
// create coroutine that calls bdrv_co_something();
blk_something()
if(qemu_in_coroutine()):
blk_co_something();
else:
// create coroutine that calls blk_co_something();
// blk_co_something() then eventually calls bdrv_co_something()
The bdrv_co_something functions are recursively traversing the graph,
therefore they all need to be protected with the graph rdlock.
Instead, blk_co_something() calls bdrv_co_something(), so given that and
being always called at the root of the graph (not in recursive
callbacks), they should take the graph rdlock.
The contract is simple, from now on, all bdrv_co_* functions called by g_c_w
callbacks assume that the graph rdlock is taken at the coroutine
creation, i.e. in g_c_w or in specific coroutines (right now we just
consider the g_c_w case).
All the blk_co_* are responsible of taking the rdlock (at this point is still a TBD).
Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
scripts/block-coroutine-wrapper.py | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py
index 21ecb3e896..05267761f0 100644
--- a/scripts/block-coroutine-wrapper.py
+++ b/scripts/block-coroutine-wrapper.py
@@ -67,8 +67,11 @@ def __init__(self, return_type: str, name: str, args: str,
self.return_type = return_type.strip()
self.name = name.strip()
self.args = [ParamDecl(arg.strip()) for arg in args.split(',')]
+ self.lock = True
self.create_only_co = False
+ if variant == '_blk':
+ self.lock = False
if variant == '_simple':
self.create_only_co = True
@@ -86,7 +89,6 @@ def gen_block(self, format: str) -> str:
r'(?P<wrapper_name>[a-z][a-z0-9_]*)'
r'\((?P<args>[^)]*)\);$', re.MULTILINE)
-
def func_decl_iter(text: str) -> Iterator:
for m in func_decl_re.finditer(text):
yield FuncDecl(return_type=m.group('return_type'),
@@ -160,6 +162,13 @@ def gen_wrapper(func: FuncDecl) -> str:
func.co_name = f'{subsystem}_co_{subname}'
name = func.co_name
+ graph_lock=''
+ graph_unlock=''
+ if func.lock:
+ graph_lock=' bdrv_graph_co_rdlock();'
+ graph_unlock=' bdrv_graph_co_rdunlock();'
+
+
t = func.args[0].type
if t == 'BlockDriverState *':
bs = 'bs'
@@ -192,7 +201,9 @@ def gen_wrapper(func: FuncDecl) -> str:
{{
{struct_name} *s = opaque;
+{graph_lock}
s->ret = {name}({ func.gen_list('s->{name}') });
+{graph_unlock}
s->poll_state.in_progress = false;
aio_wait_kick();
--
2.31.1
next prev parent reply other threads:[~2022-11-16 13:53 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-16 13:48 [PATCH 00/20] Protect the block layer with a rwlock: part 1 Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 01/20] block: introduce a lock to protect graph operations Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 02/20] graph-lock: introduce BdrvGraphRWlock structure Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 03/20] async: register/unregister aiocontext in graph lock list Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 04/20] block.c: wrlock in bdrv_replace_child_noperm Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 05/20] block: remove unnecessary assert_bdrv_graph_writable() Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 06/20] block: assert that graph read and writes are performed correctly Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 07/20] graph-lock: implement WITH_GRAPH_RDLOCK_GUARD and GRAPH_RDLOCK_GUARD macros Emanuele Giuseppe Esposito
2022-11-16 13:48 ` Emanuele Giuseppe Esposito [this message]
2022-11-16 13:48 ` [PATCH 09/20] block-backend: introduce new generated_co_wrapper_blk annotation Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 10/20] block-gen: assert that {bdrv/blk}_co_truncate is always called with graph rdlock taken Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 11/20] block-gen: assert that bdrv_co_{check/invalidate_cache} are " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 12/20] block-gen: assert that bdrv_co_pwrite is " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 13/20] block-gen: assert that bdrv_co_pwrite_{zeros/sync} " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 14/20] block-gen: assert that bdrv_co_pread " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 15/20] block-gen: assert that {bdrv/blk}_co_flush " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 16/20] block-gen: assert that bdrv_co_{read/write}v_vmstate are " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 17/20] block-gen: assert that bdrv_co_pdiscard is " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 18/20] block-gen: assert that bdrv_co_common_block_status_above " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 19/20] block-gen: assert that bdrv_co_ioctl " Emanuele Giuseppe Esposito
2022-11-16 13:48 ` [PATCH 20/20] block-gen: assert that nbd_co_do_establish_connection " Emanuele Giuseppe Esposito
2022-11-21 15:02 ` [PATCH 00/20] Protect the block layer with a rwlock: part 1 Emanuele Giuseppe Esposito
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=20221116134850.3051419-9-eesposit@redhat.com \
--to=eesposit@redhat.com \
--cc=crosa@redhat.com \
--cc=eblake@redhat.com \
--cc=fam@euphon.net \
--cc=hreitz@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
--cc=vsementsov@yandex-team.ru \
/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).