All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
@ 2018-12-12 15:20 Olaf Hering
  2018-12-12 16:39 ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Olaf Hering @ 2018-12-12 15:20 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Olaf Hering, Wei Liu, Jan Beulich, Roger Pau Monné

Improve decision when vTSC emulation will be activated for a domU with
tsc_mode=default. The current approach is to compare the cpu_khz value
from two physical hosts. Since this value is not accurate, it can not be
used verbatim to decide if vTSC emulation needs to be enabled. Without
this change each TSC access from domU will be emulated after migration,
which causes a significant perfomance drop for workloads that make use
of rdtsc.

If a domU uses TSC as clocksoure it also must run NTP in some way to
avoid the potential drift what will most likely happen, independent of
any migration. The calculation of the drift is based on the time
returned by remote servers versus how fast the local clock advances. NTP
can handle a drift up to 500PPM. This means the local clocksource can
run up to 500us slower or faster. This calculation is based on the TSC
frequency of the host where the domU was started. Once a domU is
migrated to a host of a different class, like from "2.3GHz" to "2.4GHz",
the TSC frequency changes, but the domU kernel may not recalibrate
itself. As a result, the drift will be larger and might be outside of
the 500 PPM range. In addition, the kernel may notice the change of
speed in which the TSC advances and could change the clocksource. All
this depends of course on the type of OS that is running in the domU.

If the domU is migrated to another host of the same class, both hosts
may have a slightly different TSC frequency. The difference is small
enough and most likely within the drift range that NTP can handle.

The formula to set the tolerance for this host calculates the ticks
within a timespan of 500 PPM, which is 500us. From this number the
assumed jitter in the TSC frequency measurement must be substracted
because Xen itself can not know if the estimated value in cpu_khz is at
the edge or in the middle of the range of possible freqencies. Data
collected during the incident which triggered this change showed a
jitter of up to 200 KHz across systems of the same class. The resulting
tolerance is larger than needed, and it is expected to still cover the
possible drift that NTP can handle.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
--

v11:
 - trim patch and use calculated tolerance value, no admin interaction
   required
v10:
 - rebase to ae01a8e315
 - remove changes for libxl and save/restore protocol, the feature has
   to be per host instead of per guest
 - add newline to tsc_set_info (Andrew)
 - add pointer to xen-tscmode(7) in xl.cfg(5)/vtsc_tolerance_khz (Andrew)
 - mention potential clock drift in the domU (Andrew)
 - reword the newly added paragraph in xen-tscmode(7) (Andrew),
   and also mention that it is about the measured/estimated TSC value
   rather than the real value. The latter is simply unknown.
 - use uint32 for internal representation of xen_domctl_tsc_info.vtsc_tolerance_khz
   and remove padding field
 - add math for real TSC frequency to xen-tscmode
v9:
 - extend commit msg, mention potential issues with xc_sr_rec_tsc_info._res1
v8:
 - adjust also python stream checker for added tolerance member
v7:
 - use uint16 in libxl_types.idl to match type used elsewhere in the patch
v6:
 - mention default value in xl.cfg
 - tsc_set_info: remove usage of __func__, use %d for domid
 - tsc_set_info: use ABS to calculate khz_diff
v5:
 - reduce functionality to allow setting of the tolerance value
   only at initial domU startup
v4:
 - add missing copyback in XEN_DOMCTL_set_vtsc_tolerance_khz
v3:
 - rename vtsc_khz_tolerance to vtsc_tolerance_khz
 - separate domctls to adjust values
 - more docs
 - update libxl.h
 - update python tests
 - flask check bound to tsc permissions
 - not runtime tested due to dlsym() build errors in staging
---
 xen/arch/x86/time.c | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 24d4c2794b..2ffdc2ea8f 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -43,6 +43,9 @@ static char __initdata opt_clocksource[10];
 string_param("clocksource", opt_clocksource);
 
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
+#define VTSC_NTP_PPM_TOLERANCE 500UL  /* Amount of drift NTP will handle */
+#define VTSC_JITTER_RANGE_KHZ 200UL   /* Assumed jitter in cpu_khz */
+static unsigned int __read_mostly vtsc_tolerance_khz;
 DEFINE_SPINLOCK(rtc_lock);
 unsigned long pit0_ticks;
 
@@ -1885,6 +1888,16 @@ void __init early_time_init(void)
     printk("Detected %lu.%03lu MHz processor.\n", 
            cpu_khz / 1000, cpu_khz % 1000);
 
