Re: [Qemu-devel] [3/4] cpus: protect TimerState writes with a spinlock

[Pavel Dovgalyuk]

> From: Paolo Bonzini [mailto:address@hidden
> On 28/08/2018 09:23, Pavel Dovgalyuk wrote:
> > Hi, Paolo!
> >
> > Seems that this one breaks the record/replay.
> 
> What are the symptoms?

Please look below.

> >> From: Paolo Bonzini [mailto:address@hidden
> >> In the next patch, we will need to write cpu_ticks_offset from any
> >> thread, even outside the BQL.  Currently, it is protected by the BQL
> >> just because cpu_enable_ticks and cpu_disable_ticks happen to hold it,
> >> but the critical sections are well delimited and it's easy to remove
> >> the BQL dependency.
> >>
> >> Add a spinlock that matches vm_clock_seqlock, and hold it when writing
> >> to the TimerState.  This also lets us fix cpu_update_icount when 64-bit
> >> atomics are not available.
> >>
> >> Fields of TiemrState are reordered to avoid padding.
> >>
> >> Signed-off-by: Paolo Bonzini <address@hidden>
> >> ---
> >>  cpus.c | 72 ++++++++++++++++++++++++++++++++++++++--------------------
> >>  1 file changed, 47 insertions(+), 25 deletions(-)
> >>


Here is the description:

> >>  static void icount_adjust_rt(void *opaque)
> >> @@ -480,7 +494,8 @@ static void icount_warp_rt(void)
> >>          return;
> >>      }
> >>
> >> -    seqlock_write_begin(&timers_state.vm_clock_seqlock);
> >> +    seqlock_write_lock(&timers_state.vm_clock_seqlock,
> >> +                       &timers_state.vm_clock_lock);
> >
> > After locking here,
> >
> >>      if (runstate_is_running()) {
> >>          int64_t clock = REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT,
> >>                                       cpu_get_clock_locked());
> >
> > REPLAY_CLOCK can't request icount with cpu_get_icount_raw, because
> > it loops infinitely here:
> >
> >     do {
> >         start = seqlock_read_begin(&timers_state.vm_clock_seqlock);
> >         icount = cpu_get_icount_raw_locked();
> >     } while (seqlock_read_retry(&timers_state.vm_clock_seqlock, start));
> >
> >



Pavel Dovgalyuk