Re: [Qemu-devel] [RFC PATCH v12 16/21]

[Pavel Dovgaluk]

> From: Paolo Bonzini [mailto:address@hidden
> On 05/05/2015 12:22, Pavel Dovgaluk wrote:
> > This patch is the reduced version of prior "bottom halves" patch.
> 
> dma-helpers.c is also related to block devices, so it's better not to
> change it now.

Ok.

> Perhaps you can add a replay event for ptimers instead of touching
> bottom halves?

It seems reasonable.
Then I'll have to use BH as a simple callback by implementing "run bh" function.
This BH will be added to replay queue instead of adding it to BH queue.

Pavel Dovgalyuk
> >
> >
> >> -----Original Message-----
> >> From: Pavel Dovgalyuk [mailto:address@hidden
> >> Sent: Tuesday, May 05, 2015 1:19 PM
> >> To: address@hidden
> >> Cc: address@hidden; address@hidden; address@hidden;
> >> address@hidden; address@hidden; address@hidden;
> address@hidden;
> >> address@hidden; address@hidden; address@hidden;
> >> address@hidden
> >> Subject: [RFC PATCH v12 16/21]
> >>
> >>
> >>
> >> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> >> ---
> >>  async.c                  |   24 +++++++++++++++++++++++-
> >>  dma-helpers.c            |    4 +++-
> >>  hw/timer/arm_timer.c     |    2 +-
> >>  include/block/aio.h      |   18 ++++++++++++++++++
> >>  include/qemu/main-loop.h |    1 +
> >>  main-loop.c              |    5 +++++
> >>  replay/replay-events.c   |   16 ++++++++++++++++
> >>  replay/replay-internal.h |    1 +
> >>  replay/replay.h          |    2 ++
> >>  stubs/replay.c           |    4 ++++
> >>  10 files changed, 74 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/async.c b/async.c
> >> index bd975c9..d092aa2 100644
> >> --- a/async.c
> >> +++ b/async.c
> >> @@ -27,6 +27,7 @@
> >>  #include "block/thread-pool.h"
> >>  #include "qemu/main-loop.h"
> >>  #include "qemu/atomic.h"
> >> +#include "replay/replay.h"
> >>
> >>  /***********************************************************/
> >>  /* bottom halves (can be seen as timers which expire ASAP) */
> >> @@ -39,6 +40,8 @@ struct QEMUBH {
> >>      bool scheduled;
> >>      bool idle;
> >>      bool deleted;
> >> +    bool replay;
> >> +    uint64_t id;
> >>  };
> >>
> >>  QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque)
> >> @@ -56,6 +59,21 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, 
> >> void *opaque)
> >>      return bh;
> >>  }
> >>
> >> +QEMUBH *aio_bh_new_replay(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
> >> +                          uint64_t id)
> >> +{
> >> +    QEMUBH *bh = aio_bh_new(ctx, cb, opaque);
> >> +    bh->replay = true;
> >> +    bh->id = id;
> >> +    return bh;
> >> +}
> >> +
> >> +void aio_bh_call(void *opaque)
> >> +{
> >> +    QEMUBH *bh = (QEMUBH *)opaque;
> >> +    bh->cb(bh->opaque);
> >> +}
> >> +
> >>  /* Multiple occurrences of aio_bh_poll cannot be called concurrently */
> >>  int aio_bh_poll(AioContext *ctx)
> >>  {
> >> @@ -78,7 +96,11 @@ int aio_bh_poll(AioContext *ctx)
> >>              if (!bh->idle)
> >>                  ret = 1;
> >>              bh->idle = 0;
> >> -            bh->cb(bh->opaque);
> >> +            if (!bh->replay) {
> >> +                aio_bh_call(bh);
> >> +            } else {
> >> +                replay_add_bh_event(bh, bh->id);
> >> +            }
> >>          }
> >>      }
> >>
> >> diff --git a/dma-helpers.c b/dma-helpers.c
> >> index 6918572..357d7e9 100644
> >> --- a/dma-helpers.c
> >> +++ b/dma-helpers.c
> >> @@ -13,6 +13,7 @@
> >>  #include "qemu/range.h"
> >>  #include "qemu/thread.h"
> >>  #include "qemu/main-loop.h"
> >> +#include "replay/replay.h"
> >>
> >>  /* #define DEBUG_IOMMU */
> >>
> >> @@ -96,7 +97,8 @@ static void continue_after_map_failure(void *opaque)
> >>  {
> >>      DMAAIOCB *dbs = (DMAAIOCB *)opaque;
> >>
> >> -    dbs->bh = qemu_bh_new(reschedule_dma, dbs);
> >> +    dbs->bh = qemu_bh_new_replay(reschedule_dma, dbs,
> >> +                                 replay_get_current_step());
> >>      qemu_bh_schedule(dbs->bh);
> >>  }
> >>
> >> diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c
> >> index 1452910..97784a0 100644
> >> --- a/hw/timer/arm_timer.c
> >> +++ b/hw/timer/arm_timer.c
> >> @@ -168,7 +168,7 @@ static arm_timer_state *arm_timer_init(uint32_t freq)
> >>      s->freq = freq;
> >>      s->control = TIMER_CTRL_IE;
> >>
> >> -    bh = qemu_bh_new(arm_timer_tick, s);
> >> +    bh = qemu_bh_new_replay(arm_timer_tick, s, 0);
> >>      s->timer = ptimer_init(bh);
> >>      vmstate_register(NULL, -1, &vmstate_arm_timer, s);
> >>      return s;
> >> diff --git a/include/block/aio.h b/include/block/aio.h
> >> index 82cdf78..ed76b43 100644
> >> --- a/include/block/aio.h
> >> +++ b/include/block/aio.h
> >> @@ -35,6 +35,8 @@ struct BlockAIOCB {
> >>      const AIOCBInfo *aiocb_info;
> >>      BlockDriverState *bs;
> >>      BlockCompletionFunc *cb;
> >> +    bool replay;
> >> +    uint64_t replay_step;
> >>      void *opaque;
> >>      int refcnt;
> >>  };
> >> @@ -144,6 +146,17 @@ void aio_context_release(AioContext *ctx);
> >>  QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque);
> >>
> >>  /**
> >> + * aio_bh_new_replay: Allocate a new bottom half structure for replay.
> >> + *
> >> + * This function calls aio_bh_new function and also fills replay 
> >> parameters
> >> + * of the BH structure. BH created with this function in record/replay 
> >> mode
> >> + * are executed through the replay queue only at checkpoints and 
> >> instructions
> >> + * executions.
> >> + */
> >> +QEMUBH *aio_bh_new_replay(AioContext *ctx, QEMUBHFunc *cb, void *opaque,
> >> +                          uint64_t id);
> >> +
> >> +/**
> >>   * aio_notify: Force processing of pending events.
> >>   *
> >>   * Similar to signaling a condition variable, aio_notify forces
> >> @@ -159,6 +172,11 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, 
> >> void *opaque);
> >>  void aio_notify(AioContext *ctx);
> >>
> >>  /**
> >> + * aio_bh_call: Executes callback function of the specified BH.
> >> + */
> >> +void aio_bh_call(void *opaque);
> >> +
> >> +/**
> >>   * aio_bh_poll: Poll bottom halves for an AioContext.
> >>   *
> >>   * These are internal functions used by the QEMU main loop.
> >> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> >> index 62c68c0..f5a98fe 100644
> >> --- a/include/qemu/main-loop.h
> >> +++ b/include/qemu/main-loop.h
> >> @@ -306,6 +306,7 @@ void qemu_iohandler_fill(GArray *pollfds);
> >>  void qemu_iohandler_poll(GArray *pollfds, int rc);
> >>
> >>  QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
> >> +QEMUBH *qemu_bh_new_replay(QEMUBHFunc *cb, void *opaque, uint64_t id);
> >>  void qemu_bh_schedule_idle(QEMUBH *bh);
> >>
> >>  #endif
> >> diff --git a/main-loop.c b/main-loop.c
> >> index 06aad06..87ebad4 100644
> >> --- a/main-loop.c
> >> +++ b/main-loop.c
> >> @@ -514,3 +514,8 @@ QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque)
> >>  {
> >>      return aio_bh_new(qemu_aio_context, cb, opaque);
> >>  }
> >> +
> >> +QEMUBH *qemu_bh_new_replay(QEMUBHFunc *cb, void *opaque, uint64_t id)
> >> +{
> >> +    return aio_bh_new_replay(qemu_aio_context, cb, opaque, id);
> >> +}
> >> diff --git a/replay/replay-events.c b/replay/replay-events.c
> >> index 409c9ad..ec15683 100755
> >> --- a/replay/replay-events.c
> >> +++ b/replay/replay-events.c
> >> @@ -35,6 +35,9 @@ static bool events_enabled = false;
> >>  static void replay_run_event(Event *event)
> >>  {
> >>      switch (event->event_kind) {
> >> +    case REPLAY_ASYNC_EVENT_BH:
> >> +        aio_bh_call(event->opaque);
> >> +        break;
> >>      default:
> >>          error_report("Replay: invalid async event ID (%d) in the queue",
> >>                      event->event_kind);
> >> @@ -133,10 +136,18 @@ static void replay_save_event(Event *event, int 
> >> checkpoint)
> >>
> >>          /* save event-specific data */
> >>          switch (event->event_kind) {
> >> +        case REPLAY_ASYNC_EVENT_BH:
> >> +            replay_put_qword(event->id);
> >> +            break;
> >>          }
> >>      }
> >>  }
> >>
> >> +void replay_add_bh_event(void *bh, uint64_t id)
> >> +{
> >> +    replay_add_event_internal(REPLAY_ASYNC_EVENT_BH, bh, NULL, id);
> >> +}
> >> +
> >>  /* Called with replay mutex locked */
> >>  void replay_save_events(int checkpoint)
> >>  {
> >> @@ -168,6 +179,11 @@ static Event *replay_read_event(int checkpoint)
> >>
> >>      /* Events that has not to be in the queue */
> >>      switch (read_event_kind) {
> >> +    case REPLAY_ASYNC_EVENT_BH:
> >> +        if (read_id == -1) {
> >> +            read_id = replay_get_qword();
> >> +        }
> >> +        break;
> >>      default:
> >>          error_report("Unknown ID %d of replay event", read_event_kind);
> >>          exit(1);
> >> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> >> index f758371..29722cf 100755
> >> --- a/replay/replay-internal.h
> >> +++ b/replay/replay-internal.h
> >> @@ -39,6 +39,7 @@ enum ReplayEvents {
> >>  /* Asynchronous events IDs */
> >>
> >>  enum ReplayAsyncEventKind {
> >> +    REPLAY_ASYNC_EVENT_BH,
> >>      REPLAY_ASYNC_COUNT
> >>  };
> >>
> >> diff --git a/replay/replay.h b/replay/replay.h
> >> index b6c4a8d..f83dc8e 100755
> >> --- a/replay/replay.h
> >> +++ b/replay/replay.h
> >> @@ -96,5 +96,7 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint);
> >>  void replay_disable_events(void);
> >>  /*! Returns true when saving events is enabled */
> >>  bool replay_events_enabled(void);
> >> +/*! Adds BH event to the queue */
> >> +void replay_add_bh_event(void *bh, uint64_t id);
> >>
> >>  #endif
> >> diff --git a/stubs/replay.c b/stubs/replay.c
> >> index 1be3575..268f3e0 100755
> >> --- a/stubs/replay.c
> >> +++ b/stubs/replay.c
> >> @@ -21,3 +21,7 @@ int runstate_is_running(void)
> >>  {
> >>      return 0;
> >>  }
> >> +
> >> +void replay_add_bh_event(void *bh, uint64_t id)
> >> +{
> >> +}
> >
> >