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

[Olaf Hering <olaf@aepfle.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3192 bytes --]

Am Mon, 11 Mar 2019 04:16:07 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> >>> On 08.03.19 at 20:20, <olaf@aepfle.de> 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. pvops provides a clocksource=xen,
but apparently with a low resolution.


> > +#define VTSC_NTP_PPM_TOLERANCE 500UL  /* Amount of drift NTP will handle */  
> As per above, "will" is too strong here: I think this wants to be "can", and
> you want to add "if enabled in the guest".

I will reword this comment.

> > +#define VTSC_JITTER_RANGE_KHZ 200UL   /* Assumed jitter in cpu_khz */  
> I'm struggling to understand the comment: Surely not every single
> CPU surfaces a jitter of precisely 200kHz?

This is the observed range of frequencies on a large number of hosts.
The frequencies are expected to be exactly like "2.4GHz", but due to
inaccurate measurement the value of "cpu_khz" is higher or lower.
The value of "200" is simple the range of observed frequencies.

I will find a better name for this variable.


> > +     *    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.
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.

> > +    vtsc_tolerance_khz = tmp;
> > +    printk("Tolerating vtsc jitter for domUs: %u kHz\n", vtsc_tolerance_khz);  
> Could you remind me again how Dom0 remains unaffected here?
> And if that's indeed the case, why would that be?

A dom0 does not move from one host to another.
And it better synchronizes with an external clock if domUs are supposed to be migrated in the future.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

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