[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH v8 11/21] replay: recording and replaying cl
From: |
Pavel Dovgaluk |
Subject: |
Re: [Qemu-devel] [RFC PATCH v8 11/21] replay: recording and replaying clock ticks |
Date: |
Tue, 3 Feb 2015 14:23:57 +0300 |
> From: Paolo Bonzini [mailto:address@hidden
> On 03/02/2015 11:51, Pavel Dovgaluk wrote:
> >> From: Paolo Bonzini [mailto:address@hidden
> >> On 22/01/2015 09:52, Pavel Dovgalyuk wrote:
> >>> Clock ticks are considered as the sources of non-deterministic data for
> >>> virtual machine. This patch implements saving the clock values when they
> >>> are acquired (virtual, host clock, rdtsc, and some other timers).
> >>> When replaying the execution corresponding values are read from log and
> >>> transfered to the module, which wants to read the values.
> >>> Such a design required the clock polling to be synchronized. Sometimes
> >>> it is not true - e.g. when timeouts for timer lists are checked. In this
> >>> case
> >>> we use a cached value of the clock, passing it to the client code.
> >>>
> >>> Signed-off-by: Pavel Dovgalyuk <address@hidden>
> >>> ---
> >>> cpus.c | 3 +-
> >>> include/qemu/timer.h | 10 +++++
> >>> qemu-timer.c | 7 ++--
> >>> replay/Makefile.objs | 1 +
> >>> replay/replay-internal.h | 13 +++++++
> >>> replay/replay-time.c | 84
> >>> ++++++++++++++++++++++++++++++++++++++++++++++
> >>> replay/replay.h | 25 ++++++++++++++
> >>> stubs/replay.c | 9 +++++
> >>> 8 files changed, 147 insertions(+), 5 deletions(-)
> >>> create mode 100755 replay/replay-time.c
> >>>
> >>> diff --git a/cpus.c b/cpus.c
> >>> index 8787277..01d89aa 100644
> >>> --- a/cpus.c
> >>> +++ b/cpus.c
> >>> @@ -353,7 +353,8 @@ static void icount_warp_rt(void *opaque)
> >>>
> >>> seqlock_write_lock(&timers_state.vm_clock_seqlock);
> >>> if (runstate_is_running()) {
> >>> - int64_t clock = cpu_get_clock_locked();
> >>> + int64_t clock = REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT,
> >>> + cpu_get_clock_locked());
> >>> int64_t warp_delta;
> >>>
> >>> warp_delta = clock - vm_clock_warp_start;
> >>> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> >>> index 0666920..0c2472c 100644
> >>> --- a/include/qemu/timer.h
> >>> +++ b/include/qemu/timer.h
> >>> @@ -4,6 +4,7 @@
> >>> #include "qemu/typedefs.h"
> >>> #include "qemu-common.h"
> >>> #include "qemu/notify.h"
> >>> +#include "replay/replay.h"
> >>>
> >>> /* timers */
> >>>
> >>> @@ -760,6 +761,8 @@ int64_t cpu_icount_to_ns(int64_t icount);
> >>> /*******************************************/
> >>> /* host CPU ticks (if available) */
> >>>
> >>> +#define cpu_get_real_ticks cpu_get_real_ticks_impl
> >>> +
> >>> #if defined(_ARCH_PPC)
> >>>
> >>> static inline int64_t cpu_get_real_ticks(void)
> >>> @@ -913,6 +916,13 @@ static inline int64_t cpu_get_real_ticks (void)
> >>> }
> >>> #endif
> >>>
> >>> +#undef cpu_get_real_ticks
> >>> +
> >>> +static inline int64_t cpu_get_real_ticks(void)
> >>
> >> cpu_get_real_ticks should never be used. Please instead wrap
> >> cpu_get_ticks() with REPLAY_CLOCK.
> >
> > I don't quite understand this comment.
> > Do you mean that I should move REPLAY_CLOCK to the cpu_get_real_ticks
> > usages instead of it's
> implementation?
>
> Only to the cpu_get_ticks usage. The others are okay.
cpu_get_ticks cannot call cpu_get_real_ticks in icount mode.
And other functions can. Then we should put REPLAY_CLOCK into those functions?
>
> >>> +/*! Reads next clock value from the file.
> >>> + If clock kind read from the file is different from the parameter,
> >>> + the value is not used.
> >>> + If the parameter is -1, the clock value is read to the cache anyway.
> >>> */
> >>
> >> In what case could the clock kind not match?
> >>
> >
> > It was used in full version which had to skip clock from the log while
> > loading the VM state.
>
> So can it be removed for now?
>
I think it can.
Pavel Dovgalyuk