| Message ID | 20221116122241.2856527-6-eesposit@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Still more coroutine and various fixes in block layer | expand |
Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben: > Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the > functions that it uses are both using bdrv_get_aio_context, that > defaults to qemu_get_aio_context() if bs is NULL. > > Therefore pass NULL to BdrvPollCo to automatically generate a function > that create and runs a coroutine in the main loop. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> It happens to work, but it's kind of ugly to call bdrv_coroutine_enter() and BDRV_POLL_WHILE() with a NULL bs. How hard would it be to generate code that doesn't use these functions, but directly aio_co_enter() and AIO_WAIT_WHILE() for wrappers that are not related to a BDS? Kevin
Am 21/11/2022 um 16:30 schrieb Kevin Wolf: > Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben: >> Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the >> functions that it uses are both using bdrv_get_aio_context, that >> defaults to qemu_get_aio_context() if bs is NULL. >> >> Therefore pass NULL to BdrvPollCo to automatically generate a function >> that create and runs a coroutine in the main loop. >> >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > > It happens to work, but it's kind of ugly to call bdrv_coroutine_enter() > and BDRV_POLL_WHILE() with a NULL bs. > > How hard would it be to generate code that doesn't use these functions, > but directly aio_co_enter() and AIO_WAIT_WHILE() for wrappers that are > not related to a BDS? > At this point, I would get rid of s->poll_state.bs and instead use s->poll_state.aio_context. Then call directly aio_co_enter and AIO_WAIT_WHILE, as you suggested but just everywhere, without differentiating the cases. Then we would have something similar to what it is currently done with bs: if t == 'BlockDriverState *': bs = 'bdrv_get_aio_context(bs)' elif t == 'BdrvChild *': bs = 'bdrv_get_aio_context(child->bs)' elif t == 'BlockBackend *': bs = 'bdrv_get_aio_context(blk_bs(blk))' else: bs = 'qemu_get_aio_context()' I haven't tried it yet, but it should work. Thank you, Emanuele
Am 21.11.2022 um 16:52 hat Emanuele Giuseppe Esposito geschrieben: > Am 21/11/2022 um 16:30 schrieb Kevin Wolf: > > Am 16.11.2022 um 13:22 hat Emanuele Giuseppe Esposito geschrieben: > >> Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the > >> functions that it uses are both using bdrv_get_aio_context, that > >> defaults to qemu_get_aio_context() if bs is NULL. > >> > >> Therefore pass NULL to BdrvPollCo to automatically generate a function > >> that create and runs a coroutine in the main loop. > >> > >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > > > > It happens to work, but it's kind of ugly to call bdrv_coroutine_enter() > > and BDRV_POLL_WHILE() with a NULL bs. > > > > How hard would it be to generate code that doesn't use these functions, > > but directly aio_co_enter() and AIO_WAIT_WHILE() for wrappers that are > > not related to a BDS? > > > > At this point, I would get rid of s->poll_state.bs and instead use > s->poll_state.aio_context. Then call directly aio_co_enter and > AIO_WAIT_WHILE, as you suggested but just everywhere, without > differentiating the cases. Oh, yes, that's better. > Then we would have something similar to what it is currently done with bs: > > if t == 'BlockDriverState *': > bs = 'bdrv_get_aio_context(bs)' > elif t == 'BdrvChild *': > bs = 'bdrv_get_aio_context(child->bs)' > elif t == 'BlockBackend *': > bs = 'bdrv_get_aio_context(blk_bs(blk))' blk_get_aio_context(blk) seems more correct. > else: > bs = 'qemu_get_aio_context()' > > I haven't tried it yet, but it should work. Kevin
diff --git a/scripts/block-coroutine-wrapper.py b/scripts/block-coroutine-wrapper.py index f88ef53964..0f842386d4 100644 --- a/scripts/block-coroutine-wrapper.py +++ b/scripts/block-coroutine-wrapper.py @@ -152,8 +152,6 @@ def create_coroutine_only(func: FuncDecl) -> str: 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) @@ -165,8 +163,10 @@ def gen_wrapper(func: FuncDecl) -> str: bs = 'bs' elif t == 'BdrvChild *': bs = 'child->bs' - else: + elif t == 'BlockBackend *': bs = 'blk_bs(blk)' + else: + bs = 'NULL' func.bs = bs func.struct_name = snake_to_camel(func.name) struct_name = func.struct_name
Basically BdrvPollCo->bs is only used by bdrv_poll_co(), and the functions that it uses are both using bdrv_get_aio_context, that defaults to qemu_get_aio_context() if bs is NULL. Therefore pass NULL to BdrvPollCo to automatically generate a function that create and runs a coroutine in the main loop. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- scripts/block-coroutine-wrapper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)