[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 0/4] Introduce attributes for timers subsyste
Re: [Qemu-devel] [PATCH v3 0/4] Introduce attributes for timers subsystem and remove QEMU_CLOCK_VIRTUAL_EXT clock type
Thu, 17 Jan 2019 10:42:33 +0300
> From: Artem Pisarenko [mailto:address@hidden
> ср, 16 янв. 2019 г. в 12:15, Pavel Dovgalyuk <address@hidden>:
> > > From: Artem Pisarenko [mailto:address@hidden
> > > > It seems, that this approach is not always correct.
> > > > Now timerlist_deadline_ns uses all virtual timers for deadline
> > > > calculation (including
> > > external
> > > > ones).
> > > > qemu_start_warp_timer uses the deadline for setting warp timer (which
> > > > should be
> > > deterministic).
> > > > Therefore warp timer may become non-deterministic and replay may behave
> > > > differently
> > > to
> > > > recording phase.
> > > >
> > > > We have to rollback these or improve somehow to avoid non-determinism.
> > >
> > > I dont understand how this approach would even introduce
> > > non-determinism. I'm not sure about aspects of timers subsystem you
> > > mentioned, assuming that maybe we missed something when tried to
> > > optimize. But this has nothing to do with determinism as long as we
> > > treat all virtual clock timers as deterministic, regardless of
> > > EXTERNAL attribute being set or not. They are intended to be used as
> > > such by design, aren't? This attribute was introduced purely to avoid
> > > extra events in log.
> > Right, but deadline calculation and icount warp events (that are written
> > into the log too)
> > depend on the active virtual timers. Therefore external ones may affect
> > icount warp
> > operation sequence.
> > Pavel Dovgalyuk
> Sorry, but I still don't understand the source of non-determinism.
The source is the external timers, that depend on "outer world", e.g., slirp.
These timers are not recorded/replayed, but may affect the icount warping
(which should remain deterministic).
> From what you said I may understand that replay module actually has
> some additional non-trivial (for me) dependencies on EXTERNAL
> attribute other than one, introduced in 'timerlist_run_timers()' (i.e.
> it expects deadline calculations somewhere else to match decisions
> made in this function, or something like that). I would agree that
> these patch series might introduce some incorrect behavior. But it
> must be identical in all emulations running in same conditions,
> because deadline, warp timer and their effects are all dependent only
> on virtual timers, and, therefore, are deterministic too. Of course,
> it needs to be fixed. Did you find solution ?
Please check the new version of the series.