Re: [Xen-devel] [PATCH 05/12] xen/pvclock: add monotonicity check

[john stultz <johnstul@xxxxxxxxxx>]

On Thu, Oct 15, 2009 at 6:27 AM, Dan Magenheimer
<dan.magenheimer@xxxxxxxxxx> wrote:
>> On 10/14/09 20:26, Dan Magenheimer wrote:
>> > As long as we are going through the trouble of making
>> > this monotonic, shouldn't it be monotonically increasing
>> > (rather than just monotonically non-decreasing)?  The
>> > rdtsc instruction and any suitably high-precision
>> > hardware timer will never return the same value
>> > on subsequent uses so this might be a reasonable
>> > precedent to obey.  E.g.
>> >
>> > +   return ret > xen_clocksource.cycle_last ?
>> > +           ret : ++xen_clocksource.cycle_last;

Oof. Modifying .cycle_last will cause timekeeping wreckage.  Please don't.
Also the above would break on SMP.

Ideally we would have moved cycle_last to the timekeeper structure,
since its a timekeeping specific value, not really clocksource
related.

However, there is the situation where we have don't have perfectly
synced TSCs, but TSCs that are very very close. In this case, the only
time we might see skew is if update_wall time were to run on the
slightly faster TSC, and then immediately after the gettimeofday()
code runs and sees the cycle_last value ahead of the rdtsc.

So the cycle_last check is hackish, but it lets folks use the much
faster TSC when we'd have to otherwise throw it out.

If you wanted something like this, a global last_tsc value could be
used and cmpxchged to ensure you always have an increasing TSC value.
However I suspect the performance hit there would be painful.

>> No, cycle_last isn't updated on every read, only on timer ticks.  This
>> test doesn't seem to be intended to make sure that every
>> clocksource_read is globally monotonic, but just to avoid
>> some boundary
>> conditions in the timer interrupt.  I just copied it directly from
>> read_tsc().
>
> I understand but you are now essentially emulating a
> reliable platform timer with a potentially unreliable
> (but still high resolution) per-CPU timer AND probably
> delivering that result to userland.
>
> Read_tsc should only be used if either CONSTANT_TSC
> or TSC_RELIABLE is true, so read_tsc is guaranteed
> to be monotonically-strictly-increasing by hardware
> (and enforced for CONSTANT_TSC by check_tsc_warp
> at boot).

Ideally, yes, only perfect TSCs should be used.

But in reality, its a big performance win for folks who can get away
with just slightly offset TSCs.

thanks
-john

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel