From 1bd542016cc2c132e3d52dbc9e663966dfc10e72 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 28 Nov 2022 09:23:32 -0500 Subject: block: rename generated_co_wrapper in co_wrapper_mixed In preparation to the incoming new function specifiers, rename g_c_w with a more meaningful name and document it. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20221128142337.657646-10-eesposit@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- scripts/block-coroutine-wrapper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'scripts/block-coroutine-wrapper.py') diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index 08be813407..56e6425356 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -2,7 +2,7 @@ """Generate coroutine wrappers for block subsystem. The program parses one or several concatenated c files from stdin, -searches for functions with the 'generated_co_wrapper' specifier +searches for functions with the 'co_wrapper_mixed' specifier and generates corresponding wrappers on stdout. Usage: block-coroutine-wrapper.py generated-file.c FILE.[ch]... @@ -74,8 +74,8 @@ class FuncDecl: return '\n'.join(format.format_map(arg.__dict__) for arg in self.args) -# Match wrappers declared with a generated_co_wrapper mark -func_decl_re = re.compile(r'^int\s*generated_co_wrapper\s*' +# Match wrappers declared with a co_wrapper_mixed mark +func_decl_re = re.compile(r'^int\s*co_wrapper_mixed\s*' r'(?P[a-z][a-z0-9_]*)' r'\((?P[^)]*)\);$', re.MULTILINE) -- cgit 1.4.1 From 76a2f554c1e7b8acb332f765034fdf0ab3525202 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 28 Nov 2022 09:23:33 -0500 Subject: block-coroutine-wrapper.py: introduce co_wrapper This new annotation starts just a function wrapper that creates a new coroutine. It assumes the caller is not a coroutine. It will be the default annotation to be used in the future. This is much better as c_w_mixed, because it is clear if the caller is a coroutine or not, and provides the advantage of automating the code creation. In the future all c_w_mixed functions will be substituted by co_wrapper. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20221128142337.657646-11-eesposit@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- docs/devel/block-coroutine-wrapper.rst | 6 +- include/block/block-common.h | 8 ++- scripts/block-coroutine-wrapper.py | 110 +++++++++++++++++++++++---------- 3 files changed, 86 insertions(+), 38 deletions(-) (limited to 'scripts/block-coroutine-wrapper.py') diff --git a/docs/devel/block-coroutine-wrapper.rst b/docs/devel/block-coroutine-wrapper.rst index 64acc8d65d..6dd2cdcab3 100644 --- a/docs/devel/block-coroutine-wrapper.rst +++ b/docs/devel/block-coroutine-wrapper.rst @@ -26,12 +26,12 @@ called ``bdrv_foo()``. In this case the script can help. To trigger the generation: 1. You need ``bdrv_foo`` declaration somewhere (for example, in - ``block/coroutines.h``) with the ``co_wrapper_mixed`` mark, + ``block/coroutines.h``) with the ``co_wrapper`` mark, like this: .. code-block:: c - int co_wrapper_mixed bdrv_foo(); + int co_wrapper bdrv_foo(); 2. You need to feed this declaration to block-coroutine-wrapper script. For this, add the .h (or .c) file with the declaration to the @@ -46,7 +46,7 @@ Links 1. The script location is ``scripts/block-coroutine-wrapper.py``. -2. Generic place for private ``co_wrapper_mixed`` declarations is +2. Generic place for private ``co_wrapper`` declarations is ``block/coroutines.h``, for public declarations: ``include/block/block.h`` diff --git a/include/block/block-common.h b/include/block/block-common.h index ec2309055b..847e4d4626 100644 --- a/include/block/block-common.h +++ b/include/block/block-common.h @@ -42,9 +42,13 @@ * * Usage: read docs/devel/block-coroutine-wrapper.rst * - * co_wrapper_mixed functions can be called by both coroutine and - * non-coroutine context. + * There are 2 kind of specifiers: + * - co_wrapper functions can be called by only non-coroutine context, because + * they always generate a new coroutine. + * - co_wrapper_mixed functions can be called by both coroutine and + * non-coroutine context. */ +#define co_wrapper #define co_wrapper_mixed /* block.c */ diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index 56e6425356..2090c3bf73 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -2,7 +2,7 @@ """Generate coroutine wrappers for block subsystem. The program parses one or several concatenated c files from stdin, -searches for functions with the 'co_wrapper_mixed' specifier +searches for functions with the 'co_wrapper' specifier and generates corresponding wrappers on stdout. Usage: block-coroutine-wrapper.py generated-file.c FILE.[ch]... @@ -62,10 +62,25 @@ class ParamDecl: class FuncDecl: - def __init__(self, return_type: str, name: str, args: str) -> None: + def __init__(self, return_type: str, name: str, args: str, + variant: str) -> None: self.return_type = return_type.strip() self.name = name.strip() + self.struct_name = snake_to_camel(self.name) self.args = [ParamDecl(arg.strip()) for arg in args.split(',')] + self.create_only_co = 'mixed' not in variant + + subsystem, subname = self.name.split('_', 1) + self.co_name = f'{subsystem}_co_{subname}' + + t = self.args[0].type + if t == 'BlockDriverState *': + bs = 'bs' + elif t == 'BdrvChild *': + bs = 'child->bs' + else: + bs = 'blk_bs(blk)' + self.bs = bs def gen_list(self, format: str) -> str: return ', '.join(format.format_map(arg.__dict__) for arg in self.args) @@ -74,8 +89,9 @@ class FuncDecl: return '\n'.join(format.format_map(arg.__dict__) for arg in self.args) -# Match wrappers declared with a co_wrapper_mixed mark -func_decl_re = re.compile(r'^int\s*co_wrapper_mixed\s*' +# Match wrappers declared with a co_wrapper mark +func_decl_re = re.compile(r'^int\s*co_wrapper' + r'(?P(_[a-z][a-z0-9_]*)?)\s*' r'(?P[a-z][a-z0-9_]*)' r'\((?P[^)]*)\);$', re.MULTILINE) @@ -84,7 +100,8 @@ def func_decl_iter(text: str) -> Iterator: for m in func_decl_re.finditer(text): yield FuncDecl(return_type='int', name=m.group('wrapper_name'), - args=m.group('args')) + args=m.group('args'), + variant=m.group('variant')) def snake_to_camel(func_name: str) -> str: @@ -97,24 +114,67 @@ def snake_to_camel(func_name: str) -> str: return ''.join(words) +def create_mixed_wrapper(func: FuncDecl) -> str: + """ + Checks if we are already in coroutine + """ + name = func.co_name + struct_name = func.struct_name + return f"""\ +int {func.name}({ func.gen_list('{decl}') }) +{{ + if (qemu_in_coroutine()) {{ + return {name}({ func.gen_list('{name}') }); + }} else {{ + {struct_name} s = {{ + .poll_state.bs = {func.bs}, + .poll_state.in_progress = true, + +{ func.gen_block(' .{name} = {name},') } + }}; + + s.poll_state.co = qemu_coroutine_create({name}_entry, &s); + + return bdrv_poll_co(&s.poll_state); + }} +}}""" + + +def create_co_wrapper(func: FuncDecl) -> str: + """ + Assumes we are not in coroutine, and creates one + """ + name = func.co_name + struct_name = func.struct_name + return f"""\ +int {func.name}({ func.gen_list('{decl}') }) +{{ + {struct_name} s = {{ + .poll_state.bs = {func.bs}, + .poll_state.in_progress = true, + +{ func.gen_block(' .{name} = {name},') } + }}; + assert(!qemu_in_coroutine()); + + s.poll_state.co = qemu_coroutine_create({name}_entry, &s); + + return bdrv_poll_co(&s.poll_state); +}}""" + + def gen_wrapper(func: FuncDecl) -> str: assert not '_co_' in func.name assert func.return_type == 'int' assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *', 'BlockBackend *'] - subsystem, subname = func.name.split('_', 1) - - name = f'{subsystem}_co_{subname}' + name = func.co_name + struct_name = func.struct_name - t = func.args[0].type - if t == 'BlockDriverState *': - bs = 'bs' - elif t == 'BdrvChild *': - bs = 'child->bs' - else: - bs = 'blk_bs(blk)' - struct_name = snake_to_camel(name) + creation_function = create_mixed_wrapper + if func.create_only_co: + creation_function = create_co_wrapper return f"""\ /* @@ -136,23 +196,7 @@ static void coroutine_fn {name}_entry(void *opaque) aio_wait_kick(); }} -int {func.name}({ func.gen_list('{decl}') }) -{{ - if (qemu_in_coroutine()) {{ - return {name}({ func.gen_list('{name}') }); - }} else {{ - {struct_name} s = {{ - .poll_state.bs = {bs}, - .poll_state.in_progress = true, - -{ func.gen_block(' .{name} = {name},') } - }}; - - s.poll_state.co = qemu_coroutine_create({name}_entry, &s); - - return bdrv_poll_co(&s.poll_state); - }} -}}""" +{creation_function(func)}""" def gen_wrappers(input_code: str) -> str: -- cgit 1.4.1 From 0582fb8293ad4a5d67810fb362789eba6e0ae75e Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 28 Nov 2022 09:23:34 -0500 Subject: block-coroutine-wrapper.py: support functions without bs arg Right now, we take the first parameter of the function to get the BlockDriverState to pass to bdrv_poll_co(), that internally calls functions that figure in which aiocontext the coroutine should run. However, it is useless to pass a bs just to get its own AioContext, so instead pass it directly, and default to the main loop if no BlockDriverState is passed as parameter. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20221128142337.657646-12-eesposit@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- block/block-gen.h | 6 +++--- scripts/block-coroutine-wrapper.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) (limited to 'scripts/block-coroutine-wrapper.py') diff --git a/block/block-gen.h b/block/block-gen.h index f80cf4897d..08d977f493 100644 --- a/block/block-gen.h +++ b/block/block-gen.h @@ -30,7 +30,7 @@ /* Base structure for argument packing structures */ typedef struct BdrvPollCo { - BlockDriverState *bs; + AioContext *ctx; bool in_progress; int ret; Coroutine *co; /* Keep pointer here for debugging */ @@ -40,8 +40,8 @@ static inline int bdrv_poll_co(BdrvPollCo *s) { assert(!qemu_in_coroutine()); - bdrv_coroutine_enter(s->bs, s->co); - BDRV_POLL_WHILE(s->bs, s->in_progress); + aio_co_enter(s->ctx, s->co); + AIO_WAIT_WHILE(s->ctx, s->in_progress); return s->ret; } diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index 2090c3bf73..f540003af1 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -75,12 +75,14 @@ class FuncDecl: t = self.args[0].type if t == 'BlockDriverState *': - bs = 'bs' + ctx = 'bdrv_get_aio_context(bs)' elif t == 'BdrvChild *': - bs = 'child->bs' + ctx = 'bdrv_get_aio_context(child->bs)' + elif t == 'BlockBackend *': + ctx = 'blk_get_aio_context(blk)' else: - bs = 'blk_bs(blk)' - self.bs = bs + ctx = 'qemu_get_aio_context()' + self.ctx = ctx def gen_list(self, format: str) -> str: return ', '.join(format.format_map(arg.__dict__) for arg in self.args) @@ -127,7 +129,7 @@ int {func.name}({ func.gen_list('{decl}') }) return {name}({ func.gen_list('{name}') }); }} else {{ {struct_name} s = {{ - .poll_state.bs = {func.bs}, + .poll_state.ctx = {func.ctx}, .poll_state.in_progress = true, { func.gen_block(' .{name} = {name},') } @@ -150,7 +152,7 @@ def create_co_wrapper(func: FuncDecl) -> str: int {func.name}({ func.gen_list('{decl}') }) {{ {struct_name} s = {{ - .poll_state.bs = {func.bs}, + .poll_state.ctx = {func.ctx}, .poll_state.in_progress = true, { func.gen_block(' .{name} = {name},') } @@ -166,8 +168,6 @@ int {func.name}({ func.gen_list('{decl}') }) def gen_wrapper(func: FuncDecl) -> str: assert not '_co_' in func.name assert func.return_type == 'int' - assert func.args[0].type in ['BlockDriverState *', 'BdrvChild *', - 'BlockBackend *'] name = func.co_name struct_name = func.struct_name -- cgit 1.4.1 From 6700dfb1b8c2828aa0c851136892c4774de87c95 Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Mon, 28 Nov 2022 09:23:35 -0500 Subject: block-coroutine-wrapper.py: support also basic return types Extend the regex to cover also return type, pointers included. This implies that the value returned by the function cannot be a simple "int" anymore, but the custom return type. Therefore remove poll_state->ret and instead use a per-function custom "ret" field. Signed-off-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20221128142337.657646-13-eesposit@redhat.com> Signed-off-by: Kevin Wolf --- block/block-gen.h | 5 +---- scripts/block-coroutine-wrapper.py | 19 +++++++++++-------- 2 files changed, 12 insertions(+), 12 deletions(-) (limited to 'scripts/block-coroutine-wrapper.py') diff --git a/block/block-gen.h b/block/block-gen.h index 08d977f493..89b7daaa1f 100644 --- a/block/block-gen.h +++ b/block/block-gen.h @@ -32,18 +32,15 @@ typedef struct BdrvPollCo { AioContext *ctx; bool in_progress; - int ret; Coroutine *co; /* Keep pointer here for debugging */ } BdrvPollCo; -static inline int bdrv_poll_co(BdrvPollCo *s) +static inline void bdrv_poll_co(BdrvPollCo *s) { assert(!qemu_in_coroutine()); aio_co_enter(s->ctx, s->co); AIO_WAIT_WHILE(s->ctx, s->in_progress); - - return s->ret; } #endif /* BLOCK_BLOCK_GEN_H */ diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index f540003af1..71a06e917a 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -92,7 +92,8 @@ class FuncDecl: # Match wrappers declared with a co_wrapper mark -func_decl_re = re.compile(r'^int\s*co_wrapper' +func_decl_re = re.compile(r'^(?P[a-zA-Z][a-zA-Z0-9_]* [\*]?)' + r'\s*co_wrapper' r'(?P(_[a-z][a-z0-9_]*)?)\s*' r'(?P[a-z][a-z0-9_]*)' r'\((?P[^)]*)\);$', re.MULTILINE) @@ -100,7 +101,7 @@ func_decl_re = re.compile(r'^int\s*co_wrapper' def func_decl_iter(text: str) -> Iterator: for m in func_decl_re.finditer(text): - yield FuncDecl(return_type='int', + yield FuncDecl(return_type=m.group('return_type'), name=m.group('wrapper_name'), args=m.group('args'), variant=m.group('variant')) @@ -123,7 +124,7 @@ def create_mixed_wrapper(func: FuncDecl) -> str: name = func.co_name struct_name = func.struct_name return f"""\ -int {func.name}({ func.gen_list('{decl}') }) +{func.return_type} {func.name}({ func.gen_list('{decl}') }) {{ if (qemu_in_coroutine()) {{ return {name}({ func.gen_list('{name}') }); @@ -137,7 +138,8 @@ int {func.name}({ func.gen_list('{decl}') }) s.poll_state.co = qemu_coroutine_create({name}_entry, &s); - return bdrv_poll_co(&s.poll_state); + bdrv_poll_co(&s.poll_state); + return s.ret; }} }}""" @@ -149,7 +151,7 @@ def create_co_wrapper(func: FuncDecl) -> str: name = func.co_name struct_name = func.struct_name return f"""\ -int {func.name}({ func.gen_list('{decl}') }) +{func.return_type} {func.name}({ func.gen_list('{decl}') }) {{ {struct_name} s = {{ .poll_state.ctx = {func.ctx}, @@ -161,13 +163,13 @@ int {func.name}({ func.gen_list('{decl}') }) s.poll_state.co = qemu_coroutine_create({name}_entry, &s); - return bdrv_poll_co(&s.poll_state); + bdrv_poll_co(&s.poll_state); + return s.ret; }}""" def gen_wrapper(func: FuncDecl) -> str: assert not '_co_' in func.name - assert func.return_type == 'int' name = func.co_name struct_name = func.struct_name @@ -183,6 +185,7 @@ def gen_wrapper(func: FuncDecl) -> str: typedef struct {struct_name} {{ BdrvPollCo poll_state; + {func.return_type} ret; { func.gen_block(' {decl};') } }} {struct_name}; @@ -190,7 +193,7 @@ static void coroutine_fn {name}_entry(void *opaque) {{ {struct_name} *s = opaque; - s->poll_state.ret = {name}({ func.gen_list('s->{name}') }); + s->ret = {name}({ func.gen_list('s->{name}') }); s->poll_state.in_progress = false; aio_wait_kick(); -- cgit 1.4.1 From e6d3f7a602a370362bc52b0aed7dfff1a0bf726d Mon Sep 17 00:00:00 2001 From: Emanuele Giuseppe Esposito Date: Wed, 7 Dec 2022 14:18:36 +0100 Subject: block-coroutine-wrapper.py: introduce annotations that take the graph rdlock Add co_wrapper_bdrv_rdlock and co_wrapper_mixed_bdrv_rdlock option to the block-coroutine-wrapper.py script. This "_bdrv_rdlock" option takes and releases the graph rdlock when a coroutine function is created. This means that when used together with "_mixed", the function marked with co_wrapper_mixed_bdrv_rdlock will support both coroutine and non-coroutine case, and in the latter case it will create a coroutine that takes and releases the rdlock. When called from a coroutine, the caller must already hold the graph lock. Example: void co_wrapper_mixed_bdrv_rdlock bdrv_f1(); Becomes static void bdrv_co_enter_f1() { bdrv_graph_co_rdlock(); bdrv_co_function(); bdrv_graph_co_rdunlock(); } void bdrv_f1() { if (qemu_in_coroutine) { assume_graph_lock(); bdrv_co_function(); } else { qemu_co_enter(bdrv_co_enter_f1); ... } } When used alone, the function will not work in coroutine context, and when called in non-coroutine context it will create a new coroutine that takes care of taking and releasing the rdlock automatically. Example: void co_wrapper_bdrv_rdlock bdrv_f1(); Becomes static void bdrv_co_enter_f1() { bdrv_graph_co_rdlock(); bdrv_co_function(); bdrv_graph_co_rdunlock(); } void bdrv_f1() { assert(!qemu_in_coroutine()); qemu_co_enter(bdrv_co_enter_f1); ... } About their usage: - co_wrapper does not take the rdlock, so it can be used also outside the block layer. - co_wrapper_mixed will be used by many blk_* functions, since the coroutine function needs to call blk_wait_while_drained() and the rdlock *must* be taken afterwards, otherwise it's a deadlock. In the future this annotation will go away, and blk_* will use co_wrapper directly. - co_wrapper_bdrv_rdlock will be used by BlockDriver callbacks, ideally by all of them in the future. - co_wrapper_mixed_bdrv_rdlock will be used by the remaining functions that are still called by coroutine and non-coroutine context. In the future this annotation will go away, as we will split such mixed functions. Signed-off-by: Emanuele Giuseppe Esposito Signed-off-by: Kevin Wolf Message-Id: <20221207131838.239125-17-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/block/block-common.h | 9 ++++++++- scripts/block-coroutine-wrapper.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) (limited to 'scripts/block-coroutine-wrapper.py') diff --git a/include/block/block-common.h b/include/block/block-common.h index 6cf603ab06..4749c46a5e 100644 --- a/include/block/block-common.h +++ b/include/block/block-common.h @@ -40,14 +40,21 @@ * * Usage: read docs/devel/block-coroutine-wrapper.rst * - * There are 2 kind of specifiers: + * There are 4 kind of specifiers: * - co_wrapper functions can be called by only non-coroutine context, because * they always generate a new coroutine. * - co_wrapper_mixed functions can be called by both coroutine and * non-coroutine context. + * - co_wrapper_bdrv_rdlock are co_wrapper functions but automatically take and + * release the graph rdlock when creating a new coroutine + * - co_wrapper_mixed_bdrv_rdlock are co_wrapper_mixed functions but + * automatically take and release the graph rdlock when creating a new + * coroutine. */ #define co_wrapper #define co_wrapper_mixed +#define co_wrapper_bdrv_rdlock +#define co_wrapper_mixed_bdrv_rdlock #include "block/dirty-bitmap.h" #include "block/blockjob.h" diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index 71a06e917a..6e087fa0b7 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -69,6 +69,7 @@ class FuncDecl: self.struct_name = snake_to_camel(self.name) self.args = [ParamDecl(arg.strip()) for arg in args.split(',')] self.create_only_co = 'mixed' not in variant + self.graph_rdlock = 'bdrv_rdlock' in variant subsystem, subname = self.name.split('_', 1) self.co_name = f'{subsystem}_co_{subname}' @@ -123,10 +124,13 @@ def create_mixed_wrapper(func: FuncDecl) -> str: """ name = func.co_name struct_name = func.struct_name + graph_assume_lock = 'assume_graph_lock();' if func.graph_rdlock else '' + return f"""\ {func.return_type} {func.name}({ func.gen_list('{decl}') }) {{ if (qemu_in_coroutine()) {{ + {graph_assume_lock} return {name}({ func.gen_list('{name}') }); }} else {{ {struct_name} s = {{ @@ -174,6 +178,12 @@ def gen_wrapper(func: FuncDecl) -> str: name = func.co_name struct_name = func.struct_name + graph_lock='' + graph_unlock='' + if func.graph_rdlock: + graph_lock=' bdrv_graph_co_rdlock();' + graph_unlock=' bdrv_graph_co_rdunlock();' + creation_function = create_mixed_wrapper if func.create_only_co: creation_function = create_co_wrapper @@ -193,7 +203,9 @@ static void coroutine_fn {name}_entry(void *opaque) {{ {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(); -- cgit 1.4.1