Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758953AbcJYNEC (ORCPT ); Tue, 25 Oct 2016 09:04:02 -0400 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 Date: Tue, 25 Oct 2016 15:03:56 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Wanpeng Li Cc: Wanpeng Li , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" , Paolo Bonzini , Yunhong Jiang Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers 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-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 25 Oct 2016 13:03:59 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2327 Lines: 58 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.