diff mbox series

[v5,23/24] replay: add BH oneshot event for block layer

Message ID 20180725121701.12867.49392.stgit@pasha-VirtualBox
State New
Headers show
Series Fixing record/replay and adding reverse debugging | expand

Commit Message

Pavel Dovgalyuk July 25, 2018, 12:17 p.m. UTC
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 <Pavel.Dovgaluk@ispras.ru>
---
 block/block-backend.c    |    3 ++-
 include/sysemu/replay.h  |    3 +++
 replay/replay-events.c   |   16 ++++++++++++++++
 replay/replay-internal.h |    1 +
 replay/replay.c          |    2 +-
 stubs/replay.c           |    6 ++++++
 6 files changed, 29 insertions(+), 2 deletions(-)

Comments

Alex Bennée July 26, 2018, 3:36 p.m. UTC | #1
Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> 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 <Pavel.Dovgaluk@ispras.ru>

I'm not sure what about this commit causes the compile breakage I'm
seeing:

  LINK    aarch64-linux-user/qemu-aarch64
../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
exec.o:(.bss+0x58): first defined here
collect2: error: ld returned 1 exit status
Makefile:199: recipe for target 'qemu-aarch64' failed
make[1]: *** [qemu-aarch64] Error 1
Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
make: *** [subdir-aarch64-linux-user] Error 2

It only occurs on a make clean && make -j on that commit though. It's
hidden if you do incremental builds.



