Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1508225

Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers

From Radim Krčmář <rkrcmar@redhat.com>
Newsgroups linux.kernel
Subject Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers
Date 2016-10-25 13:50 +0200
Message-ID <sw8oV-53d-1@gated-at.bofh.it> (permalink)
References <svKFX-6hE-3@gated-at.bofh.it> <svP2V-Pj-7@gated-at.bofh.it> <svP2V-Pj-5@gated-at.bofh.it> <svPmi-ZI-35@gated-at.bofh.it> <svX0t-5XK-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


2016-10-25 07:39+0800, Wanpeng Li:
> 2016-10-24 23:27 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>> 2016-10-24 17:09+0200, Paolo Bonzini:
>>> On 24/10/2016 17:03, Radim Krčmář wrote:
>> [...]
>>>
>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>>>
>>> Go ahead, squash it into 5/5 and commit to kvm/queue. :)
>>
>> Did that, thanks.
>>
>> Wanpeng, the code is now under your name so please check it and/or
>> complain.
> 
> This patch 6/5 incurred regressions.
> 
> - The latency of the periodic mode which is emulated by VMX preemption
> is almost the same as periodic mode which is emulated by hrtimer.

Hm, what numbers are you getting?

When I ran the test with the original series, then it actually had worse
results with the VMX preemption than it did with the hrimer:

  hlt average latency   = 1464151
  pause average latency = 1467605

htl tests the hrtimer, pause tests the VMX preemption.  I just replaced
"hlt" with "pause" in the assembly loop.

The worse result was because the VMX preemption period was computed
incorrectly -- it was being added to now().  Some time passes between
the expiration and reading of now(), so this time was extending the
period while it shouldn't have.

If I run the test with [6/5], it gets sane numbers:

  hlt average latency   = 1465107
  pause average latency = 1465093

The numbers are sane bacause the test is not computing latency (= how
long after the timer should have fired have we received the interrupt)
-- it is computing the duration of the period in cycles, which is much
better right now.

> - The oneshot mode test of kvm-unit-tests/apic_timer_latency.flat almost fail.

Oops, silly mistake -- apic_timer_expired() was in the 'else' branch in
[5/5] and I didn't invert the condition after moving it.

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6244988418be..d7e74c8ec8ca 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1354,8 +1354,8 @@ static void start_sw_period(struct kvm_lapic *apic)
 		return;
 
 	if (apic_lvtt_oneshot(apic) &&
-	    ktime_after(apic->lapic_timer.target_expiration,
-	                apic->lapic_timer.timer.base->get_time())) {
+	    !ktime_after(apic->lapic_timer.target_expiration,
+	                 apic->lapic_timer.timer.base->get_time())) {
 		apic_timer_expired(apic);
 		return;
 	}

Paolo, can you squash that?

> Btw, hope you can also apply the testcase for kvm-unit-tests. :)

I will have some comments, because it would be nicer if it measured the
latency ... expected_expiration is not computed correctly.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li <kernellwp@gmail.com> - 2016-10-24 12:30 +0200
  [PATCH v3 3/5] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() Wanpeng Li <kernellwp@gmail.com> - 2016-10-24 12:30 +0200
  [PATCH v3 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li <kernellwp@gmail.com> - 2016-10-24 12:30 +0200
    Re: [PATCH v3 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode  VMX preemption timer support Radim Krčmář <rkrcmar@redhat.com> - 2016-10-24 17:00 +0200
      Re: [PATCH v3 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode  VMX preemption timer support Wanpeng Li <kernellwp@gmail.com> - 2016-10-25 01:40 +0200
  [PATCH v3 2/5] KVM: LAPIC: guarantee the timer is in tsc-deadline mode Wanpeng Li <kernellwp@gmail.com> - 2016-10-24 12:30 +0200
  Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Paolo Bonzini <pbonzini@redhat.com> - 2016-10-24 17:10 +0200
    Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-24 17:30 +0200
      Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-10-25 01:40 +0200
        Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-25 13:50 +0200
          Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Paolo Bonzini <pbonzini@redhat.com> - 2016-10-25 14:00 +0200
          Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-10-26 08:10 +0200
            Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-10-26 08:10 +0200
              Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-26 16:10 +0200
            Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-26 15:40 +0200
          Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-10-26 12:30 +0200
            Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Paolo Bonzini <pbonzini@redhat.com> - 2016-10-26 13:20 +0200
              Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Wanpeng Li <kernellwp@gmail.com> - 2016-10-26 13:30 +0200
  [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-24 17:10 +0200
    Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář <rkrcmar@redhat.com> - 2016-10-25 15:10 +0200

csiph-web