[PATCH v13] tolerate jitter in cpu_khz calculation to avoid TSC emulation

[PATCH v13] tolerate jitter in cpu_khz calculation to avoid TSC emulation

[Olaf Hering]

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 sync with an external
clocksource 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 in Linux can handle a drift up to 500PPM,
other OS and ntp implementations are known to handle a higher drift.
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.

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 upper drift limit of 500PPM is 1MHz on a 2.0GHz host.

Once a domU is migrated to a host of a different class, like from
"2.3GHz" to "2.4GHz", the TSC frequency change is significant. The domU
kernel may not recalibrate itself. As a result, the drift will be larger
and will be way 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. This will impact the workload within the domU.
All this depends of course on the type of OS that is running in the
domU. This patch can do nothing for this case.

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.

To reiterate the second paragraph: if a domU uses TSC as primary clock
source, it is expected that it syncs with external clocksources to cover
for the resulting drift. This is the same expectation as it exists for
bare metal. Therefore this change does not need a knob to turn it on or off.

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

v13:
 - rename a variable to better describe its meaning
 - expand comments in the code
 - reword commit message
 - mention the tolerance in xen-tscmode(7)
v12:
 - rebase to 4deeaf2a3e
 - remove casts and trailing dot in early_time_init
 - add comments to explain how vtsc_tolerance_khz is calculated
 - adjust cast in ABS()
 - adjust comment in tsc_set_info
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
---
 docs/man/xen-tscmode.7.pod | 13 ++++++++++
 xen/arch/x86/time.c        | 63 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/docs/man/xen-tscmode.7.pod b/docs/man/xen-tscmode.7.pod
index 1d81a3fe18..51d5d378f7 100644
--- a/docs/man/xen-tscmode.7.pod
+++ b/docs/man/xen-tscmode.7.pod
@@ -213,6 +213,19 @@ is emulated.  Note that, though emulated, the "apparent" TSC frequency
 will be the TSC frequency of the initial physical machine, even after
 migration.
 
+Since the calibration of the TSC frequency isn't 100% accurate, the
+value measured by Xen will vary across reboots. This means also several
+otherwise identical systems can have a slightly different _measured_ TSC
+frequency. As a result TSC access will be emulated if a domU is migrated
+from one host to another, identical host. To avoid the performance
+impact of TSC emulation a certain tolerance of the measured host TSC
+frequency is allowed by Xen. If the measured "cpu_khz" value is within
+the tolerance range, TSC access remains native. Otherwise it will be
+emulated. This allows to migrate domUs between identical hardware. If
+the domU will be migrated to a different kind of hardware, say from a
+"2.3GHz" to a "2.5GHz" system, TSC will be emualted to maintain the TSC
+frequency expected by the domU.
+
 Finally, tsc_mode==1 always enables TSC emulation, regardless of
 the underlying physical hardware. The "apparent" TSC frequency will
 be the TSC frequency of the initial physical machine, even after migration.
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 9a6ea8ffcb..5cc38ed34d 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -43,6 +43,23 @@ static char __initdata opt_clocksource[10];
 string_param("clocksource", opt_clocksource);
 
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
+/*
+ * NTP implementations running within the domU can handle a certain
+ * difference of the system clockspeed, compared to an external
+ * clocksource. This is ususally described as "drift". How much drift an
+ * OS can handle is described in its documentation. For NTP in Linux the
+ * value is 500 PPM, which is the lowest compared to other OS.
+ */
+#define VTSC_NTP_PPM_TOLERANCE 500UL
+/*
+ * The measurement of cpu_khz is not accurate. Its accuracy depends on the
+ * hardware. A bunch of systems with supposedly identical frequencies will
+ * measure different frequencies, which will also vary accross reboots.
+ * This variable tries to cover a range of frequencies seen in the wild.
+ * The range is substracted from the PPM value above.
+ */
+#define VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ 200UL
+static unsigned int __read_mostly vtsc_tolerance_khz;
 DEFINE_SPINLOCK(rtc_lock);
 unsigned long pit0_ticks;
 
@@ -1885,6 +1902,27 @@ void __init early_time_init(void)
     printk("Detected %lu.%03lu MHz processor.\n", 
            cpu_khz / 1000, cpu_khz % 1000);
 