+    tmp = 1000 * 1000;
+    tmp += VTSC_NTP_PPM_TOLERANCE;
+    tmp *= cpu_khz;
+    tmp /= 1000 * 1000;
+    tmp -= cpu_khz;
+    if (tmp >= VTSC_JITTER_RANGE_KHZ)
+        tmp -= VTSC_JITTER_RANGE_KHZ;
+    vtsc_tolerance_khz = (unsigned int)tmp;
+    printk("Tolerating vtsc jitter for domUs: %u kHz.\n", vtsc_tolerance_khz);
+
     setup_irq(0, 0, &irq0);
 }
 
@@ -2208,6 +2221,7 @@ void tsc_set_info(struct domain *d,
 
     switch ( d->arch.tsc_mode = tsc_mode )
     {
+        bool disable_vtsc;
         bool enable_tsc_scaling;
 
     case TSC_MODE_DEFAULT:
@@ -2223,8 +2237,25 @@ void tsc_set_info(struct domain *d,
          * When a guest is created, gtsc_khz is passed in as zero, making
          * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation.
          */
+        disable_vtsc = d->arch.tsc_khz == cpu_khz;
+
+        if ( tsc_mode == TSC_MODE_DEFAULT && gtsc_khz && vtsc_tolerance_khz )
+        {
+            long khz_diff;
+
+            khz_diff = ABS((long)(cpu_khz - gtsc_khz));
+            disable_vtsc = khz_diff <= vtsc_tolerance_khz;
+
+            printk(XENLOG_G_INFO "d%d: host has %lu kHz,"
+                   " domU expects %u kHz,"
+                   " difference of %ld is %s tolerance of %u\n",
+                   d->domain_id, cpu_khz, gtsc_khz, khz_diff,
+                   disable_vtsc ? "within" : "outside",
+                   vtsc_tolerance_khz);
+        }
+
         if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
-             (d->arch.tsc_khz == cpu_khz ||
+             (disable_vtsc ||
               (is_hvm_domain(d) &&
                hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
         {

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-12 15:20 [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation Olaf Hering
@ 2018-12-12 16:39 ` Jan Beulich
  2018-12-13  8:18   ` Olaf Hering
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2018-12-12 16:39 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne

>>> On 12.12.18 at 16:20, <olaf@aepfle.de> wrote:
> Improve decision when vTSC emulation will be activated for a domU with
> tsc_mode=default. The current approach is to compare the cpu_khz value
> from two physical hosts. Since this value is not accurate, it can not be
> used verbatim to decide if vTSC emulation needs to be enabled. Without
> this change each TSC access from domU will be emulated after migration,
> which causes a significant perfomance drop for workloads that make use
> of rdtsc.
> 
> If a domU uses TSC as clocksoure it also must run NTP in some way to
> avoid the potential drift what will most likely happen, independent of
> any migration.

Which drift? While anyone's well advised to run NTP, a completely
isolated set of systems may have no need to, if their interactions don't
depend on exactly matching time.

> The calculation of the drift is based on the time
> returned by remote servers versus how fast the local clock advances. NTP
> can handle a drift up to 500PPM. This means the local clocksource can
> run up to 500us slower or faster. This calculation is based on the TSC
> frequency of the host where the domU was started. Once a domU is
> migrated to a host of a different class, like from "2.3GHz" to "2.4GHz",
> the TSC frequency changes, but the domU kernel may not recalibrate
> itself.

That's why we switch to emulated (or hardware scaling) mode in that
case. It's anyway not really clear to me what this entire ...

> As a result, the drift will be larger and might be outside of
> the 500 PPM range. In addition, the kernel may notice the change of
> speed in which the TSC advances and could change the clocksource. All
> this depends of course on the type of OS that is running in the domU.

... (up to here) paragraph is supposed to tell the reader.

> @@ -1885,6 +1888,16 @@ void __init early_time_init(void)
>      printk("Detected %lu.%03lu MHz processor.\n", 
>             cpu_khz / 1000, cpu_khz % 1000);
>  
> +    tmp = 1000 * 1000;
> +    tmp += VTSC_NTP_PPM_TOLERANCE;
> +    tmp *= cpu_khz;
> +    tmp /= 1000 * 1000;
> +    tmp -= cpu_khz;
> +    if (tmp >= VTSC_JITTER_RANGE_KHZ)
> +        tmp -= VTSC_JITTER_RANGE_KHZ;

Besides the style issue in the if() - how can this be correct? This
clearly introduces a discontinuity (just consider the case where
tmp is exactly VTSC_JITTER_RANGE_KHZ before the if()). And
I also can't see how it guarantees the resulting value to be
below VTSC_JITTER_RANGE_KHZ. Did you perhaps mean to
cap the value (i.e. = instead of -= )?

> +    vtsc_tolerance_khz = (unsigned int)tmp;

Stray cast.

> +    printk("Tolerating vtsc jitter for domUs: %u kHz.\n", vtsc_tolerance_khz);

Please omit the full stop; the printk() in context above shouldn't
have one either.

> @@ -2223,8 +2237,25 @@ void tsc_set_info(struct domain *d,
>           * When a guest is created, gtsc_khz is passed in as zero, making
>           * d->arch.tsc_khz == cpu_khz. Thus no need to check incarnation.
>           */
> +        disable_vtsc = d->arch.tsc_khz == cpu_khz;
> +
> +        if ( tsc_mode == TSC_MODE_DEFAULT && gtsc_khz && vtsc_tolerance_khz )
> +        {
> +            long khz_diff;
> +
> +            khz_diff = ABS((long)(cpu_khz - gtsc_khz));

I think

            khz_diff = ABS((long)cpu_khz - gtsc_khz);

or some such would be less fragile, if e.g. we decided to make
cpu_khz an unsigned int (which is an option, as I don't think
we'll see above 4THz systems any time soon).

> +            disable_vtsc = khz_diff <= vtsc_tolerance_khz;
> +
> +            printk(XENLOG_G_INFO "d%d: host has %lu kHz,"
> +                   " domU expects %u kHz,"
> +                   " difference of %ld is %s tolerance of %u\n",
> +                   d->domain_id, cpu_khz, gtsc_khz, khz_diff,
> +                   disable_vtsc ? "within" : "outside",
> +                   vtsc_tolerance_khz);
> +        }
> +
>          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
> -             (d->arch.tsc_khz == cpu_khz ||
> +             (disable_vtsc ||
>                (is_hvm_domain(d) &&
>                 hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
>          {

In any event I don't follow why all of the sudden this becomes
an always-on mode, with not even a boot command line override.

Jan


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-12 16:39 ` Jan Beulich
@ 2018-12-13  8:18   ` Olaf Hering
  2018-12-13  8:45     ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Olaf Hering @ 2018-12-13  8:18 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne


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

Am Wed, 12 Dec 2018 09:39:25 -0700
schrieb "Jan Beulich" <JBeulich@suse.com>:

> >>> On 12.12.18 at 16:20, <olaf@aepfle.de> wrote:  
> > If a domU uses TSC as clocksoure it also must run NTP in some way to
> > avoid the potential drift what will most likely happen, independent of
> > any migration.  
> Which drift? While anyone's well advised to run NTP, a completely
> isolated set of systems may have no need to, if their interactions don't
> depend on exactly matching time.

If these hosts do not sync time to some reference host, the advancing of time
is undefined before and after my change.

> > The calculation of the drift is based on the time
> > returned by remote servers versus how fast the local clock advances. NTP
> > can handle a drift up to 500PPM. This means the local clocksource can
> > run up to 500us slower or faster. This calculation is based on the TSC
> > frequency of the host where the domU was started. Once a domU is
> > migrated to a host of a different class, like from "2.3GHz" to "2.4GHz",
> > the TSC frequency changes, but the domU kernel may not recalibrate
> > itself.  
> 
> That's why we switch to emulated (or hardware scaling) mode in that
> case. It's anyway not really clear to me what this entire ...
> 
> > As a result, the drift will be larger and might be outside of
> > the 500 PPM range. In addition, the kernel may notice the change of
> > speed in which the TSC advances and could change the clocksource. All
> > this depends of course on the type of OS that is running in the domU.  
> 
> ... (up to here) paragraph is supposed to tell the reader.
> 
> > @@ -1885,6 +1888,16 @@ void __init early_time_init(void)
> >      printk("Detected %lu.%03lu MHz processor.\n", 
> >             cpu_khz / 1000, cpu_khz % 1000);
> >  
> > +    tmp = 1000 * 1000;
> > +    tmp += VTSC_NTP_PPM_TOLERANCE;
> > +    tmp *= cpu_khz;
> > +    tmp /= 1000 * 1000;
> > +    tmp -= cpu_khz;
> > +    if (tmp >= VTSC_JITTER_RANGE_KHZ)
> > +        tmp -= VTSC_JITTER_RANGE_KHZ;  
> 
> Besides the style issue in the if() - how can this be correct? This
> clearly introduces a discontinuity (just consider the case where
> tmp is exactly VTSC_JITTER_RANGE_KHZ before the if()). And
> I also can't see how it guarantees the resulting value to be
> below VTSC_JITTER_RANGE_KHZ. Did you perhaps mean to
> cap the value (i.e. = instead of -= )?

This is supposed to make sure the value of Hz for 500us is always larger
than the assumed jitter. So for a 2GHz host, with a theoretical tolerance
of 1000, the actual tolerance is set to 800. tmp will be larger than the
assumed jitter for cpu_khz > 400.

> > +    vtsc_tolerance_khz = (unsigned int)tmp;  
> Stray cast.

Copy&paste from the cpu_khz assignment, perhaps a remainder from i386 support?

> > +    printk("Tolerating vtsc jitter for domUs: %u kHz.\n", vtsc_tolerance_khz);  
> Please omit the full stop; the printk() in context above shouldn't
> have one either.

You mean the trailing dot, or what means "full stop" in this context?

> > +            disable_vtsc = khz_diff <= vtsc_tolerance_khz;
> > +
> > +            printk(XENLOG_G_INFO "d%d: host has %lu kHz,"
> > +                   " domU expects %u kHz,"
> > +                   " difference of %ld is %s tolerance of %u\n",
> > +                   d->domain_id, cpu_khz, gtsc_khz, khz_diff,
> > +                   disable_vtsc ? "within" : "outside",
> > +                   vtsc_tolerance_khz);
> > +        }
> > +
> >          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
> > -             (d->arch.tsc_khz == cpu_khz ||
> > +             (disable_vtsc ||
> >                (is_hvm_domain(d) &&
> >                 hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
> >          {  
> 
> In any event I don't follow why all of the sudden this becomes
> an always-on mode, with not even a boot command line override.

Perhaps I failed to explain why there is no need to make this a knob.

If a domU uses TSC as its timesource, and if it also uses NTP to make
sure the time advances correctly, then this change will make sure the
advancing of time will be withing the bounds that NTP can correct.
If it does use TSC, but does not use NTP then the advancing will be
undefined before and after my change.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13  8:18   ` Olaf Hering
@ 2018-12-13  8:45     ` Jan Beulich
  2018-12-13  9:04       ` Olaf Hering
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2018-12-13  8:45 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne

>>> On 13.12.18 at 09:18, <olaf@aepfle.de> wrote:
> Am Wed, 12 Dec 2018 09:39:25 -0700
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> >>> On 12.12.18 at 16:20, <olaf@aepfle.de> wrote:  
>> > If a domU uses TSC as clocksoure it also must run NTP in some way to
>> > avoid the potential drift what will most likely happen, independent of
>> > any migration.  
>> Which drift? While anyone's well advised to run NTP, a completely
>> isolated set of systems may have no need to, if their interactions don't
>> depend on exactly matching time.
> 
> If these hosts do not sync time to some reference host, the advancing of time
> is undefined before and after my change.

I'm lost. I simply don't understand what you're trying to tell me,
or how your answer relates to my question.

>> > The calculation of the drift is based on the time
>> > returned by remote servers versus how fast the local clock advances. NTP
>> > can handle a drift up to 500PPM. This means the local clocksource can
>> > run up to 500us slower or faster. This calculation is based on the TSC
>> > frequency of the host where the domU was started. Once a domU is
>> > migrated to a host of a different class, like from "2.3GHz" to "2.4GHz",
>> > the TSC frequency changes, but the domU kernel may not recalibrate
>> > itself.  
>> 
>> That's why we switch to emulated (or hardware scaling) mode in that
>> case. It's anyway not really clear to me what this entire ...
>> 
>> > As a result, the drift will be larger and might be outside of
>> > the 500 PPM range. In addition, the kernel may notice the change of
>> > speed in which the TSC advances and could change the clocksource. All
>> > this depends of course on the type of OS that is running in the domU.  
>> 
>> ... (up to here) paragraph is supposed to tell the reader.
>> 
>> > @@ -1885,6 +1888,16 @@ void __init early_time_init(void)
>> >      printk("Detected %lu.%03lu MHz processor.\n", 
>> >             cpu_khz / 1000, cpu_khz % 1000);
>> >  
>> > +    tmp = 1000 * 1000;
>> > +    tmp += VTSC_NTP_PPM_TOLERANCE;
>> > +    tmp *= cpu_khz;
>> > +    tmp /= 1000 * 1000;
>> > +    tmp -= cpu_khz;
>> > +    if (tmp >= VTSC_JITTER_RANGE_KHZ)
>> > +        tmp -= VTSC_JITTER_RANGE_KHZ;  
>> 
>> Besides the style issue in the if() - how can this be correct? This
>> clearly introduces a discontinuity (just consider the case where
>> tmp is exactly VTSC_JITTER_RANGE_KHZ before the if()). And
>> I also can't see how it guarantees the resulting value to be
>> below VTSC_JITTER_RANGE_KHZ. Did you perhaps mean to
>> cap the value (i.e. = instead of -= )?
> 
> This is supposed to make sure the value of Hz for 500us is always larger
> than the assumed jitter. So for a 2GHz host, with a theoretical tolerance
> of 1000, the actual tolerance is set to 800. tmp will be larger than the
> assumed jitter for cpu_khz > 400.

Again you don't appear to answer my question regarding the
discontinuity you introduce.

>> > +    vtsc_tolerance_khz = (unsigned int)tmp;  
>> Stray cast.
> 
> Copy&paste from the cpu_khz assignment, perhaps a remainder from i386 
> support?

Copy-and-paste is an explanation but not an excuse. Style
violations should not be cloned and thus furthered.

>> > +    printk("Tolerating vtsc jitter for domUs: %u kHz.\n", 
> vtsc_tolerance_khz);  
>> Please omit the full stop; the printk() in context above shouldn't
>> have one either.
> 
> You mean the trailing dot, or what means "full stop" in this context?

Yes, "full stop" means the final period in a sentence.

>> > +            disable_vtsc = khz_diff <= vtsc_tolerance_khz;
>> > +
>> > +            printk(XENLOG_G_INFO "d%d: host has %lu kHz,"
>> > +                   " domU expects %u kHz,"
>> > +                   " difference of %ld is %s tolerance of %u\n",
>> > +                   d->domain_id, cpu_khz, gtsc_khz, khz_diff,
>> > +                   disable_vtsc ? "within" : "outside",
>> > +                   vtsc_tolerance_khz);
>> > +        }
>> > +
>> >          if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
>> > -             (d->arch.tsc_khz == cpu_khz ||
>> > +             (disable_vtsc ||
>> >                (is_hvm_domain(d) &&
>> >                 hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
>> >          {  
>> 
>> In any event I don't follow why all of the sudden this becomes
>> an always-on mode, with not even a boot command line override.
> 
> Perhaps I failed to explain why there is no need to make this a knob.
> 
> If a domU uses TSC as its timesource, and if it also uses NTP to make
> sure the time advances correctly, then this change will make sure the
> advancing of time will be withing the bounds that NTP can correct.
> If it does use TSC, but does not use NTP then the advancing will be
> undefined before and after my change.

As per above - I'm lost. I simply don't understand. All I notice is that
you talk about one specific use case of the TSC in a guest, without
(apparently) considering uses for other than what Linux calls its
clocksource. I in particular don't understand how anything can be
"undefined" here.

Jan


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13  8:45     ` Jan Beulich
@ 2018-12-13  9:04       ` Olaf Hering
  2018-12-13 10:41         ` Jan Beulich
  0 siblings, 1 reply; 10+ messages in thread
From: Olaf Hering @ 2018-12-13  9:04 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne


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

Am Thu, 13 Dec 2018 01:45:39 -0700
schrieb "Jan Beulich" <JBeulich@suse.com>:

> >>> On 13.12.18 at 09:18, <olaf@aepfle.de> wrote:  
> > Am Wed, 12 Dec 2018 09:39:25 -0700
> > schrieb "Jan Beulich" <JBeulich@suse.com>:
> >   
> >> >>> On 12.12.18 at 16:20, <olaf@aepfle.de> wrote:    
> >> > If a domU uses TSC as clocksoure it also must run NTP in some way to
> >> > avoid the potential drift what will most likely happen, independent of
> >> > any migration.    
> >> Which drift? While anyone's well advised to run NTP, a completely
> >> isolated set of systems may have no need to, if their interactions don't
> >> depend on exactly matching time.  
> > 
> > If these hosts do not sync time to some reference host, the advancing of time
> > is undefined before and after my change.  
> 
> I'm lost. I simply don't understand what you're trying to tell me,
> or how your answer relates to my question.

Then please rephrase the question? I do not see how my path affects the
advancing of time in domUs running on isolated systems. If their hardware
clocks advance at the same speed, my patch will not affect it. And if they
do advance at a different speed, what can emulation do to help with "correctness"?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13  9:04       ` Olaf Hering
@ 2018-12-13 10:41         ` Jan Beulich
  2018-12-13 10:47           ` Olaf Hering
  2018-12-13 13:25           ` Olaf Hering
  0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2018-12-13 10:41 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne

>>> On 13.12.18 at 10:04, <olaf@aepfle.de> wrote:
> Am Thu, 13 Dec 2018 01:45:39 -0700
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> >>> On 13.12.18 at 09:18, <olaf@aepfle.de> wrote:  
>> > Am Wed, 12 Dec 2018 09:39:25 -0700
>> > schrieb "Jan Beulich" <JBeulich@suse.com>:
>> >   
>> >> >>> On 12.12.18 at 16:20, <olaf@aepfle.de> wrote:    
>> >> > If a domU uses TSC as clocksoure it also must run NTP in some way to
>> >> > avoid the potential drift what will most likely happen, independent of
>> >> > any migration.    
>> >> Which drift? While anyone's well advised to run NTP, a completely
>> >> isolated set of systems may have no need to, if their interactions don't
>> >> depend on exactly matching time.  
>> > 
>> > If these hosts do not sync time to some reference host, the advancing of time
>> > is undefined before and after my change.  
>> 
>> I'm lost. I simply don't understand what you're trying to tell me,
>> or how your answer relates to my question.
> 
> Then please rephrase the question?

I was asking the drift of what you are talking about.

> I do not see how my path affects the
> advancing of time in domUs running on isolated systems. If their hardware
> clocks advance at the same speed, my patch will not affect it. And if they
> do advance at a different speed, what can emulation do to help with 
> "correctness"?

In a first step, let's assume clock calibration produces a precise result.
In such a case, are we in agreement that emulation is needed after
migration if clock speeds differ, even if just slightly?

In a second step, let's consider what impact errors in calibration have.
Between two systems with exactly the same hardware crystal
frequency there of course is going to be some drift. The problem
though is - between the two calibrated clock values from two systems
you can't easily tell what part of the difference is a result of the
calibration being imprecise, and what part of it is because of the
crystals not providing the exact same frequency. Without knowing
the possible range of both errors, the argumentation of _one of
them_ being tolerable within a certain range to consider _both_
systems sufficiently equal is at least questionable.

And further argumentation that everyone is using NTP anyway
doesn't make it any better, when it's no-where written down that
Xen is unusable with NTP running in all guests (I'm exaggerating
here just to get the point over). Don't forget that e.g. with
XenoLinux'es independent-wallclock setting defaulting to false, we've
been suggesting that people _don't_ need to use NTP inside their
(admittedly PV) guests. IOW - your change may not break people
not using NTP. Hence I don't think the mode you introduce can be
a default-on one, which in turn means a per-domain or at least
global enable control is needed (as over previous iterations we
seem to have been agreeing).

Jan



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13 10:41         ` Jan Beulich
@ 2018-12-13 10:47           ` Olaf Hering
  2018-12-13 13:25           ` Olaf Hering
  1 sibling, 0 replies; 10+ messages in thread
From: Olaf Hering @ 2018-12-13 10:47 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne


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

Am Thu, 13 Dec 2018 03:41:44 -0700
schrieb "Jan Beulich" <JBeulich@suse.com>:

> In a second step, let's consider what impact errors in calibration have.
> Between two systems with exactly the same hardware crystal
> frequency there of course is going to be some drift. The problem
> though is - between the two calibrated clock values from two systems
> you can't easily tell what part of the difference is a result of the
> calibration being imprecise, and what part of it is because of the
> crystals not providing the exact same frequency. Without knowing
> the possible range of both errors, the argumentation of _one of
> them_ being tolerable within a certain range to consider _both_
> systems sufficiently equal is at least questionable.

We already have a knob for that, if a reference clock can not be provided:
 tsc_mode=always_emulate
This would exactly cover the case there the assumed frequency that is
available during the start of domU will be used after migration.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13 10:41         ` Jan Beulich
  2018-12-13 10:47           ` Olaf Hering
@ 2018-12-13 13:25           ` Olaf Hering
  2018-12-13 14:46             ` Jan Beulich
  1 sibling, 1 reply; 10+ messages in thread
From: Olaf Hering @ 2018-12-13 13:25 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne


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

Am Thu, 13 Dec 2018 03:41:44 -0700
schrieb "Jan Beulich" <JBeulich@suse.com>:

> And further argumentation that everyone is using NTP anyway
> doesn't make it any better, when it's no-where written down that
> Xen is unusable with NTP running in all guests (I'm exaggerating
> here just to get the point over). Don't forget that e.g. with
> XenoLinux'es independent-wallclock setting defaulting to false, we've
> been suggesting that people _don't_ need to use NTP inside their
> (admittedly PV) guests. IOW - your change may not break people
> not using NTP. Hence I don't think the mode you introduce can be
> a default-on one, which in turn means a per-domain or at least
> global enable control is needed (as over previous iterations we
> seem to have been agreeing).

Regarding the possible unexpected drift if NTP is not used, and the domU
uses TSC as clocksource anyway: If one host is on one edge of the assumed
cpu_khz value and the other host is on the opposite edge, and the range
will be 200 as it is in my patch, the daily drift on a 2.3GHz host would
be 7.4seconds per day. This number is based on the fact that 200kz happen
within a time span of 86us, which I think is correct.

The question is, how much drift can be tolerated even without my patch.
Looking through 5 different hosts I use, the range is between -11 and +22,
and it happens to be +56 on my Laptop. So even that host with a calculated
drift of +22 would be off by 2 seconds per day if ntpd would not run.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13 13:25           ` Olaf Hering
@ 2018-12-13 14:46             ` Jan Beulich
  2018-12-14 15:33               ` Olaf Hering
  0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2018-12-13 14:46 UTC (permalink / raw)
  To: Olaf Hering; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne

>>> On 13.12.18 at 14:25, <olaf@aepfle.de> wrote:
> The question is, how much drift can be tolerated even without my patch.

This depends on what a system is used for. A few seconds off may
be fine for one purpose, but a significant problem for another.
Similarly a sudden (however small) change to the TSC tick rate
may be a problem for one purpose, but not for another.

If otoh (and just to give an example) it was determined that the
TSC tick rate on a physical system varies over time within
certain boundaries, then leveraging this to change (slowly, i.e.
with no steeper a gradient than might be observable on bare
hardware) the rate to match the new host's might be an option.
Emulation then could be turned off after a certain period of time.

Jan



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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation
  2018-12-13 14:46             ` Jan Beulich
@ 2018-12-14 15:33               ` Olaf Hering
  0 siblings, 0 replies; 10+ messages in thread
From: Olaf Hering @ 2018-12-14 15:33 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel, Roger Pau Monne


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

Am Thu, 13 Dec 2018 07:46:36 -0700
schrieb "Jan Beulich" <JBeulich@suse.com>:

> >>> On 13.12.18 at 14:25, <olaf@aepfle.de> wrote:  
> > The question is, how much drift can be tolerated even without my patch.  
> This depends on what a system is used for. A few seconds off may
> be fine for one purpose, but a significant problem for another.
> Similarly a sudden (however small) change to the TSC tick rate
> may be a problem for one purpose, but not for another.

Sure it depends on the usage. Should the default favor the "common" case
for domUs that are aware of a drift when using TSC and therefore use NTP,
or should the default favor the other case of domUs that, intentionally
or not, rely on emulation to reduce the expected drift?

And if this has to become a knob, then v10 might be the way to go.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-12-14 15:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-12 15:20 [PATCH v11] tolerate jitter in cpu_khz calculation to avoid TSC emulation Olaf Hering
2018-12-12 16:39 ` Jan Beulich
2018-12-13  8:18   ` Olaf Hering
2018-12-13  8:45     ` Jan Beulich
2018-12-13  9:04       ` Olaf Hering
2018-12-13 10:41         ` Jan Beulich
2018-12-13 10:47           ` Olaf Hering
2018-12-13 13:25           ` Olaf Hering
2018-12-13 14:46             ` Jan Beulich
2018-12-14 15:33               ` Olaf Hering

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.