> ---
>  block/block-backend.c    |    3 ++-
>  include/sysemu/replay.h  |    3 +++
>  replay/replay-events.c   |   16 ++++++++++++++++
>  replay/replay-internal.h |    1 +
>  replay/replay.c          |    2 +-
>  stubs/replay.c           |    6 ++++++
>  6 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index f2f75a9..232d114 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -17,6 +17,7 @@
>  #include "block/throttle-groups.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/replay.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-block.h"
>  #include "qemu/id.h"
> @@ -1370,7 +1371,7 @@ 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),
> +        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
>                                  blk_aio_complete_bh, acb);
>      }
>
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 8118b00..945bc74 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -152,6 +152,9 @@ bool replay_events_enabled(void);
>  void replay_flush_events(void);
>  /*! Adds bottom half event to the queue */
>  void replay_bh_schedule_event(QEMUBH *bh);
> +/*! Adds oneshot bottom half event to the queue */
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb, void *opaque);
>  /*! Adds input event to the queue */
>  void replay_input_event(QemuConsole *src, InputEvent *evt);
>  /*! Adds input sync event to the queue */
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 0964a82..0ac8a5c 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -37,6 +37,9 @@ static void replay_run_event(Event *event)
>      case REPLAY_ASYNC_EVENT_BH:
>          aio_bh_call(event->opaque);
>          break;
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> +        ((QEMUBHFunc*)event->opaque)(event->opaque2);
> +        break;
>      case REPLAY_ASYNC_EVENT_INPUT:
>          qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
>          qapi_free_InputEvent((InputEvent *)event->opaque);
> @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh)
>      }
>  }
>
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    if (events_enabled) {
> +        uint64_t id = replay_get_current_step();
> +        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> +    } else {
> +        aio_bh_schedule_oneshot(ctx, cb, opaque);
> +    }
> +}
> +
>  void replay_add_input_event(struct InputEvent *event)
>  {
>      replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
> @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint)
>          /* save event-specific data */
>          switch (event->event_kind) {
>          case REPLAY_ASYNC_EVENT_BH:
> +        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>              replay_put_qword(event->id);
>              break;
>          case REPLAY_ASYNC_EVENT_INPUT:
> @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint)
>      /* Events that has not to be in the queue */
>      switch (replay_state.read_event_kind) {
>      case REPLAY_ASYNC_EVENT_BH:
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>          if (replay_state.read_event_id == -1) {
>              replay_state.read_event_id = replay_get_qword();
>          }
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 08ef2ec..0c0ed16 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -51,6 +51,7 @@ enum ReplayEvents {
>
>  enum ReplayAsyncEventKind {
>      REPLAY_ASYNC_EVENT_BH,
> +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
>      REPLAY_ASYNC_EVENT_INPUT,
>      REPLAY_ASYNC_EVENT_INPUT_SYNC,
>      REPLAY_ASYNC_EVENT_CHAR_READ,
> diff --git a/replay/replay.c b/replay/replay.c
> index 6e82764..061b1e2 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -22,7 +22,7 @@
>
>  /* Current version of the replay mechanism.
>     Increase it when file format changes. */
> -#define REPLAY_VERSION              0xe02007
> +#define REPLAY_VERSION              0xe02008
>  /* Size of replay log header */
>  #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
>
> diff --git a/stubs/replay.c b/stubs/replay.c
> index 781974e..cbdac80 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -90,3 +90,9 @@ bool replay_reverse_continue(void)
>  {
>      return false;
>  }
> +
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    aio_bh_schedule_oneshot(ctx, cb, opaque);
> +}


--
Alex Bennée
Alex Bennée July 26, 2018, 5:17 p.m. UTC | #2
Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:

> 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 <Pavel.Dovgaluk@ispras.ru>
> ---
>  block/block-backend.c    |    3 ++-
>  include/sysemu/replay.h  |    3 +++
>  replay/replay-events.c   |   16 ++++++++++++++++
>  replay/replay-internal.h |    1 +
>  replay/replay.c          |    2 +-
>  stubs/replay.c           |    6 ++++++
>  6 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index f2f75a9..232d114 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -17,6 +17,7 @@
>  #include "block/throttle-groups.h"
>  #include "sysemu/blockdev.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/replay.h"
>  #include "qapi/error.h"
>  #include "qapi/qapi-events-block.h"
>  #include "qemu/id.h"
> @@ -1370,7 +1371,7 @@ 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),
> +        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
>                                  blk_aio_complete_bh, acb);
>      }
>
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 8118b00..945bc74 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -152,6 +152,9 @@ bool replay_events_enabled(void);
>  void replay_flush_events(void);
>  /*! Adds bottom half event to the queue */
>  void replay_bh_schedule_event(QEMUBH *bh);
> +/*! Adds oneshot bottom half event to the queue */
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb, void *opaque);
>  /*! Adds input event to the queue */
>  void replay_input_event(QemuConsole *src, InputEvent *evt);
>  /*! Adds input sync event to the queue */
> diff --git a/replay/replay-events.c b/replay/replay-events.c
> index 0964a82..0ac8a5c 100644
> --- a/replay/replay-events.c
> +++ b/replay/replay-events.c
> @@ -37,6 +37,9 @@ static void replay_run_event(Event *event)
>      case REPLAY_ASYNC_EVENT_BH:
>          aio_bh_call(event->opaque);
>          break;
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> +        ((QEMUBHFunc*)event->opaque)(event->opaque2);
> +        break;
>      case REPLAY_ASYNC_EVENT_INPUT:
>          qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
>          qapi_free_InputEvent((InputEvent *)event->opaque);
> @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh)
>      }
>  }
>
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    if (events_enabled) {
> +        uint64_t id = replay_get_current_step();
> +        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> +    } else {
> +        aio_bh_schedule_oneshot(ctx, cb, opaque);
> +    }
> +}
> +
>  void replay_add_input_event(struct InputEvent *event)
>  {
>      replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
> @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint)
>          /* save event-specific data */
>          switch (event->event_kind) {
>          case REPLAY_ASYNC_EVENT_BH:
> +        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>              replay_put_qword(event->id);
>              break;
>          case REPLAY_ASYNC_EVENT_INPUT:
> @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint)
>      /* Events that has not to be in the queue */
>      switch (replay_state.read_event_kind) {
>      case REPLAY_ASYNC_EVENT_BH:
> +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
>          if (replay_state.read_event_id == -1) {
>              replay_state.read_event_id = replay_get_qword();
>          }
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 08ef2ec..0c0ed16 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -51,6 +51,7 @@ enum ReplayEvents {
>
>  enum ReplayAsyncEventKind {
>      REPLAY_ASYNC_EVENT_BH,
> +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
>      REPLAY_ASYNC_EVENT_INPUT,
>      REPLAY_ASYNC_EVENT_INPUT_SYNC,
>      REPLAY_ASYNC_EVENT_CHAR_READ,
> diff --git a/replay/replay.c b/replay/replay.c
> index 6e82764..061b1e2 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -22,7 +22,7 @@
>
>  /* Current version of the replay mechanism.
>     Increase it when file format changes. */
> -#define REPLAY_VERSION              0xe02007
> +#define REPLAY_VERSION              0xe02008
>  /* Size of replay log header */
>  #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
>
> diff --git a/stubs/replay.c b/stubs/replay.c
> index 781974e..cbdac80 100644
> --- a/stubs/replay.c
> +++ b/stubs/replay.c
> @@ -90,3 +90,9 @@ bool replay_reverse_continue(void)
>  {
>      return false;
>  }
> +
> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> +    QEMUBHFunc *cb,void *opaque)
> +{
> +    aio_bh_schedule_oneshot(ctx, cb, opaque);
> +}

It seems wrong to have something in stubs that actively does stuff.
Isn't this meant to be a bunch of NOPs?

--
Alex Bennée
Alex Bennée July 26, 2018, 5:20 p.m. UTC | #3
Alex Bennée <alex.bennee@linaro.org> writes:

> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:
>
>> 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 <Pavel.Dovgaluk@ispras.ru>
>
> I'm not sure what about this commit causes the compile breakage I'm
> seeing:
>
>   LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2
>
> It only occurs on a make clean && make -j on that commit though. It's
> hidden if you do incremental builds.

And it seems to be the same failure across all the cross builds:

  https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console

--
Alex Bennée
Pavel Dovgalyuk July 27, 2018, 4:52 a.m. UTC | #4
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> > 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 <Pavel.Dovgaluk@ispras.ru>
> 
> I'm not sure what about this commit causes the compile breakage I'm
> seeing:
> 
>   LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2
> 
> It only occurs on a make clean && make -j on that commit though. It's
> hidden if you do incremental builds.

make distclean works for me in such cases.

Pavel Dovgalyuk
Alex Bennée July 27, 2018, 4:44 p.m. UTC | #5
Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:

>> From: Alex Bennée [mailto:alex.bennee@linaro.org]
>> > 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 <Pavel.Dovgaluk@ispras.ru>
>>
>> I'm not sure what about this commit causes the compile breakage I'm
>> seeing:
>>
>>   LINK    aarch64-linux-user/qemu-aarch64
>> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
>> exec.o:(.bss+0x58): first defined here
>> collect2: error: ld returned 1 exit status
>> Makefile:199: recipe for target 'qemu-aarch64' failed
>> make[1]: *** [qemu-aarch64] Error 1
>> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
>> make: *** [subdir-aarch64-linux-user] Error 2
>>
>> It only occurs on a make clean && make -j on that commit though. It's
>> hidden if you do incremental builds.
>
> make distclean works for me in such cases.

Hmmm

make distclean
./configure
make

And I get:

LINK    aarch64-linux-user/qemu-aarch64
../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
exec.o:(.bss+0x58): first defined here
collect2: error: ld returned 1 exit status
Makefile:199: recipe for target 'qemu-aarch64' failed
make[1]: *** [qemu-aarch64] Error 1
Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
make: *** [subdir-aarch64-linux-user] Error 2

The CI builds all look pretty broken too:

  https://travis-ci.org/stsquad/qemu/builds/408537385
  https://app.shippable.com/github/stsquad/qemu/runs/538/summary/console


>
> Pavel Dovgalyuk


--
Alex Bennée
Pavel Dovgalyuk July 31, 2018, 7 a.m. UTC | #6
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru> writes:
> 
> > 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 <Pavel.Dovgaluk@ispras.ru>
> > ---
> >  block/block-backend.c    |    3 ++-
> >  include/sysemu/replay.h  |    3 +++
> >  replay/replay-events.c   |   16 ++++++++++++++++
> >  replay/replay-internal.h |    1 +
> >  replay/replay.c          |    2 +-
> >  stubs/replay.c           |    6 ++++++
> >  6 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/block-backend.c b/block/block-backend.c
> > index f2f75a9..232d114 100644
> > --- a/block/block-backend.c
> > +++ b/block/block-backend.c
> > @@ -17,6 +17,7 @@
> >  #include "block/throttle-groups.h"
> >  #include "sysemu/blockdev.h"
> >  #include "sysemu/sysemu.h"
> > +#include "sysemu/replay.h"
> >  #include "qapi/error.h"
> >  #include "qapi/qapi-events-block.h"
> >  #include "qemu/id.h"
> > @@ -1370,7 +1371,7 @@ 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),
> > +        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
> >                                  blk_aio_complete_bh, acb);
> >      }
> >
> > diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> > index 8118b00..945bc74 100644
> > --- a/include/sysemu/replay.h
> > +++ b/include/sysemu/replay.h
> > @@ -152,6 +152,9 @@ bool replay_events_enabled(void);
> >  void replay_flush_events(void);
> >  /*! Adds bottom half event to the queue */
> >  void replay_bh_schedule_event(QEMUBH *bh);
> > +/*! Adds oneshot bottom half event to the queue */
> > +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> > +    QEMUBHFunc *cb, void *opaque);
> >  /*! Adds input event to the queue */
> >  void replay_input_event(QemuConsole *src, InputEvent *evt);
> >  /*! Adds input sync event to the queue */
> > diff --git a/replay/replay-events.c b/replay/replay-events.c
> > index 0964a82..0ac8a5c 100644
> > --- a/replay/replay-events.c
> > +++ b/replay/replay-events.c
> > @@ -37,6 +37,9 @@ static void replay_run_event(Event *event)
> >      case REPLAY_ASYNC_EVENT_BH:
> >          aio_bh_call(event->opaque);
> >          break;
> > +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> > +        ((QEMUBHFunc*)event->opaque)(event->opaque2);
> > +        break;
> >      case REPLAY_ASYNC_EVENT_INPUT:
> >          qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
> >          qapi_free_InputEvent((InputEvent *)event->opaque);
> > @@ -132,6 +135,17 @@ void replay_bh_schedule_event(QEMUBH *bh)
> >      }
> >  }
> >
> > +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> > +    QEMUBHFunc *cb,void *opaque)
> > +{
> > +    if (events_enabled) {
> > +        uint64_t id = replay_get_current_step();
> > +        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
> > +    } else {
> > +        aio_bh_schedule_oneshot(ctx, cb, opaque);
> > +    }
> > +}
> > +
> >  void replay_add_input_event(struct InputEvent *event)
> >  {
> >      replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
> > @@ -162,6 +176,7 @@ static void replay_save_event(Event *event, int checkpoint)
> >          /* save event-specific data */
> >          switch (event->event_kind) {
> >          case REPLAY_ASYNC_EVENT_BH:
> > +        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> >              replay_put_qword(event->id);
> >              break;
> >          case REPLAY_ASYNC_EVENT_INPUT:
> > @@ -216,6 +231,7 @@ static Event *replay_read_event(int checkpoint)
> >      /* Events that has not to be in the queue */
> >      switch (replay_state.read_event_kind) {
> >      case REPLAY_ASYNC_EVENT_BH:
> > +    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
> >          if (replay_state.read_event_id == -1) {
> >              replay_state.read_event_id = replay_get_qword();
> >          }
> > diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> > index 08ef2ec..0c0ed16 100644
> > --- a/replay/replay-internal.h
> > +++ b/replay/replay-internal.h
> > @@ -51,6 +51,7 @@ enum ReplayEvents {
> >
> >  enum ReplayAsyncEventKind {
> >      REPLAY_ASYNC_EVENT_BH,
> > +    REPLAY_ASYNC_EVENT_BH_ONESHOT,
> >      REPLAY_ASYNC_EVENT_INPUT,
> >      REPLAY_ASYNC_EVENT_INPUT_SYNC,
> >      REPLAY_ASYNC_EVENT_CHAR_READ,
> > diff --git a/replay/replay.c b/replay/replay.c
> > index 6e82764..061b1e2 100644
> > --- a/replay/replay.c
> > +++ b/replay/replay.c
> > @@ -22,7 +22,7 @@
> >
> >  /* Current version of the replay mechanism.
> >     Increase it when file format changes. */
> > -#define REPLAY_VERSION              0xe02007
> > +#define REPLAY_VERSION              0xe02008
> >  /* Size of replay log header */
> >  #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
> >
> > diff --git a/stubs/replay.c b/stubs/replay.c
> > index 781974e..cbdac80 100644
> > --- a/stubs/replay.c
> > +++ b/stubs/replay.c
> > @@ -90,3 +90,9 @@ bool replay_reverse_continue(void)
> >  {
> >      return false;
> >  }
> > +
> > +void replay_bh_schedule_oneshot_event(AioContext *ctx,
> > +    QEMUBHFunc *cb,void *opaque)
> > +{
> > +    aio_bh_schedule_oneshot(ctx, cb, opaque);
> > +}
> 
> It seems wrong to have something in stubs that actively does stuff.
> Isn't this meant to be a bunch of NOPs?

I thinks stubs are meant to be functions that are used by qemu-img and other tools.
As I replaced aio_bh_schedule_oneshot with replay_bh_schedule_oneshot_event,
the tools should not notice this. Therefore the stub performs this call.

Pavel Dovgalyuk
Paolo Bonzini Aug. 1, 2018, 4:54 p.m. UTC | #7
On 26/07/2018 19:17, Alex Bennée wrote:
>> +
>> +void replay_bh_schedule_oneshot_event(AioContext *ctx,
>> +    QEMUBHFunc *cb,void *opaque)
>> +{
>> +    aio_bh_schedule_oneshot(ctx, cb, opaque);
>> +}
> It seems wrong to have something in stubs that actively does stuff.
> Isn't this meant to be a bunch of NOPs?

No, not necessarily, for example

int64_t cpu_get_clock(void)
{
    return get_clock_realtime();
}

or even

void error_vprintf(const char *fmt, va_list ap)
{
    if (g_test_initialized() && !g_test_subprocess() &&
        getenv("QTEST_SILENT_ERRORS")) {
        char *msg = g_strdup_vprintf(fmt, ap);
        g_test_message("%s", msg);
        g_free(msg);
    } else {
        vfprintf(stderr, fmt, ap);
    }
}

Paolo
Pavel Dovgalyuk Aug. 2, 2018, 5:50 a.m. UTC | #8
> -----Original Message-----
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Sent: Friday, July 27, 2018 7:45 PM
> To: Pavel Dovgalyuk
> Cc: 'Pavel Dovgalyuk'; qemu-devel@nongnu.org; kwolf@redhat.com; peter.maydell@linaro.org;
> war2jordan@live.com; crosthwaite.peter@gmail.com; boost.lists@gmail.com; quintela@redhat.com;
> ciro.santilli@gmail.com; jasowang@redhat.com; mst@redhat.com; zuban32s@gmail.com;
> armbru@redhat.com; maria.klimushenkova@ispras.ru; kraxel@redhat.com;
> thomas.dullien@googlemail.com; pbonzini@redhat.com; mreitz@redhat.com; dgilbert@redhat.com;
> rth@twiddle.net
> Subject: Re: [PATCH v5 23/24] replay: add BH oneshot event for block layer
> 
> 
> Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:
> 
> >> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> >> > 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 <Pavel.Dovgaluk@ispras.ru>
> >>
> >> I'm not sure what about this commit causes the compile breakage I'm
> >> seeing:
> >>
> >>   LINK    aarch64-linux-user/qemu-aarch64
> >> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> >> exec.o:(.bss+0x58): first defined here
> >> collect2: error: ld returned 1 exit status
> >> Makefile:199: recipe for target 'qemu-aarch64' failed
> >> make[1]: *** [qemu-aarch64] Error 1
> >> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> >> make: *** [subdir-aarch64-linux-user] Error 2
> >>
> >> It only occurs on a make clean && make -j on that commit though. It's
> >> hidden if you do incremental builds.
> >
> > make distclean works for me in such cases.
> 
> Hmmm
> 
> make distclean
> ./configure
> make
> 
> And I get:
> 
> LINK    aarch64-linux-user/qemu-aarch64
> ../libqemuutil.a(cpu-get-icount.o):(.bss+0x0): multiple definition of `use_icount'
> exec.o:(.bss+0x58): first defined here
> collect2: error: ld returned 1 exit status
> Makefile:199: recipe for target 'qemu-aarch64' failed
> make[1]: *** [qemu-aarch64] Error 1
> Makefile:481: recipe for target 'subdir-aarch64-linux-user' failed
> make: *** [subdir-aarch64-linux-user] Error 2

That's strange. As I understand, linux-user emulators does not include block layer.
They also don't include record/replay.
Why then block- and record-related stub affects the build?

Pavel Dovgalyuk
diff mbox series

Patch

diff --git a/block/block-backend.c b/block/block-backend.c
index f2f75a9..232d114 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -17,6 +17,7 @@ 
 #include "block/throttle-groups.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/replay.h"
 #include "qapi/error.h"
 #include "qapi/qapi-events-block.h"
 #include "qemu/id.h"
@@ -1370,7 +1371,7 @@  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),
+        replay_bh_schedule_oneshot_event(blk_get_aio_context(blk),
                                 blk_aio_complete_bh, acb);
     }
 
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 8118b00..945bc74 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -152,6 +152,9 @@  bool replay_events_enabled(void);
 void replay_flush_events(void);
 /*! Adds bottom half event to the queue */
 void replay_bh_schedule_event(QEMUBH *bh);
+/*! Adds oneshot bottom half event to the queue */
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb, void *opaque);
 /*! Adds input event to the queue */
 void replay_input_event(QemuConsole *src, InputEvent *evt);
 /*! Adds input sync event to the queue */
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 0964a82..0ac8a5c 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -37,6 +37,9 @@  static void replay_run_event(Event *event)
     case REPLAY_ASYNC_EVENT_BH:
         aio_bh_call(event->opaque);
         break;
+    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
+        ((QEMUBHFunc*)event->opaque)(event->opaque2);
+        break;
     case REPLAY_ASYNC_EVENT_INPUT:
         qemu_input_event_send_impl(NULL, (InputEvent *)event->opaque);
         qapi_free_InputEvent((InputEvent *)event->opaque);
@@ -132,6 +135,17 @@  void replay_bh_schedule_event(QEMUBH *bh)
     }
 }
 
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb,void *opaque)
+{
+    if (events_enabled) {
+        uint64_t id = replay_get_current_step();
+        replay_add_event(REPLAY_ASYNC_EVENT_BH_ONESHOT, cb, opaque, id);
+    } else {
+        aio_bh_schedule_oneshot(ctx, cb, opaque);
+    }
+}
+
 void replay_add_input_event(struct InputEvent *event)
 {
     replay_add_event(REPLAY_ASYNC_EVENT_INPUT, event, NULL, 0);
@@ -162,6 +176,7 @@  static void replay_save_event(Event *event, int checkpoint)
         /* save event-specific data */
         switch (event->event_kind) {
         case REPLAY_ASYNC_EVENT_BH:
+        case REPLAY_ASYNC_EVENT_BH_ONESHOT:
             replay_put_qword(event->id);
             break;
         case REPLAY_ASYNC_EVENT_INPUT:
@@ -216,6 +231,7 @@  static Event *replay_read_event(int checkpoint)
     /* Events that has not to be in the queue */
     switch (replay_state.read_event_kind) {
     case REPLAY_ASYNC_EVENT_BH:
+    case REPLAY_ASYNC_EVENT_BH_ONESHOT:
         if (replay_state.read_event_id == -1) {
             replay_state.read_event_id = replay_get_qword();
         }
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 08ef2ec..0c0ed16 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -51,6 +51,7 @@  enum ReplayEvents {
 
 enum ReplayAsyncEventKind {
     REPLAY_ASYNC_EVENT_BH,
+    REPLAY_ASYNC_EVENT_BH_ONESHOT,
     REPLAY_ASYNC_EVENT_INPUT,
     REPLAY_ASYNC_EVENT_INPUT_SYNC,
     REPLAY_ASYNC_EVENT_CHAR_READ,
diff --git a/replay/replay.c b/replay/replay.c
index 6e82764..061b1e2 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -22,7 +22,7 @@ 
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe02007
+#define REPLAY_VERSION              0xe02008
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
diff --git a/stubs/replay.c b/stubs/replay.c
index 781974e..cbdac80 100644
--- a/stubs/replay.c
+++ b/stubs/replay.c
@@ -90,3 +90,9 @@  bool replay_reverse_continue(void)
 {
     return false;
 }
+
+void replay_bh_schedule_oneshot_event(AioContext *ctx,
+    QEMUBHFunc *cb,void *opaque)
+{
+    aio_bh_schedule_oneshot(ctx, cb, opaque);
+}