replay: rewrite async event handling

This patch decouples checkpoints and async events.
It was a tricky part of replay implementation. Now it becomes
much simpler and easier to maintain.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgalyuk@ispras.ru>
Acked-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <165364837856.688121.8785039478408995979.stgit@pasha-ThinkPad-X280>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

accel/tcg/tcg-accel-ops-icount.c

@@ -84,8 +84,7 @@ void icount_handle_deadline(void)
      * Don't interrupt cpu thread, when these events are waiting
      * (i.e., there is no checkpoint)
      */
-    if (deadline == 0
-        && (replay_mode != REPLAY_MODE_PLAY || replay_has_checkpoint())) {
+    if (deadline == 0) {
         icount_notify_aio_contexts();
     }
 }
@@ -109,7 +108,7 @@ void icount_prepare_for_run(CPUState *cpu)
 
     replay_mutex_lock();
 
-    if (cpu->icount_budget == 0 && replay_has_checkpoint()) {
+    if (cpu->icount_budget == 0) {
         icount_notify_aio_contexts();
     }
 }

docs/replay.txt

@@ -366,11 +366,9 @@ Here is the list of events that are written into the log:
    Argument: 4-byte number of executed instructions.
  - EVENT_INTERRUPT. Used to synchronize interrupt processing.
  - EVENT_EXCEPTION. Used to synchronize exception handling.
- - EVENT_ASYNC. This is a group of events. They are always processed
-   together with checkpoints. When such an event is generated, it is
-   stored in the queue and processed only when checkpoint occurs.
-   Every such event is followed by 1-byte checkpoint id and 1-byte
-   async event id from the following list:
+ - EVENT_ASYNC. This is a group of events. When such an event is generated,
+   it is stored in the queue and processed in icount_account_warp_timer().
+   Every such event has it's own id from the following list:
      - REPLAY_ASYNC_EVENT_BH. Bottom-half callback. This event synchronizes
        callbacks that affect virtual machine state, but normally called
        asynchronously.
@@ -405,6 +403,5 @@ Here is the list of events that are written into the log:
  - EVENT_CLOCK + clock_id. Group of events for host clock read operations.
    Argument: 8-byte clock value.
  - EVENT_CHECKPOINT + checkpoint_id. Checkpoint for synchronization of
-   CPU, internal threads, and asynchronous input events. May be followed
-   by one or more EVENT_ASYNC events.
+   CPU, internal threads, and asynchronous input events.
  - EVENT_END. Last event in the log.

include/sysemu/replay.h

@@ -160,9 +160,14 @@ void replay_shutdown_request(ShutdownCause cause);
     Returns 0 in PLAY mode if checkpoint was not found.
     Returns 1 in all other cases. */
 bool replay_checkpoint(ReplayCheckpoint checkpoint);
-/*! Used to determine that checkpoint is pending.
+/*! Used to determine that checkpoint or async event is pending.
     Does not proceed to the next event in the log. */
-bool replay_has_checkpoint(void);
+bool replay_has_event(void);
+/*
+ * Processes the async events added to the queue (while recording)
+ * or reads the events from the file (while replaying).
+ */
+void replay_async_events(void);
 
 /* Asynchronous events queue */
 

replay/replay-events.c

@@ -170,12 +170,11 @@ void replay_block_event(QEMUBH *bh, uint64_t id)
     }
 }
 
