From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751922AbdF1OAd (ORCPT ); Wed, 28 Jun 2017 10:00:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59260 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751509AbdF1OA1 (ORCPT ); Wed, 28 Jun 2017 10:00:27 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5CF987F417 Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx01.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 5CF987F417 Subject: Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay To: Wanpeng Li Cc: "linux-kernel@vger.kernel.org" , kvm , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Wanpeng Li References: <1498613352-4651-1-git-send-email-wanpeng.li@hotmail.com> <68c0d7f8-683f-c7fa-b328-e90e8dd9a789@redhat.com> From: Paolo Bonzini Message-ID: <0f94e3f2-d869-c0a0-4032-84f3a4dd6436@redhat.com> Date: Wed, 28 Jun 2017 16:00:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Wed, 28 Jun 2017 14:00:26 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/06/2017 15:55, Wanpeng Li wrote: >>> if ((atomic_read(&apic->lapic_timer.pending) && >>> !apic_lvtt_period(apic)) || >>> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) { >>> + (ret = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline))) { >>> if (apic->lapic_timer.hv_timer_in_use) >>> cancel_hv_timer(apic); >>> + if (ret == 1) { >>> + apic_timer_expired(apic); >>> + return true; >>> + } >> The preemption timer can also be used for modes other than TSC deadline. >> >> In periodic mode, your patch would miss a call to >> advance_periodic_target_expiration, which is only called by >> kvm_lapic_expired_hv_timer. > Actually I considered this before, however, I referred to apic timer > periodic mode which is emulated by hrtimer Periodic mode can also be emulated by preemption timer... it was added by some Wanpeng Li in commit 8003c9ae204e ("KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support", 2016-11-02), do you know him? ;) > , there is no hrtimer start > for the next period in start_sw_period(). If it is also buggy? start_sw_period always goes through the hrtimer for periodic timer: if (apic_lvtt_oneshot(apic) && ktime_after(ktime_get(), apic->lapic_timer.target_expiration)) { apic_timer_expired(apic); return; } hrtimer_start(&apic->lapic_timer.timer, apic->lapic_timer.target_expiration, HRTIMER_MODE_ABS_PINNED); (the direct call to apic_timer_expired is conditonal to apic_lvtt_oneshot). This way, apic_timer_fn takes care of advancing the hrtimer deadline and returning HRTIMER_RESTART. Paolo