Re: [Xen-devel] [PATCH v12] tolerate jitter in cpu_khz calculation to avoid TSC emulation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.03.19 at 11:49, <olaf@xxxxxxxxx> wrote:
> Am Mon, 11 Mar 2019 04:16:07 -0600
> schrieb "Jan Beulich" <JBeulich@xxxxxxxx>:
> 
>> >>> On 08.03.19 at 20:20, <olaf@xxxxxxxxx> wrote:  
>> > To reiterate the second paragraph: if a domU uses TSC as primary clock
>> > source, it is expected that it runs NTP to cover for the resulting
>> > drift. Therefore this change does no need a knob to turn it on or off.  
>> Did you omit a 't' or a 'w' above? Judging from the patch I think you
>> mean "not", but I don't see how this follows, especially with your
>> subsequent reply validly stating that such a requirement did not
>> exist with the XenoLinux kernels.
> 
> This does indeed mean 'does not need a knob'.
> 
> I do not see how the HVM domU clock can be without drift if it does not
> sync with an external source. It seems xenlinux based PV drivers lack
> a clocksource, so they would better run ntp.

Yes indeed. But it still cannot be a requirement. There may be people
wanting to fully isolate their systems.

Plus - is your change somehow limited to HVM guests? I can't seem to
spot why that would be. And if it isn't, then leaving PV guests out of
the discussion is simply wrong.

Also I'm having trouble seeing how the connection to "drift" has come
up all of the sudden. Your change is to deal with singular events
(migration) aiui.

>> > +     *    freq    tolerated freq difference
>> > +     *  ------- = -------------------------
>> > +     *  Million         Million + PPM      
>> > +     */
>> > +    tmp = 1000 * 1000;
>> > +    tmp += VTSC_NTP_PPM_TOLERANCE;
>> > +    tmp *= cpu_khz;
>> > +    tmp /= 1000 * 1000;
>> > +
>> > +    tmp -= cpu_khz;
>> > +
>> > +    /*
>> > +     * Reduce the theoretical upper limit by the assumed measuring 
>> > inaccuracy.
>> > +     */
>> > +    if ( tmp >= VTSC_JITTER_RANGE_KHZ )
>> > +        tmp -= VTSC_JITTER_RANGE_KHZ;  
>> This could suggest the constant is meant to be an upper bound, but
>> (as said in review of prior versions) subtracting introduce a non-
>> linearity, _and_ it doesn't guarantee the result to be within the
>> assumed bounds.
> 
> The upper bound is the PPM value. ntpd in Linux can handle up to 500.
> What is unclear about this formula?
> First PPM is converted into "khz", on a 2GHz system tmp will become 1000.

I didn't question this part (except again for its Linux connection, which
you point out in your reply).

> Then the inaccuracy has to be handled, Xen can not know if cpu_khz is correct.
> As said above, the observed range was 200, so this will be subtracted.

This is what I consider wrong, because it results in

   tmp (initial)        |   tmp (result)
   198          |   198
   199          |   199
   200          |   0
   201          |   1
   ...
   398          |   198
   399          |   199
   400          |   0
   401          |   1

I'd expect you to _cap_ at 200 instead. But of course the seemingly
random 200 will itself need a much better reasoning, or at least a
clear indication of the data base (number of different systems) that
it was derived from. "Large number of hosts", after all, may mean 12
to you and tens of thousands to me.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel