Re: [Qemu-devel] [PATCH v13 19/25] replay: add BH oneshot event for block layer

[Kevin Wolf]

Am 21.02.2019 um 12:05 hat Pavel Dovgalyuk geschrieben:
> Replay is capable of recording normal BH events, but sometimes
> there are single use callbacks scheduled with aio_bh_schedule_oneshot
> function. This patch enables recording and replaying such callbacks.
> Block layer uses these events for calling the completion function.
> Replaying these calls makes the execution deterministic.
> 
> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> 
> --
> 
> v6:
>  - moved stub function to the separate file for fixing linux-user build
> v10:
>  - replaced all block layer aio_bh_schedule_oneshot calls
> ---
>  block/block-backend.c    |    8 +++++---
>  block/io.c               |    4 ++--
>  block/iscsi.c            |    5 +++--
>  block/nfs.c              |    5 +++--
>  block/null.c             |    4 +++-
>  block/nvme.c             |    6 ++++--
>  block/rbd.c              |    5 +++--
>  block/vxhs.c             |    5 +++--
>  include/sysemu/replay.h  |    3 +++
>  replay/replay-events.c   |   16 ++++++++++++++++
>  replay/replay-internal.h |    1 +
>  replay/replay.c          |    2 +-
>  stubs/Makefile.objs      |    1 +
>  stubs/replay-user.c      |    9 +++++++++
>  14 files changed, 57 insertions(+), 17 deletions(-)
>  create mode 100644 stubs/replay-user.c

This still doesn't catch all instances, e.g. everything that goes
through aio_co_schedule() is missing.

But I fully expect this to get broken anyway all the time because nobody
understands which function to use, and if it works for your special case
now and we'll fix other stuff as you encouter it, maybe that's good
enough for you.

> @@ -1349,8 +1351,8 @@ static BlockAIOCB *blk_aio_prwv(BlockBackend *blk, 
> int64_t offset, int bytes,
>  
>      acb->has_returned = true;
>      if (acb->rwco.ret != NOT_DONE) {
> -        aio_bh_schedule_oneshot(blk_get_aio_context(blk),
> -                                blk_aio_complete_bh, acb);
> +        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
> +                                         blk_aio_complete_bh, acb);
>      }

This, and a few other places that you convert, are in fast paths and add
some calls that are unnecessary for non-replay cases.

I wonder if we could make replay optional in ./configure and then make
replay_bh_schedule_oneshot_event() a static inline function that can get
optimised away at compile time if the feature is disabled.

Kevin