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

[Paolo Bonzini]

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.

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

Paolo

> 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)
>> +{
>> +}
> 
>