+    /*
+     * How many kHz (in other words: drift) is ntpd in domU expected to handle?
+     *    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_MEASUREMENT_INACCURACY_RANGE_KHZ )
+        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;
+    vtsc_tolerance_khz = tmp;
+    printk("Tolerating vtsc jitter for domUs: %u kHz\n", vtsc_tolerance_khz);
+
     setup_irq(0, 0, &irq0);
 }
 
@@ -2193,6 +2231,8 @@ int tsc_set_info(struct domain *d,
 
     switch ( tsc_mode )
     {
+        bool disable_vtsc;
+
     case TSC_MODE_DEFAULT:
     case TSC_MODE_ALWAYS_EMULATE:
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
@@ -2201,13 +2241,30 @@ int tsc_set_info(struct domain *d,
 
         /*
          * In default mode use native TSC if the host has safe TSC and
-         * host and guest frequencies are the same (either "naturally" or
-         * - for HVM/PVH - via TSC scaling).
+         * host and guest frequencies are (almost) the same (either "naturally"
+         * or - for HVM/PVH - via TSC scaling).
          * 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

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

[Jan Beulich]

>>> On 13.03.19 at 09:28, <olaf@aepfle.de> wrote:
> --- a/xen/arch/x86/time.c
> +++ b/xen/arch/x86/time.c
> @@ -43,6 +43,23 @@ static char __initdata opt_clocksource[10];
>  string_param("clocksource", opt_clocksource);
>  
>  unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
> +/*
> + * NTP implementations running within the domU can handle a certain
> + * difference of the system clockspeed, compared to an external
> + * clocksource. This is ususally described as "drift". How much drift an
> + * OS can handle is described in its documentation. For NTP in Linux the
> + * value is 500 PPM, which is the lowest compared to other OS.
> + */
> +#define VTSC_NTP_PPM_TOLERANCE 500UL
> +/*
> + * The measurement of cpu_khz is not accurate. Its accuracy depends on the
> + * hardware. A bunch of systems with supposedly identical frequencies will
> + * measure different frequencies, which will also vary accross reboots.
> + * This variable tries to cover a range of frequencies seen in the wild.
> + * The range is substracted from the PPM value above.

subtracted

I also don't think the inaccuracy is only because of measurement
being imprecise. I don't think any two crystals will ever provide the
exact same frequencies.

> @@ -1885,6 +1902,27 @@ void __init early_time_init(void)
>      printk("Detected %lu.%03lu MHz processor.\n", 
>             cpu_khz / 1000, cpu_khz % 1000);
>  
> +    /*
> +     * How many kHz (in other words: drift) is ntpd in domU expected to handle?
> +     *    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_MEASUREMENT_INACCURACY_RANGE_KHZ )
> +        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;

The discontinuity is still there, and so far you've failed to explain
why a discontinuity is what you want here.

> @@ -2193,6 +2231,8 @@ int tsc_set_info(struct domain *d,
>  
>      switch ( tsc_mode )
>      {
> +        bool disable_vtsc;
> +
>      case TSC_MODE_DEFAULT:
>      case TSC_MODE_ALWAYS_EMULATE:
>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
> @@ -2201,13 +2241,30 @@ int tsc_set_info(struct domain *d,
>  
>          /*
>           * In default mode use native TSC if the host has safe TSC and
> -         * host and guest frequencies are the same (either "naturally" or
> -         * - for HVM/PVH - via TSC scaling).
> +         * host and guest frequencies are (almost) the same (either "naturally"
> +         * or - for HVM/PVH - via TSC scaling).
>           * 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));

This could easily be the initializer of the variable. And there's one
too many pair of parentheses.

I also don't see why the variable needs to be of a signed type.
Then again maybe the ABS() would better move ...

> +            disable_vtsc = khz_diff <= vtsc_tolerance_khz;

... here anyway, such that ...

> +            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);

... this logs the properly signed value?

> +        }
> +
>          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))) )
>          {

I'm sorry, but I continue to object to this adjustment getting done
both by default _and_ not in a per-guest manner. As said before,
you can't demand guests to run NTP, and hence you can't expect
them to get along with a few hundred kHz jump in observed TSC
frequency. Whether the performance drop due to vTSC use is
better or worse is a policy decision, which we should leave to the
admin. Hence the feature needs to be off by default, and there
needs to be at least a host-wide control to enable it; a per-guest
control would be better. IOW I explicitly do not agree with the
last sentence of the commit message.

Jan


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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 03:18:39 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> > +    if ( tmp >= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ )
> > +        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;  
> The discontinuity is still there, and so far you've failed to explain
> why a discontinuity is what you want here.

I think this is part of the commit message, there is an entire paragraph
for it. Perhaps the word "jitter" needs to be replaced by something else.

> > +            khz_diff = ABS(((long)cpu_khz - gtsc_khz));  
> This could easily be the initializer of the variable. And there's one
> too many pair of parentheses.

I will look into that part.

> I'm sorry, but I continue to object to this adjustment getting done
> both by default _and_ not in a per-guest manner. As said before,
> you can't demand guests to run NTP, and hence you can't expect
> them to get along with a few hundred kHz jump in observed TSC
> frequency. Whether the performance drop due to vTSC use is
> better or worse is a policy decision, which we should leave to the
> admin. Hence the feature needs to be off by default, and there
> needs to be at least a host-wide control to enable it; a per-guest
> control would be better. IOW I explicitly do not agree with the
> last sentence of the commit message.

This per-domU knob exists already: tsc_mode=always_emulate
IF a workload expects such behavior, the host admin can enable it.

Also I explained several times that the speed in which TSC advances is
kind of "opqaue" (if that is the correct english term), not only due to the
unavoidable inaccuracy. IF a workload on bare-metal or virtualized needs
to know a more accurate time, it needs an external clocksource. Without
such source the speed will remain inaccurate before and after this change.


Also: anyone else with an opinion on this change? Cant be a Jan-One-Men-Show...

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

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

[Jan Beulich]

>>> On 13.03.19 at 11:23, <olaf@aepfle.de> wrote:
> Am Wed, 13 Mar 2019 03:18:39 -0600
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> > +    if ( tmp >= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ )
>> > +        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;  
>> The discontinuity is still there, and so far you've failed to explain
>> why a discontinuity is what you want here.
> 
> I think this is part of the commit message, there is an entire paragraph
> for it. Perhaps the word "jitter" needs to be replaced by something else.

I don't think the commit message explains the anomaly.

>> > +            khz_diff = ABS(((long)cpu_khz - gtsc_khz));  
>> This could easily be the initializer of the variable. And there's one
>> too many pair of parentheses.
> 
> I will look into that part.
> 
>> I'm sorry, but I continue to object to this adjustment getting done
>> both by default _and_ not in a per-guest manner. As said before,
>> you can't demand guests to run NTP, and hence you can't expect
>> them to get along with a few hundred kHz jump in observed TSC
>> frequency. Whether the performance drop due to vTSC use is
>> better or worse is a policy decision, which we should leave to the
>> admin. Hence the feature needs to be off by default, and there
>> needs to be at least a host-wide control to enable it; a per-guest
>> control would be better. IOW I explicitly do not agree with the
>> last sentence of the commit message.
> 
> This per-domU knob exists already: tsc_mode=always_emulate
> IF a workload expects such behavior, the host admin can enable it.

But this affects a guest even ahead of migration. What if one
uses migration really just for emergencies? Why would they
run their guests in always-emulate mode?

> Also I explained several times that the speed in which TSC advances is
> kind of "opqaue" (if that is the correct english term), not only due to the
> unavoidable inaccuracy. IF a workload on bare-metal or virtualized needs
> to know a more accurate time, it needs an external clocksource. Without
> such source the speed will remain inaccurate before and after this change.

But quite possibly by far less than after a migration to a system
with a slightly different base frequency. The more that you didn't
even quantify how much drift there may be vs how much of the
inaccuracy is merely because of how we measure.

> Also: anyone else with an opinion on this change? Cant be a Jan-One-Men-Show...

Indeed, others participating would be much appreciated.

Jan



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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 04:38:06 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> But this affects a guest even ahead of migration. What if one
> uses migration really just for emergencies? Why would they
> run their guests in always-emulate mode?

Because this is what they will end up for the lifetime of the domU after migration?
The domU can not be just migrated back in the hope the originating
host has the expected cpu_khz value.

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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 03:18:39 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> > +    if ( tmp >= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ )
> > +        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;  
> The discontinuity is still there, and so far you've failed to explain
> why a discontinuity is what you want here.

This exists to make sure the non-emulated case stays within the PPM range.

But there is one more aspect to it that was not mentioned or covered:

The cpu_khz value used by Xen and its equivalent in dom0 or domU is just
a base frequency. ntpd will calculate the drift based on the external
source. That drift I think is what the PPM value is really all about.

If some frequency was measured and ntpd calculated some drift that is
close but not beyond the limit, then it can adjust the system clock as
needed. Once that domU is migrated to another, equally inaccurate host
Xen may decide that its cpu_khz value and the value expected by the domU
is still within the assumed PPM limit. As a result the time within the
domU may advance now in a slightly different speed, which is outside
that PPM limit. Now ntpd would stop synchronizing the system clock.
The correct action would be to emulate. But, Xen has no info at all what
the drift of its own clock actually is. All it has is that cpu_khz thing.

So ideally next to that cpu_khz value should be the drift value of a
given host to allow calculation of the real frequency. That frequency
will still vary to some degree according to various docs, but it would
be more accurate than the current cpu_khz value. If Xen had such feature
it would be in a better position to compare the frequency of two hosts.

In my case for example, if both of my hosts report an identical cpu_khz,
the ntp.drift file is 22 for one host and 4 for the other. So the real
frequency does actually differ.

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

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

[Jan Beulich]

>>> On 13.03.19 at 14:35, <olaf@aepfle.de> wrote:
> Am Wed, 13 Mar 2019 04:38:06 -0600
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> But this affects a guest even ahead of migration. What if one
>> uses migration really just for emergencies? Why would they
>> run their guests in always-emulate mode?
> 
> Because this is what they will end up for the lifetime of the domU after 
> migration?

Well, my reply was really suggesting that the guest may never get
migrated in the first place.

Jan



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

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

[Jan Beulich]

>>> On 13.03.19 at 14:51, <olaf@aepfle.de> wrote:
> Am Wed, 13 Mar 2019 03:18:39 -0600
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> > +    if ( tmp >= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ )
>> > +        tmp -= VTSC_MEASUREMENT_INACCURACY_RANGE_KHZ;  
>> The discontinuity is still there, and so far you've failed to explain
>> why a discontinuity is what you want here.
> 
> This exists to make sure the non-emulated case stays within the PPM range.

Staying within range is orthogonal to the introduction of any
discontinuities. I can only draw the same table again:

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

Why is there this big step between 199 and 200? And why does an
initial value of 200 get handled the same as an initial value of 0, but
then 400 doesn't get handled this same way again? And (seeing the
last row I've added now) how is the result staying in range?

Jan



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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 08:36:52 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> Why is there this big step between 199 and 200? And why does an
> initial value of 200 get handled the same as an initial value of 0, but
> then 400 doesn't get handled this same way again? And (seeing the
> last row I've added now) how is the result staying in range?

Well, capping the PPM-to-khz value may not work anyway as outlined in the
other email about missing 'drift'. We may just leave that PPM value as is,
or reduce it upfront by a certain amount. Since right now the real drift
is not considered, the plain 500PPM may work or not in the worst case.

I will think about if and how drift should be part of the picture.

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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 03:18:39 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> The discontinuity is still there, and so far you've failed to explain
> why a discontinuity is what you want here.

In v12 you asked for some data about the ranges. With new data the ranges are:

2.20GHz 29khz
2.30GHz 105khz
2.40GHz 3524khz
2.50GHz 114khz

There are 5 systems in total in the 2.3/2.4/2.5 classes that have a much higher frequency, making the total range 15382/6052/19179.
I have to check what these runaway values mean.

The total range within the ntp.drift file is -71.138..345.942 for >2000 hosts.

I will see if any experiments can be done on the hosts which are on the edge of a frequency change. Would be nice to know if ntpd can cope with tsc_mode=native for example when a domU is migrated from one to the other.

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

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

[Olaf Hering]

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

Am Wed, 13 Mar 2019 03:18:39 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> I'm sorry, but I continue to object to this adjustment getting done
> both by default _and_ not in a per-guest manner. As said before,
> you can't demand guests to run NTP, and hence you can't expect
> them to get along with a few hundred kHz jump in observed TSC
> frequency. Whether the performance drop due to vTSC use is
> better or worse is a policy decision, which we should leave to the
> admin. Hence the feature needs to be off by default, and there
> needs to be at least a host-wide control to enable it; a per-guest
> control would be better. IOW I explicitly do not agree with the
> last sentence of the commit message.

So this seems the be the essential part that prevents moving forward.

Your claim is basically that "we do not know how the workload reacts
to frequency change".
My claim is basically "there is enough evidence that syncing with
external clock is required if the frequency remotely matters".

I think that conflict can not be easily solved.

One way to solve it would be a knob that injects a value into the
proposed "vtsc_tolerance_khz" variable, leave the calculation to
the host admin, and leave code in tsc_set_info basically as is.

Maybe "xl set-params" can be the way to change the value, that way
it can be changed globally at runtime if needed.

In staging the change would affect HVM and PVH. I never ran PVH,
I have to assume it behaves like HVM in this regard.


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

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

[Juergen Gross]

On 14/03/2019 14:50, Olaf Hering wrote:
> Am Wed, 13 Mar 2019 03:18:39 -0600
> schrieb "Jan Beulich" <JBeulich@suse.com>:
> 
>> I'm sorry, but I continue to object to this adjustment getting done
>> both by default _and_ not in a per-guest manner. As said before,
>> you can't demand guests to run NTP, and hence you can't expect
>> them to get along with a few hundred kHz jump in observed TSC
>> frequency. Whether the performance drop due to vTSC use is
>> better or worse is a policy decision, which we should leave to the
>> admin. Hence the feature needs to be off by default, and there
>> needs to be at least a host-wide control to enable it; a per-guest
>> control would be better. IOW I explicitly do not agree with the
>> last sentence of the commit message.
> 
> So this seems the be the essential part that prevents moving forward.
> 
> Your claim is basically that "we do not know how the workload reacts
> to frequency change".
> My claim is basically "there is enough evidence that syncing with
> external clock is required if the frequency remotely matters".
> 
> I think that conflict can not be easily solved.
> 
> One way to solve it would be a knob that injects a value into the
> proposed "vtsc_tolerance_khz" variable, leave the calculation to
> the host admin, and leave code in tsc_set_info basically as is.
> 
> Maybe "xl set-params" can be the way to change the value, that way
> it can be changed globally at runtime if needed.
> 
> In staging the change would affect HVM and PVH. I never ran PVH,
> I have to assume it behaves like HVM in this regard.

I think Andrew (or Ian?) once suggested to handle this whole mess in the
migration stream instead of the hypervisor.

So why don't you:

- add a domain config item for specifying the allowed jitter
- add that value to the migration struct xc_sr_rec_tsc_info (there is a
  reserved field available)
- and then modify handle_tsc_info() in tools/libxc/xc_sr_common_x86.c to
  test the host frequency to be in the acceptable range and if this is
  the case put the host frequency into the gtsc_khz parameter of the
  xc_domain_set_tsc_info() call

This would be the least intrusive change allowing maximum flexibility
IMO.


Juergen

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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 15:10:16 +0100
schrieb Juergen Gross <jgross@suse.com>:

> This would be the least intrusive change allowing maximum flexibility
> IMO.

I think earlier attempts did alter the tooling.
But that would not help with existing domUs started on hosts that do not have that property.
A global knob would help with that.


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

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

[Jan Beulich]

>>> On 14.03.19 at 14:50, <olaf@aepfle.de> wrote:
> In staging the change would affect HVM and PVH. I never ran PVH,
> I have to assume it behaves like HVM in this regard.

And again you makie it look as if PV wasn't also going through that
same path. Yet all that's there at the top of the function is

    if ( is_pv_domain(d) && is_hardware_domain(d) )
    {
        d->arch.vtsc = 0;
        return 0;
    }

Jan



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

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

[Juergen Gross]

On 14/03/2019 15:14, Olaf Hering wrote:
> Am Thu, 14 Mar 2019 15:10:16 +0100
> schrieb Juergen Gross <jgross@suse.com>:
> 
>> This would be the least intrusive change allowing maximum flexibility
>> IMO.
> 
> I think earlier attempts did alter the tooling.
> But that would not help with existing domUs started on hosts that do not have that property.

Yes. This happens when introducing new features.

> A global knob would help with that.

I believe a per-domain controllable value is to be preferred. Different
guests might have different jitter tolerance.

And even the tools side variant would (in theory) allow for injection
of a per-host value on the receiving side. Not sure whether this is a
good idea, though.

And knowing you are targeting a special customer you could still replace
the migration stream sending program to insert the jitter value.


Juergen


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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 08:20:31 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> And again you makie it look as if PV wasn't also going through that
> same path. Yet all that's there at the top of the function is

I missed the '&&', read it as '||'.


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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 15:24:59 +0100
schrieb Juergen Gross <jgross@suse.com>:

> I believe a per-domain controllable value is to be preferred. Different
> guests might have different jitter tolerance.

If others agree with that, I can resume v10 of this patch, which was sent end last year.

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

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

[Jan Beulich]

>>> On 14.03.19 at 15:33, <olaf@aepfle.de> wrote:
> Am Thu, 14 Mar 2019 15:24:59 +0100
> schrieb Juergen Gross <jgross@suse.com>:
> 
>> I believe a per-domain controllable value is to be preferred. Different
>> guests might have different jitter tolerance.
> 
> If others agree with that, I can resume v10 of this patch, which was sent 
> end last year.

Well, unless I'm misremembering you had dropped that functionality
without anyone having asked to do so.

Jan



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

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

[Juergen Gross]

On 14/03/2019 15:33, Olaf Hering wrote:
> Am Thu, 14 Mar 2019 15:24:59 +0100
> schrieb Juergen Gross <jgross@suse.com>:
> 
>> I believe a per-domain controllable value is to be preferred. Different
>> guests might have different jitter tolerance.
> 
> If others agree with that, I can resume v10 of this patch, which was sent end last year.

V10 still involved changes in the hypervisor, while my proposal is tools
only.


Juergen

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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 08:36:16 -0600
schrieb "Jan Beulich" <JBeulich@suse.com>:

> Well, unless I'm misremembering you had dropped that functionality
> without anyone having asked to do so.

Yes, based on my view that an external clocksource has to be used if TSC
is the primary local clocksource.

I will double check what the concern was about v10.
The change for xen-tscmode(7) needs to be redone, at least.

So before I spam you guys with any more variants, I better ask if any further attempt would fly anyway.

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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 15:38:48 +0100
schrieb Juergen Gross <jgross@suse.com>:

> V10 still involved changes in the hypervisor, while my proposal is tools
> only.

Oh, you mean I should adjust the value that is supposedly expected by
domU to the one the current hypervisor has, just to bybass the check in
tsc_set_info? I never thought of doing it that way. The initial value
will be lost, if it is done that way.

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

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

[Juergen Gross]

On 14/03/2019 15:46, Olaf Hering wrote:
> Am Thu, 14 Mar 2019 15:38:48 +0100
> schrieb Juergen Gross <jgross@suse.com>:
> 
>> V10 still involved changes in the hypervisor, while my proposal is tools
>> only.
> 
> Oh, you mean I should adjust the value that is supposedly expected by
> domU to the one the current hypervisor has, just to bybass the check in
> tsc_set_info? I never thought of doing it that way. The initial value
> will be lost, if it is done that way.

This wouldn't be a must. You could save the original frequency in
Xenstore and send that value when migrating again.


Juergen

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

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

[Olaf Hering]

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

Am Thu, 14 Mar 2019 15:41:55 +0100
schrieb Olaf Hering <olaf@aepfle.de>:

> So before I spam you guys with any more variants, I better ask if any further attempt would fly anyway.

So, should I spend any more time to fix this bug?

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