Prev: Fix deep C-state TSC desynchronization
Next: max730x: make pullups configurable via platformdata
From: Avi Kivity on 15 Jun 2010 04:20
On 06/15/2010 10:34 AM, Zachary Amsden wrote:
> SMP VMs on machines with unstable TSC have their TSC offset adjusted by the
> local offset delta from last measurement. This does not take into account how
> long it has been since the measurement, leading to drift. Minimize the drift
> by accounting for any time difference the kernel has observed.
>
> Signed-off-by: Zachary Amsden<zamsden(a)redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c | 20 +++++++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 94f6ce8..1afecd7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -337,6 +337,7 @@ struct kvm_vcpu_arch {
> unsigned int time_offset;
> struct page *time_page;
> u64 last_host_tsc;
> + u64 last_host_ns;
>
> bool nmi_pending;
> bool nmi_injected;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 618c435..b1bdf05 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1810,6 +1810,19 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> /* Make sure TSC doesn't go backwards */
> s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> native_read_tsc() - vcpu->arch.last_host_tsc;
> +
> + /* Subtract elapsed cycle time from the delta computation */
> + if (check_tsc_unstable()&& vcpu->arch.last_host_ns) {
> + s64 delta;
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + delta = timespec_to_ns(&ts) - vcpu->arch.last_host_ns;
> + delta = delta * per_cpu(cpu_tsc_khz, cpu);
> + delta = delta / USEC_PER_SEC;
> + tsc_delta -= delta;
> + }
> +
> if (tsc_delta< 0 || check_tsc_unstable())
> kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
> kvm_migrate_timers(vcpu);
> @@ -1832,8 +1845,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> * vcpu->cpu != cpu can not detect this condition. So set
> * vcpu->cpu = -1 to force the recalibration above.
> */
> - if (check_tsc_unstable())
> + if (check_tsc_unstable()) {
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + vcpu->arch.last_host_ns = timespec_to_ns(&ts);
> vcpu->cpu = -1;
> + }
> }
>

Is there no way to do this calculation entirely with ktime_ts? struct
timespec has nasty multiplies and divides.

--
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Jason Wang on 16 Jun 2010 04:10
Zachary Amsden wrote:
> SMP VMs on machines with unstable TSC have their TSC offset adjusted by the
> local offset delta from last measurement. This does not take into account how
> long it has been since the measurement, leading to drift. Minimize the drift
> by accounting for any time difference the kernel has observed.
>
> Signed-off-by: Zachary Amsden <zamsden(a)redhat.com>
> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c | 20 +++++++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 94f6ce8..1afecd7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -337,6 +337,7 @@ struct kvm_vcpu_arch {
> unsigned int time_offset;
> struct page *time_page;
> u64 last_host_tsc;
> + u64 last_host_ns;
>
> bool nmi_pending;
> bool nmi_injected;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 618c435..b1bdf05 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1810,6 +1810,19 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> /* Make sure TSC doesn't go backwards */
> s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> native_read_tsc() - vcpu->arch.last_host_tsc;
> +
> + /* Subtract elapsed cycle time from the delta computation */
> + if (check_tsc_unstable() && vcpu->arch.last_host_ns) {
> + s64 delta;
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + delta = timespec_to_ns(&ts) - vcpu->arch.last_host_ns;
> + delta = delta * per_cpu(cpu_tsc_khz, cpu);
>
This seems not work well on a cpu w/o CONSTANT_TSC.
> + delta = delta / USEC_PER_SEC;
> + tsc_delta -= delta;
> + }
> +
> if (tsc_delta < 0 || check_tsc_unstable())
> kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
> kvm_migrate_timers(vcpu);
> @@ -1832,8 +1845,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> * vcpu->cpu != cpu can not detect this condition. So set
> * vcpu->cpu = -1 to force the recalibration above.
> */
> - if (check_tsc_unstable())
> + if (check_tsc_unstable()) {
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + vcpu->arch.last_host_ns = timespec_to_ns(&ts);
> vcpu->cpu = -1;
> + }
> }
>
> static int is_efer_nx(void)
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Glauber Costa on 16 Jun 2010 09:40
On Mon, Jun 14, 2010 at 09:34:07PM -1000, Zachary Amsden wrote:
> SMP VMs on machines with unstable TSC have their TSC offset adjusted by the
> local offset delta from last measurement. This does not take into account how
> long it has been since the measurement, leading to drift. Minimize the drift
> by accounting for any time difference the kernel has observed.
>
> Signed-off-by: Zachary Amsden <zamsden(a)redhat.com>
I believe this should be done not only if we have check_tsc_unstable() == true,
but anytime we adjust the tsc. I mean:

Sure it is expected to be much more relevant in this case, but if we're testing
generally for tsc_delta < 0 in the adjustment code, it is because we believe
it can happen, even if tsc is stable (otherwise, we'd better take it off completely).

And in that case, we should account elapsed time too.

> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/x86.c | 20 +++++++++++++++++++-
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 94f6ce8..1afecd7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -337,6 +337,7 @@ struct kvm_vcpu_arch {
> unsigned int time_offset;
> struct page *time_page;
> u64 last_host_tsc;
> + u64 last_host_ns;
>
> bool nmi_pending;
> bool nmi_injected;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 618c435..b1bdf05 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1810,6 +1810,19 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> /* Make sure TSC doesn't go backwards */
> s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> native_read_tsc() - vcpu->arch.last_host_tsc;
> +
> + /* Subtract elapsed cycle time from the delta computation */
> + if (check_tsc_unstable() && vcpu->arch.last_host_ns) {
> + s64 delta;
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + delta = timespec_to_ns(&ts) - vcpu->arch.last_host_ns;
> + delta = delta * per_cpu(cpu_tsc_khz, cpu);
> + delta = delta / USEC_PER_SEC;
> + tsc_delta -= delta;
> + }
> +
> if (tsc_delta < 0 || check_tsc_unstable())
> kvm_x86_ops->adjust_tsc_offset(vcpu, -tsc_delta);
> kvm_migrate_timers(vcpu);
> @@ -1832,8 +1845,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
> * vcpu->cpu != cpu can not detect this condition. So set
> * vcpu->cpu = -1 to force the recalibration above.
> */
> - if (check_tsc_unstable())
> + if (check_tsc_unstable()) {
> + struct timespec ts;
> + ktime_get_ts(&ts);
> + monotonic_to_bootbased(&ts);
> + vcpu->arch.last_host_ns = timespec_to_ns(&ts);
> vcpu->cpu = -1;
> + }
> }
>
> static int is_efer_nx(void)
> --
> 1.7.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Zachary Amsden on 16 Jun 2010 17:40
On 06/16/2010 03:32 AM, Glauber Costa wrote:
> On Mon, Jun 14, 2010 at 09:34:07PM -1000, Zachary Amsden wrote:
>
>> SMP VMs on machines with unstable TSC have their TSC offset adjusted by the
>> local offset delta from last measurement. This does not take into account how
>> long it has been since the measurement, leading to drift. Minimize the drift
>> by accounting for any time difference the kernel has observed.
>>
>> Signed-off-by: Zachary Amsden<zamsden(a)redhat.com>
>>
> I believe this should be done not only if we have check_tsc_unstable() == true,
> but anytime we adjust the tsc. I mean:
>
> Sure it is expected to be much more relevant in this case, but if we're testing
> generally for tsc_delta< 0 in the adjustment code, it is because we believe
> it can happen, even if tsc is stable (otherwise, we'd better take it off completely).
>
> And in that case, we should account elapsed time too.
>

If we get tsc_delta < 0 test turning true, we've got an unstable tsc to
begin with, so perhaps we should just check that and let the TSC code
deal with detecting an unstable TSC for us.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Rik van Riel on 14 Jul 2010 22:20
On 07/12/2010 10:25 PM, Zachary Amsden wrote:
> SMP VMs on machines with unstable TSC have their TSC offset adjusted by the
> local offset delta from last measurement. This does not take into account how
> long it has been since the measurement, leading to drift. Minimize the drift
> by accounting for any time difference the kernel has observed.
>
> Signed-off-by: Zachary Amsden<zamsden(a)redhat.com>

Acked-by: Rik van Riel <riel(a)redhat.com>

--
All rights reversed
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 | 
Pages: 1
Prev: Fix deep C-state TSC desynchronization
Next: max730x: make pullups configurable via platformdata