-static void replay_save_event(Event *event, int checkpoint)
+static void replay_save_event(Event *event)
 {
     if (replay_mode != REPLAY_MODE_PLAY) {
         /* put the event into the file */
         replay_put_event(EVENT_ASYNC);
-        replay_put_byte(checkpoint);
         replay_put_byte(event->event_kind);
 
         /* save event-specific data */
@@ -206,34 +205,27 @@ static void replay_save_event(Event *event, int checkpoint)
 }
 
 /* Called with replay mutex locked */
-void replay_save_events(int checkpoint)
+void replay_save_events(void)
 {
     g_assert(replay_mutex_locked());
-    g_assert(checkpoint != CHECKPOINT_CLOCK_WARP_START);
-    g_assert(checkpoint != CHECKPOINT_CLOCK_VIRTUAL);
     while (!QTAILQ_EMPTY(&events_list)) {
         Event *event = QTAILQ_FIRST(&events_list);
-        replay_save_event(event, checkpoint);
+        replay_save_event(event);
         replay_run_event(event);
         QTAILQ_REMOVE(&events_list, event, events);
         g_free(event);
     }
 }
 
-static Event *replay_read_event(int checkpoint)
+static Event *replay_read_event(void)
 {
     Event *event;
     if (replay_state.read_event_kind == -1) {
-        replay_state.read_event_checkpoint = replay_get_byte();
         replay_state.read_event_kind = replay_get_byte();
         replay_state.read_event_id = -1;
         replay_check_error();
     }
 
-    if (checkpoint != replay_state.read_event_checkpoint) {
-        return NULL;
-    }
-
     /* Events that has not to be in the queue */
     switch (replay_state.read_event_kind) {
     case REPLAY_ASYNC_EVENT_BH:
@@ -294,11 +286,11 @@ static Event *replay_read_event(int checkpoint)
 }
 
 /* Called with replay mutex locked */
-void replay_read_events(int checkpoint)
+void replay_read_events(void)
 {
     g_assert(replay_mutex_locked());
     while (replay_state.data_kind == EVENT_ASYNC) {
-        Event *event = replay_read_event(checkpoint);
+        Event *event = replay_read_event();
         if (!event) {
             break;
         }

replay/replay-internal.h

@@ -87,8 +87,6 @@ typedef struct ReplayState {
     int32_t read_event_kind;
     /*! Asynchronous event id read from the log */
     uint64_t read_event_id;
-    /*! Asynchronous event checkpoint id read from the log */
-    int32_t read_event_checkpoint;
 } ReplayState;
 extern ReplayState replay_state;
 
@@ -152,9 +150,9 @@ void replay_finish_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */
-void replay_save_events(int checkpoint);
+void replay_save_events(void);
 /*! Read events from the file into the input queue */
-void replay_read_events(int checkpoint);
+void replay_read_events(void);
 /*! Adds specified async event to the queue */
 void replay_add_event(ReplayAsyncEventKind event_kind, void *opaque,
                       void *opaque2, uint64_t id);

replay/replay-snapshot.c

@@ -61,7 +61,6 @@ static const VMStateDescription vmstate_replay = {
         VMSTATE_UINT64(block_request_id, ReplayState),
         VMSTATE_INT32(read_event_kind, ReplayState),
         VMSTATE_UINT64(read_event_id, ReplayState),
-        VMSTATE_INT32(read_event_checkpoint, ReplayState),
         VMSTATE_END_OF_LIST()
     },
 };

replay/replay.c

@@ -22,7 +22,7 @@
 
 /* Current version of the replay mechanism.
    Increase it when file format changes. */
-#define REPLAY_VERSION              0xe0200a
+#define REPLAY_VERSION              0xe0200b
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
@@ -171,64 +171,49 @@ void replay_shutdown_request(ShutdownCause cause)
 
 bool replay_checkpoint(ReplayCheckpoint checkpoint)
 {
-    bool res = false;
-    static bool in_checkpoint;
     assert(EVENT_CHECKPOINT + checkpoint <= EVENT_CHECKPOINT_LAST);
 
-    if (!replay_file) {
-        return true;
-    }
-
-    if (in_checkpoint) {
-        /*
-           Recursion occurs when HW event modifies timers.
-           Prevent performing icount warp in this case and
-           wait for another invocation of the checkpoint.
-        */
-        g_assert(replay_mode == REPLAY_MODE_PLAY);
-        return false;
-    }
-    in_checkpoint = true;
-
     replay_save_instructions();
 
     if (replay_mode == REPLAY_MODE_PLAY) {
         g_assert(replay_mutex_locked());
         if (replay_next_event_is(EVENT_CHECKPOINT + checkpoint)) {
             replay_finish_event();
-        } else if (replay_state.data_kind != EVENT_ASYNC) {
-            res = false;
-            goto out;
+        } else {
+            return false;
         }
-        replay_read_events(checkpoint);
-        /* replay_read_events may leave some unread events.
-           Return false if not all of the events associated with
-           checkpoint were processed */
-        res = replay_state.data_kind != EVENT_ASYNC;
     } else if (replay_mode == REPLAY_MODE_RECORD) {
         g_assert(replay_mutex_locked());
         replay_put_event(EVENT_CHECKPOINT + checkpoint);
-        /* This checkpoint belongs to several threads.
-           Processing events from different threads is
-           non-deterministic */
-        if (checkpoint != CHECKPOINT_CLOCK_WARP_START
-            /* FIXME: this is temporary fix, other checkpoints
-                      may also be invoked from the different threads someday.
-                      Asynchronous event processing should be refactored
-                      to create additional replay event kind which is
-                      nailed to the one of the threads and which processes
-                      the event queue. */
-            && checkpoint != CHECKPOINT_CLOCK_VIRTUAL) {
-            replay_save_events(checkpoint);
-        }
-        res = true;
     }
-out:
-    in_checkpoint = false;
-    return res;
+    return true;
+}
+
+void replay_async_events(void)
+{
+    static bool processing = false;
+    /*
+     * If we are already processing the events, recursion may occur
+     * in case of incorrect implementation when HW event modifies timers.
+     * Timer modification may invoke the icount warp, event processing,
+     * and cause the recursion.
+     */
+    g_assert(!processing);
+    processing = true;
+
+    replay_save_instructions();
+
+    if (replay_mode == REPLAY_MODE_PLAY) {
+        g_assert(replay_mutex_locked());
+        replay_read_events();
+    } else if (replay_mode == REPLAY_MODE_RECORD) {
+        g_assert(replay_mutex_locked());
+        replay_save_events();
+    }
+    processing = false;
 }
 
-bool replay_has_checkpoint(void)
+bool replay_has_event(void)
 {
     bool res = false;
     if (replay_mode == REPLAY_MODE_PLAY) {
@@ -236,6 +221,7 @@ bool replay_has_checkpoint(void)
         replay_account_executed_instructions();
         res = EVENT_CHECKPOINT <= replay_state.data_kind
               && replay_state.data_kind <= EVENT_CHECKPOINT_LAST;
+        res = res || replay_state.data_kind == EVENT_ASYNC;
     }
     return res;
 }

softmmu/icount.c

@@ -322,7 +322,7 @@ void icount_start_warp_timer(void)
              * to vCPU was processed in advance and vCPU went to sleep.
              * Therefore we have to wake it up for doing someting.
              */
-            if (replay_has_checkpoint()) {
+            if (replay_has_event()) {
                 qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
             }
             return;
@@ -404,6 +404,8 @@ void icount_account_warp_timer(void)
         return;
     }
 
+    replay_async_events();
+
     /* warp clock deterministically in record/replay mode */
     if (!replay_checkpoint(CHECKPOINT_CLOCK_WARP_ACCOUNT)) {
         return;