Re: [Qemu-devel] [PATCH 2/2] [RFC] time: refactor QEMU timer to use GHRTimer

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3851 bytes --]

On 2011-08-22 21:21, Anthony Liguori wrote:
> This replaces all of the QEMU timer code with GHRTimer, dramatically simplifying
> time keeping in QEMU while making it possible to use QEMUTimer code outside of
> the main loop.  The later is critical to building unit tests.
> 
> This is an RFC because I'm sure this breaks things as it changes things.  QEMU
> time keeping is quite a mess today.  Here's what we do today:
> 
> 1) We have three clocks:
>   a) the real time clock, based on system time, not monotonic
>   b) the host clock, based on the real time clock, monotonic by detecting
>      movements backward in time
>   c) the vm clock, based on real time clock but may start/stop with the guest

Not quite correct. We have:

 - QEMU_CLOCK_REALTIME: Based on monotonic source *if* the host
   supports it (there were probably once some stone-old Linuxes or
   BSDs), otherwise based on gettimeofday, i.e. non-monotonic. Always
   monotonic on Windows.

 - QEMU_CLOCK_VIRTUAL: Without -icount, same as above, but stops when
   the guest is stopped. The offset to compensate for stopped
   times is based on TSC, not sure why. With -icount, things get more
   complicated, Paolo had some nice explanations for the details.

 - QEMU_CLOCK_HOST: That's the one always based on the host's system
   time (CLOCK_REALTIME)
   + it takes potentially configured offsets into acount
   + users of that clock can register callbacks on time warps into the
     past (to adjust pending timers)

> 
> 2) A "cpu ticks" clock that uses platform specific mechanisms (inline asm)
> 
> 3) Various clock source implementations that may use a periodic timer or a
>    a dynamic time source.  We have different implementations for different
>    platforms
> 
> 4) Time events are delivered via SIGALRM which means we end up getting EINTRs
>    very often in QEMU.  This is fairly annoying.  Signals also race against
>    select leading to a very ugly set of work arounds involving writing data to
>    pipes.  This is the sort of stuff in Unix programming that I wish I never had
>    to learn about and am very eager to eliminate in QEMU :-)
> 
> (2) is just plain broken.  In modern operating systems, gettimeofday() is
> optimized CPU instructions when they can be used safely.  Often they can't be
> used safely and we ignore that in QEMU.  For instance, on x86, RDTSC races with
> the scheduler (not to mention that the TSC is infamously unstable across cores).
> The kernel does the right thing here and provides the fastest method that's
> correct.

I basically agree. Likely, these optimizations date back to the days
Linux had no fast gettimeofday syscalls. Not sure what the state on
other UNIXes is, but it's likely not worth keeping these optimizations.
Let's drop that one first and separately.

> 
> (1.a) seems like a bug more than a feature.  I don't see a lot of disadvantages
> to using a monotonic time source.
> 
> (1.b) is a bit naive in its current form.  Modern kernels export a truly
> monotonic time source which has a reliable frequency.  Even though (1.b) detects
> backwards jumps, it doesn't do anything about large forward jumps which can also
> be problematic.

These two assessments are partly just wrong, partly fail to see the real
use case. QEMU_CLOCK_HOST serves the very valid scenarios where a guest
clock shall be kept synchronized on the host time, also following its
jumps accordingly without stalling timers.

I haven't looked at the timer parts yet, but the clock assessments
indicate that some more careful thoughts are required. Strong NACK for
breaking QEMU_CLOCK_HOST in any case.

I do agree that there is likely room for cleanups, specifically when
demanding a sane POSIX/WIN32 host and/or reusing CLOCK_MONOTONIC
abstractions.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]