Skip to content

Commit

Permalink
block-coroutine-wrapper.py: introduce annotations that take the graph…
Browse files Browse the repository at this point in the history
… 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 <eesposit@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20221207131838.239125-17-kwolf@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
  • Loading branch information
esposem authored and kevmw committed Dec 15, 2022
1 parent 303de47 commit e6d3f7a
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
9 changes: 8 additions & 1 deletion include/block/block-common.h
Expand Up @@ -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"
Expand Down
12 changes: 12 additions & 0 deletions scripts/block-coroutine-wrapper.py
Expand Up @@ -69,6 +69,7 @@ def __init__(self, return_type: str, name: str, args: str,
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}'
Expand Down Expand Up @@ -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 = {{
Expand Down Expand Up @@ -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
Expand All @@ -193,7 +203,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();
Expand Down

0 comments on commit e6d3f7a

Please sign in to comment.