[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 4/4] Optimize record/replay checkpointing for
From: |
Pavel Dovgalyuk |
Subject: |
Re: [Qemu-devel] [PATCH v3 4/4] Optimize record/replay checkpointing for all clocks it applies to |
Date: |
Fri, 19 Oct 2018 08:55:09 +0300 |
> From: Artem Pisarenko [mailto:address@hidden
> Removes redundant checkpoints in replay log when there are no expired timers
> in timers list,
> associated with corresponding clock (i.e. no rr events associated with
> current clock value).
> This also improves performance in rr mode.
>
> Signed-off-by: Artem Pisarenko <address@hidden>
> ---
>
> Notes:
> v3:
> - fixed compiler warning caused non-debug build to fail
>
> include/qemu/timer.h | 2 +-
> util/qemu-timer.c | 62
> +++++++++++++++++++++++++---------------------------
> 2 files changed, 31 insertions(+), 33 deletions(-)
>
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index dc0fd14..bff8dac 100644
> --- a/include/qemu/timer.h
> +++ b/include/qemu/timer.h
> @@ -65,7 +65,7 @@ typedef enum {
> * QEMU_TIMER_ATTR_EXTERNAL: drives external subsystem
> *
> * Timers with this attribute do not recorded in rr mode, therefore it could
> be
> - * used for the subsystems that operate outside the guest core. Applicable
> only
> + * used for the subsystems that operate outside the guest core. Relevant only
> * with virtual clock type.
> */
>
> diff --git a/util/qemu-timer.c b/util/qemu-timer.c
> index e2746cf..216d107 100644
> --- a/util/qemu-timer.c
> +++ b/util/qemu-timer.c
> @@ -490,6 +490,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
> QEMUTimerCB *cb;
> void *opaque;
> bool need_replay_checkpoint = false;
> + ReplayCheckpoint replay_checkpoint_id;
>
> if (!atomic_read(&timer_list->active_timers)) {
> return false;
> @@ -500,43 +501,40 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
> goto out;
> }
>
> - switch (timer_list->clock->type) {
> - case QEMU_CLOCK_REALTIME:
> - 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.
> - */
> + if (replay_mode != REPLAY_MODE_NONE) {
> + /* Postpone actual checkpointing to timer list processing
> + * to properly check if we actually need it.
> + */
> + switch (timer_list->clock->type) {
> + case QEMU_CLOCK_VIRTUAL:
> need_replay_checkpoint = true;
> + replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL;
> + break;
> + case QEMU_CLOCK_HOST:
> + need_replay_checkpoint = true;
> + replay_checkpoint_id = CHECKPOINT_CLOCK_HOST;
> + break;
This is wrong at least for QEMU_CLOCK_HOST.
> + case QEMU_CLOCK_VIRTUAL_RT:
> + need_replay_checkpoint = true;
> + replay_checkpoint_id = CHECKPOINT_CLOCK_VIRTUAL_RT;
> + break;
> + default:
> + break;
> }
> - break;
> - case QEMU_CLOCK_HOST:
> - if (!replay_checkpoint(CHECKPOINT_CLOCK_HOST)) {
> - goto out;
> - }
> - break;
> - case QEMU_CLOCK_VIRTUAL_RT:
> - if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL_RT)) {
> - goto out;
> - }
> - break;
> }
>
> /*
> - * Extract expired timers from active timers list and and process them.
> + * Extract expired timers from active timers list and and process them,
> + * taking into account checkpointing required in rr mode.
> *
> - * In rr mode we need "filtered" checkpointing for virtual clock.
> - * Checkpoint must be replayed before any non-EXTERNAL timer has been
> - * processed and only one time (virtual clock value stays same). But
> these
> - * timers may appear in the timers list while it being processed, so this
> - * must be checked until we finally decide that "no timers left - we are
> - * done".
> + * Checkpoint must be replayed before any timer has been processed
> + * and only one time. But new timers may appear in the timers list while
> + * it's being processed, so this must be checked until we finally decide
> + * that "no timers left - we are done" (to avoid skipping checkpoint due
> to
> + * possible races).
> + * Also 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.
> */
> current_time = qemu_clock_get_ns(timer_list->clock->type);
Reading the host clock here is not protected by the checkpoint.
Therefore it may incur the inconsistency when replaying the execution.
> qemu_mutex_lock(&timer_list->active_timers_lock);
> @@ -552,7 +550,7 @@ bool timerlist_run_timers(QEMUTimerList *timer_list)
> /* once we got here, checkpoint clock only once */
> need_replay_checkpoint = false;
> qemu_mutex_unlock(&timer_list->active_timers_lock);
> - if (!replay_checkpoint(CHECKPOINT_CLOCK_VIRTUAL)) {
> + if (!replay_checkpoint(replay_checkpoint_id)) {
> goto out;
> }
> qemu_mutex_lock(&timer_list->active_timers_lock);
> --
> 2.7.4
Pavel Dovgalyuk
- [Qemu-devel] [PATCH v3 0/4] Introduce attributes for timers subsystem and remove QEMU_CLOCK_VIRTUAL_EXT clock type, Artem Pisarenko, 2018/10/18
- [Qemu-devel] [PATCH v3 1/4] Revert some patches from recent [PATCH v6] "Fixing record/replay and adding reverse debugging", Artem Pisarenko, 2018/10/18
- [Qemu-devel] [PATCH v3 3/4] Restores record/replay behavior related to special virtual clock processing for timers used in external subsystems., Artem Pisarenko, 2018/10/18
- [Qemu-devel] [PATCH v3 2/4] Introduce attributes to qemu timer subsystem, Artem Pisarenko, 2018/10/18
- [Qemu-devel] [PATCH v3] Optimize record/replay checkpointing for all clocks it applies to, Artem Pisarenko, 2018/10/18