Virtual clock is wapred from iothread and vcpu thread. When the hardware
events associated with warp checkpoint, then interrupt delivering may be
non-deterministic if checkpoint is processed in different threads in record
and replay.
This patch disables event processing for clock warp checkpoint and leaves
all hardware events to other checkpoints (e.g., virtual clock).

Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
---
 replay/replay.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/replay/replay.c b/replay/replay.c
index b9c496a..cc43c38 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -211,7 +211,12 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
     } else if (replay_mode == REPLAY_MODE_RECORD) {
         g_assert(replay_mutex_locked());
         replay_put_event(EVENT_CHECKPOINT + checkpoint);
-        replay_save_events(checkpoint);
+        /* This checkpoint belongs to several threads.
+           Processing events from different threads is
+           non-deterministic */
+        if (checkpoint != CHECKPOINT_CLOCK_WARP_START) {
+            replay_save_events(checkpoint);
+        }
         res = true;
     }
 out:

On 11/01/2018 09:27, Pavel Dovgalyuk wrote:
> Virtual clock is wapred from iothread and vcpu thread. When the hardware
> events associated with warp checkpoint, then interrupt delivering may be
> non-deterministic if checkpoint is processed in different threads in record
> and replay.
> This patch disables event processing for clock warp checkpoint and leaves
> all hardware events to other checkpoints (e.g., virtual clock).
> 
> Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> ---
>  replay/replay.c |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/replay/replay.c b/replay/replay.c
> index b9c496a..cc43c38 100644
> --- a/replay/replay.c
> +++ b/replay/replay.c
> @@ -211,7 +211,12 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
>      } else if (replay_mode == REPLAY_MODE_RECORD) {
>          g_assert(replay_mutex_locked());
>          replay_put_event(EVENT_CHECKPOINT + checkpoint);
> -        replay_save_events(checkpoint);
> +        /* This checkpoint belongs to several threads.
> +           Processing events from different threads is
> +           non-deterministic */
> +        if (checkpoint != CHECKPOINT_CLOCK_WARP_START) {
> +            replay_save_events(checkpoint);
> +        }
>          res = true;
>      }
>  out:
> 

Please add an assertion in replay_read_event that the read_checkpoint is
never CHECKPOINT_CLOCK_WARP_START.  Why is the checkpoint still needed?

Thanks,

Paolo

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> On 11/01/2018 09:27, Pavel Dovgalyuk wrote:
> > Virtual clock is wapred from iothread and vcpu thread. When the hardware
> > events associated with warp checkpoint, then interrupt delivering may be
> > non-deterministic if checkpoint is processed in different threads in record
> > and replay.
> > This patch disables event processing for clock warp checkpoint and leaves
> > all hardware events to other checkpoints (e.g., virtual clock).
> >
> > Signed-off-by: Pavel Dovgalyuk <pavel.dovgaluk@ispras.ru>
> > ---
> >  replay/replay.c |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/replay/replay.c b/replay/replay.c
> > index b9c496a..cc43c38 100644
> > --- a/replay/replay.c
> > +++ b/replay/replay.c
> > @@ -211,7 +211,12 @@ bool replay_checkpoint(ReplayCheckpoint checkpoint)
> >      } else if (replay_mode == REPLAY_MODE_RECORD) {
> >          g_assert(replay_mutex_locked());
> >          replay_put_event(EVENT_CHECKPOINT + checkpoint);
> > -        replay_save_events(checkpoint);
> > +        /* This checkpoint belongs to several threads.
> > +           Processing events from different threads is
> > +           non-deterministic */
> > +        if (checkpoint != CHECKPOINT_CLOCK_WARP_START) {
> > +            replay_save_events(checkpoint);
> > +        }
> >          res = true;
> >      }
> >  out:
> >
> 
> Please add an assertion in replay_read_event that the read_checkpoint is
> never CHECKPOINT_CLOCK_WARP_START. 

Ok.
 
> Why is the checkpoint still needed?

Because it synchronizes the warp itself.

Pavel Dovgalyuk