[PATCH RFC] handle tsc_tolerance during migration between identical hosts

[PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Olaf Hering]

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

Testing has shown that domUs with 'tsc_mode=default' can be migrated
safely between identical hardware, even if the measured clock frequency
differs by a few kHz. A change like shown below would allow to migrate
between "2.nnGHz" hosts without enforcing emulation. If the domU is
migrated to a host with "2.mmGHz" the frequency jump might have bad
effects, and tsc emulation might be good even if it comes with a
perfromance penalty.

The change below adds a new boot option to set a tolerance value.
I think its up to the host admin to decide, therefore a global option
should be enough.

Any opinions, also on the name of the cmdline option?


Olaf


--- 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);
 
+static unsigned int __read_mostly opt_tsc_tolerance;
+integer_param("tsc_tolerance", opt_tsc_tolerance);
+
 unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
 DEFINE_SPINLOCK(rtc_lock);
 unsigned long pit0_ticks;
@@ -1882,6 +1885,8 @@ void tsc_set_info(struct domain *d,
                   uint32_t tsc_mode, uint64_t elapsed_nsec,
                   uint32_t gtsc_khz, uint32_t incarnation)
 {
+    uint32_t khz_diff, tolerated;
+    printk(XENLOG_WARNING "%s: %u %x %lx %x %x\n", __func__, d->domain_id, tsc_mode, (unsigned long)elapsed_nsec, gtsc_khz, incarnation);
     if ( is_idle_domain(d) || is_hardware_domain(d) )
     {
         d->arch.vtsc = 0;
@@ -1924,6 +1929,15 @@ void tsc_set_info(struct domain *d,
         d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
         d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
         set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
+        khz_diff = 0;
+        if (gtsc_khz)
+            khz_diff = cpu_khz > gtsc_khz ? cpu_khz - gtsc_khz : gtsc_khz - cpu_khz;
+        if (opt_tsc_tolerance) {
+            tolerated = khz_diff <= opt_tsc_tolerance;
+        } else {
+            tolerated = d->arch.tsc_khz == cpu_khz;
+        }
+        printk("%s: d%u: dom0 has %lu kHz, domU expects %u kHz, difference of %u is %s tolerance of %u. (HVM scaling ratio: %llu)\n", __func__, d->domain_id, cpu_khz, gtsc_khz, khz_diff, tolerated ? "within" : "outside", opt_tsc_tolerance, has_hvm_container_domain(d) ? hvm_get_tsc_scaling_ratio(d->arch.tsc_khz) : 0 );
         /*
          * In default mode use native TSC if the host has safe TSC and:
          *  HVM/PVH: host and guest frequencies are the same (either
@@ -1931,10 +1945,8 @@ void tsc_set_info(struct domain *d,
          *  PV: guest has not migrated yet (and thus arch.tsc_khz == cpu_khz)
          */
         if ( tsc_mode == TSC_MODE_DEFAULT && host_tsc_is_safe() &&
-             (has_hvm_container_domain(d) ?
-              (d->arch.tsc_khz == cpu_khz ||
-               hvm_get_tsc_scaling_ratio(d->arch.tsc_khz)) :
-              incarnation == 0) )
+             (tolerated ||
+             (has_hvm_container_domain(d) && hvm_get_tsc_scaling_ratio(d->arch.tsc_khz))) )
         {
     case TSC_MODE_NEVER_EMULATE:
             d->arch.vtsc = 0;

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Jan Beulich]

>>> On 11.04.17 at 11:39, <olaf@aepfle.de> wrote:
> Testing has shown that domUs with 'tsc_mode=default' can be migrated
> safely between identical hardware, even if the measured clock frequency
> differs by a few kHz. A change like shown below would allow to migrate
> between "2.nnGHz" hosts without enforcing emulation. If the domU is
> migrated to a host with "2.mmGHz" the frequency jump might have bad
> effects, and tsc emulation might be good even if it comes with a
> perfromance penalty.
> 
> The change below adds a new boot option to set a tolerance value.
> I think its up to the host admin to decide, therefore a global option
> should be enough.
> 
> Any opinions,

I'm afraid successful testing is not a sufficient criteria here. At
the very least the (so far missing) documentation needs to very
clearly point out the possible risks associated with using this
option. But with there not being any functional change when
the option is not being made use of, I don't think there's a
reason not to add this (for the adventurous).

What I'm not sure about is whether having this as a global
(instead of per-guest) setting is really all that useful: As
different guests may require different tolerance, having a
single unique setting may not allow people to get very far.

> also on the name of the cmdline option?

I'm trying to advocate for dashes instead of underscores, and
I think it should say vtsc instead of tsc, to make clear it has an
effect on guests only.

Jan


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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Olaf Hering]

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

On Tue, Apr 11, Jan Beulich wrote:

> I'm afraid successful testing is not a sufficient criteria here. At
> the very least the (so far missing) documentation needs to very
> clearly point out the possible risks associated with using this
> option. But with there not being any functional change when
> the option is not being made use of, I don't think there's a
> reason not to add this (for the adventurous).

This can be handled by refering to the documentation of
'tsc_mode=native', which is what this change actually does.

> What I'm not sure about is whether having this as a global
> (instead of per-guest) setting is really all that useful: As
> different guests may require different tolerance, having a
> single unique setting may not allow people to get very far.

Another change on top of this, which adds a per-domU knob, could be
added. This also requires a decision if the per-domU or the global knob
has higher priority.

> > also on the name of the cmdline option?
> I'm trying to advocate for dashes instead of underscores, and
> I think it should say vtsc instead of tsc, to make clear it has an
> effect on guests only.

I will use "vtsc-tolerance=$khz".

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Jan Beulich]

