[PATCH v2] replay: synchronize on every virtual timer callback

Pavel Dovgalyuk posted 1 patch 3 years, 11 months ago
Test asan passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test docker-quick@centos7 failed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/159012932716.27256.8854065545365559921.stgit@pasha-ThinkPad-X280
0 files changed
[PATCH v2] replay: synchronize on every virtual timer callback
Posted by Pavel Dovgalyuk 3 years, 11 months ago
Sometimes virtual timer callbacks depend on order
of virtual timer processing and warping of virtual clock.
Therefore every callback should be logged to make replay deterministic.
This patch creates a checkpoint before every virtual timer callback.
With these checkpoints virtual timers processing and clock warping
events order is completely deterministic.

Signed-off-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>
Acked-by: Alex Bennée <alex.bennee@linaro.org>

--

v2:
  - remove mutex lock/unlock for virtual clock checkpoint since it is
    not process any asynchronous events (commit ca9759c2a92f528f256fef0e3922416f7bb47bf9)
  - bump record/replay log file version
---
 0 files changed

diff --git a/replay/replay.c b/replay/replay.c
index 706c7b4f4b..53edad1377 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              0xe02009
+#define REPLAY_VERSION              0xe0200a
 /* Size of replay log header */
 #define HEADER_SIZE                 (sizeof(uint32_t) + sizeof(uint64_t))
 
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index b6575a2cd5..f62b4feecd 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -501,7 +501,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
     bool progress = false;
     QEMUTimerCB *cb;
     void *opaque;
-    bool need_replay_checkpoint = false;
 
     if (!atomic_read(&timer_list->active_timers)) {
         return false;
@@ -517,16 +516,6 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
         break;
     default:
     case QEMU_CLOCK_VIRTUAL:
-        if (replay_mode != REPLAY_MODE_NONE) {
-            /* Checkpoint for virtual clock is redundant in cases where
-             * it's being triggered with only non-EXTERNAL timers, because
-             * these timers don't change guest state directly.
-             * Since it has conditional dependence on specific timers, it is
-             * subject to race conditions and requires special handling.
-             * See below.
-             */
-            need_replay_checkpoint = true;
-        }
         break;
     case QEMU_CLOCK_HOST:
         if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
@@ -559,19 +548,16 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
              */
             break;
         }
-        if (need_replay_checkpoint
-                && !(ts->attributes & QEMU_TIMER_ATTR_EXTERNAL)) {
-            /* once we got here, checkpoint clock only once */
-            need_replay_checkpoint = false;
+        /* Checkpoint for virtual clock is redundant in cases where
+         * it's being triggered with only non-EXTERNAL timers, because
+         * these timers don't change guest state directly.
+         */
+        if (replay_mode != REPLAY_MODE_NONE
+            && timer_list->clock->type == QEMU_CLOCK_VIRTUAL
+            && !(ts->attributes & QEMU_TIMER_ATTR_EXTERNAL)
+            && !replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
             qemu_mutex_unlock(&timer_list->active_timers_lock);
-            if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
-                goto out;
-            }
-            qemu_mutex_lock(&timer_list->active_timers_lock);
-            /* The lock was released; start over again in case the list was
-             * modified.
-             */
-            continue;
+            goto out;
         }
 
         /* remove timer from the list before calling the callback */