From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Date: Tue, 25 Oct 2016 15:03:56 +0200 Message-ID: <20161025130356.GF2247@potion> References: <1477304593-3453-1-git-send-email-wanpeng.li@hotmail.com> <20161024150323.GB2247@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: Wanpeng Li , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , Paolo Bonzini , Yunhong Jiang To: Wanpeng Li Return-path: Received: from mx1.redhat.com ([209.132.183.28]:57410 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751594AbcJYNEA (ORCPT ); Tue, 25 Oct 2016 09:04:00 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 2016-10-25 12:48+0000, Wanpeng Li: >> 在 2016年10月24日，下午11:03，Radim Krčmář 写道： >> >> 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ář >> --- >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> @@ -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)); >> - } > > Why you remove this? We computed those values in advance_periodic_target_expiration() and can re-use them here. hrtimer expiration == apic->lapic_timer.target_expiration. (Otherwise we're doing something wrong.) If that holds then the code does target_expiration = now + (target_expiration - now) Which can be optimized to target_expiration = target_expiration and to nothing. The same principle holds for apic->lapic_timer.tscdeadline as well. In other words: It doesn't matter if the timer switches between hrtimer and VMX deadline -- the target expiration is still the same. This hunk only added imprecision, because kvm_read_l1_tsc() and ktime_to_ns() were not using the same time for computation.