>>> On 18.04.17 at 11:50, <olaf@aepfle.de> wrote:
> On Tue, Apr 11, Jan Beulich wrote:
> 
>> I'm afraid successful testing is not a sufficient criteria here. At
>> the very least the (so far missing) documentation needs to very
>> clearly point out the possible risks associated with using this
>> option. But with there not being any functional change when
>> the option is not being made use of, I don't think there's a
>> reason not to add this (for the adventurous).
> 
> This can be handled by refering to the documentation of
> 'tsc_mode=native', which is what this change actually does.

Does it? I've just looked at th patch again, and I didn't find any
such referral. Are you perhaps talking about some change you
only have done locally?

>> What I'm not sure about is whether having this as a global
>> (instead of per-guest) setting is really all that useful: As
>> different guests may require different tolerance, having a
>> single unique setting may not allow people to get very far.
> 
> Another change on top of this, which adds a per-domU knob, could be
> added. This also requires a decision if the per-domU or the global knob
> has higher priority.

This latter aspect is why I'd like to avoid having both.

Jan


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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Olaf Hering]

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

On Tue, Apr 18, Jan Beulich wrote:

> >>> On 18.04.17 at 11:50, <olaf@aepfle.de> wrote:
> > On Tue, Apr 11, Jan Beulich wrote:
> > 
> >> I'm afraid successful testing is not a sufficient criteria here. At
> >> the very least the (so far missing) documentation needs to very
> >> clearly point out the possible risks associated with using this
> >> option. But with there not being any functional change when
> >> the option is not being made use of, I don't think there's a
> >> reason not to add this (for the adventurous).
> > 
> > This can be handled by refering to the documentation of
> > 'tsc_mode=native', which is what this change actually does.
> 
> Does it? I've just looked at th patch again, and I didn't find any
> such referral. Are you perhaps talking about some change you
> only have done locally?

To me it looks like ->vtsc decides if there is emulation or not. So in
this sense tsc_mode remains "native" if the khz value is within the
tolerance range.


Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

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

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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Jan Beulich]

