Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965195AbcJXPJK (ORCPT ); Mon, 24 Oct 2016 11:09:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56168 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933483AbcJXPJH (ORCPT ); Mon, 24 Oct 2016 11:09:07 -0400 Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li References: <1477304593-3453-1-git-send-email-wanpeng.li@hotmail.com> <20161024150323.GB2247@potion> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Yunhong Jiang , Wanpeng Li From: Paolo Bonzini Message-ID: Date: Mon, 24 Oct 2016 17:09:01 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20161024150323.GB2247@potion> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Mon, 24 Oct 2016 15:09:07 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3389 Lines: 95 On 24/10/2016 17:03, Radim Krčmář wrote: > I have only compile-tested it, but it should optimize the switch and > also fix two bugs. The first one is major. > This needs that the deadline clearing in [5/5] is fixed. > ---8<--- > We must start the hrimer even if the expiration is already in the past, > otherwise the periodic timer would not rearm the hrtimer. > > And computing next expiration of a period timer does not require current > time. The period should be constant, so it is more precise to add the > period to the last expiration time. This fixes a time difference > between hrtimer expiration and apic->lapic_timer.target_expiration. > > Signed-off-by: Radim Krčmář > --- > arch/x86/kvm/lapic.c | 38 +++++++++++++------------------------- > 1 file changed, 13 insertions(+), 25 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index f4734fe12803..6244988418be 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1350,19 +1350,19 @@ static void start_sw_tscdeadline(struct kvm_lapic *apic) > > static void start_sw_period(struct kvm_lapic *apic) > { > - ktime_t now; > - > - now = apic->lapic_timer.timer.base->get_time(); > - > if (!apic->lapic_timer.period) > return; > > - if (likely(ktime_after(apic->lapic_timer.target_expiration, now))) > - hrtimer_start(&apic->lapic_timer.timer, > - apic->lapic_timer.target_expiration, > - HRTIMER_MODE_ABS_PINNED); > - else > + if (apic_lvtt_oneshot(apic) && > + ktime_after(apic->lapic_timer.target_expiration, > + apic->lapic_timer.timer.base->get_time())) { > apic_timer_expired(apic); > + return; > + } > + > + hrtimer_start(&apic->lapic_timer.timer, > + apic->lapic_timer.target_expiration, > + HRTIMER_MODE_ABS_PINNED); > } > > static bool set_target_expiration(struct kvm_lapic *apic) > @@ -1414,10 +1414,11 @@ static bool set_target_expiration(struct kvm_lapic *apic) > > static void advance_periodic_target_expiration(struct kvm_lapic *apic) > { > - apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, rdtsc()) + > + apic->lapic_timer.tscdeadline += > nsec_to_cycles(apic->vcpu, apic->lapic_timer.period); > - apic->lapic_timer.target_expiration = ktime_add_ns(apic->lapic_timer.timer.base->get_time(), > - apic->lapic_timer.period); > + apic->lapic_timer.target_expiration = > + ktime_add_ns(apic->lapic_timer.target_expiration, > + apic->lapic_timer.period); > } > > bool kvm_lapic_hv_timer_in_use(struct kvm_vcpu *vcpu) > @@ -1481,19 +1482,6 @@ void kvm_lapic_switch_to_hv_timer(struct kvm_vcpu *vcpu) > > WARN_ON(apic->lapic_timer.hv_timer_in_use); > > - if (apic_lvtt_period(apic)) { > - ktime_t remaining, now; > - u64 tscl = rdtsc(); > - > - now = apic->lapic_timer.timer.base->get_time(); > - remaining = hrtimer_get_remaining(&apic->lapic_timer.timer); > - if (ktime_to_ns(remaining) < 0) > - remaining = ktime_set(0, 0); > - > - apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) + > - nsec_to_cycles(apic->vcpu, ktime_to_ns(remaining)); > - apic->lapic_timer.target_expiration = ktime_add_ns(now, ktime_to_ns(remaining)); > - } > start_hv_timer(apic); > } > EXPORT_SYMBOL_GPL(kvm_lapic_switch_to_hv_timer); > Reviewed-by: Paolo Bonzini Go ahead, squash it into 5/5 and commit to kvm/queue. :) Paolo