Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752922AbcJZGCn (ORCPT ); Wed, 26 Oct 2016 02:02:43 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:37216 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751563AbcJZGCi (ORCPT ); Wed, 26 Oct 2016 02:02:38 -0400 MIME-Version: 1.0 In-Reply-To: <20161025114334.GD3197@potion> References: <1477304593-3453-1-git-send-email-wanpeng.li@hotmail.com> <20161024150323.GB2247@potion> <20161024152737.GB3197@potion> <20161025114334.GD3197@potion> From: Wanpeng Li Date: Wed, 26 Oct 2016 14:02:36 +0800 Message-ID: Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: Paolo Bonzini , "linux-kernel@vger.kernel.org" , kvm , Yunhong Jiang , Wanpeng Li Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9Q62q9r027162 Content-Length: 3263 Lines: 96 2016-10-25 19:43 GMT+08:00 Radim Krčmář : > 2016-10-25 07:39+0800, Wanpeng Li: >> 2016-10-24 23:27 GMT+08:00 Radim Krčmář : >>> 2016-10-24 17:09+0200, Paolo Bonzini: >>>> On 24/10/2016 17:03, Radim Krčmář wrote: >>> [...] >>>> >>>> Reviewed-by: Paolo Bonzini >>>> >>>> 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? The two fixes look good to me. However, the codes which you remove in kvm_lapic_switch_to_hv_timer() results in different numbers. w/o remove hlt average latency = 2398462 w/ remove hlt average latency = 2403845 > > When I ran the test with the original series, then it actually had worse Did you test this by running my kvm-unit-tests/apic_timer_latency.flat? > 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. Agreed. > >> - 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; > } > It works. > 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. It measured the latency from guest programs the clock event device to interrupt injected to guest after timer fire. Regards, Wanpeng Li