>>> On 19.04.17 at 09:46, <olaf@aepfle.de> wrote:
> On Tue, Apr 18, Jan Beulich wrote:
> 
>> >>> On 18.04.17 at 11:50, <olaf@aepfle.de> wrote:
>> > On Tue, Apr 11, Jan Beulich wrote:
>> > 
>> >> I'm afraid successful testing is not a sufficient criteria here. At
>> >> the very least the (so far missing) documentation needs to very
>> >> clearly point out the possible risks associated with using this
>> >> option. But with there not being any functional change when
>> >> the option is not being made use of, I don't think there's a
>> >> reason not to add this (for the adventurous).
>> > 
>> > This can be handled by refering to the documentation of
>> > 'tsc_mode=native', which is what this change actually does.
>> 
>> Does it? I've just looked at th patch again, and I didn't find any
>> such referral. Are you perhaps talking about some change you
>> only have done locally?
> 
> To me it looks like ->vtsc decides if there is emulation or not. So in
> this sense tsc_mode remains "native" if the khz value is within the
> tolerance range.

How does that answer my question? I still don't see any referral
to documentation (or another prominent warning about the risks
associated with the use of that new option).

Jan


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

Re: [PATCH RFC] handle tsc_tolerance during migration between identical hosts

[Juergen Gross]

On 11/04/17 11:39, Olaf Hering wrote:
> Testing has shown that domUs with 'tsc_mode=default' can be migrated
> safely between identical hardware, even if the measured clock frequency
> differs by a few kHz. A change like shown below would allow to migrate
> between "2.nnGHz" hosts without enforcing emulation. If the domU is
> migrated to a host with "2.mmGHz" the frequency jump might have bad
> effects, and tsc emulation might be good even if it comes with a
> perfromance penalty.
> 
> The change below adds a new boot option to set a tolerance value.
> I think its up to the host admin to decide, therefore a global option
> should be enough.
> 
> Any opinions, also on the name of the cmdline option?
> 
> 
> Olaf
> 
> 
> --- 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);
>  
> +static unsigned int __read_mostly opt_tsc_tolerance;
> +integer_param("tsc_tolerance", opt_tsc_tolerance);
> +
>  unsigned long __read_mostly cpu_khz;  /* CPU clock frequency in kHz. */
>  DEFINE_SPINLOCK(rtc_lock);
>  unsigned long pit0_ticks;
> @@ -1882,6 +1885,8 @@ void tsc_set_info(struct domain *d,
>                    uint32_t tsc_mode, uint64_t elapsed_nsec,
>                    uint32_t gtsc_khz, uint32_t incarnation)
>  {
> +    uint32_t khz_diff, tolerated;

Newline missing.

> +    printk(XENLOG_WARNING "%s: %u %x %lx %x %x\n", __func__, d->domain_id, tsc_mode, (unsigned long)elapsed_nsec, gtsc_khz, incarnation);
>      if ( is_idle_domain(d) || is_hardware_domain(d) )
>      {
>          d->arch.vtsc = 0;
> @@ -1924,6 +1929,15 @@ void tsc_set_info(struct domain *d,
>          d->arch.vtsc_offset = get_s_time() - elapsed_nsec;
>          d->arch.tsc_khz = gtsc_khz ?: cpu_khz;
>          set_time_scale(&d->arch.vtsc_to_ns, d->arch.tsc_khz * 1000);
> +        khz_diff = 0;
> +        if (gtsc_khz)
> +            khz_diff = cpu_khz > gtsc_khz ? cpu_khz - gtsc_khz : gtsc_khz - cpu_khz;
> +        if (opt_tsc_tolerance) {
> +            tolerated = khz_diff <= opt_tsc_tolerance;
> +        } else {
> +            tolerated = d->arch.tsc_khz == cpu_khz;
> +        }

Why so complicated?

khz_diff = cpu_khz > d->arch.tsc_khz ? cpu_khz - d->arch.tsc_khz
				     : d->arch.tsc_khz - cpu_khz;
tolerated = khz_diff <= opt_tsc_tolerance;

should work as well.


Juergen

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