Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935372AbcJ0CLd (ORCPT ); Wed, 26 Oct 2016 22:11:33 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:34163 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932707AbcJ0CL2 (ORCPT ); Wed, 26 Oct 2016 22:11:28 -0400 MIME-Version: 1.0 In-Reply-To: <20161026133231.GA3452@potion> References: <1477304593-3453-1-git-send-email-wanpeng.li@hotmail.com> <20161024150323.GB2247@potion> <20161024152737.GB3197@potion> <20161025114334.GD3197@potion> <20161026133231.GA3452@potion> From: Wanpeng Li Date: Thu, 27 Oct 2016 10:11:26 +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 u9R2BfE7005017 Content-Length: 2589 Lines: 83 2016-10-26 21:32 GMT+08:00 Radim Krčmář : > 2016-10-26 14:02+0800, Wanpeng Li: >> 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. > > Which of those two results is closer to the expected duration of the > period? The result of w/ remove is more closer to the expected duration. > >> w/o remove hlt average latency = 2398462 >> w/ remove hlt average latency = 2403845 > > Some increase is expected when removing the code, because > kvm_lapic_switch_to_hv_timer() decreased the period by mistake: > it called > > now = get_time() > > first and then did > > remaining = target - get_time() // = hrtimer_get_remaining() > > but some time has passed in between calls of get_time(), let's call the > time that passed in between as "delta", so when the function later set > the new target, > > new_target = now + remaining // = now + target - (now + delta) > > the new_target was "delta" earlier. Agreed. > > 5k cycles is a huge difference, though ... Yeah, delta can't be as large as 5k cycles. > You tested the original kvm_lapic_switch_to_hv_timer(), with fixed > advance_periodic_target_expiration()? Yes. > >>> 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? > > Yes, I used numbers from Intel(R) Xeon(R) CPU E5-2620 v3 @ 2.40GHz, > which had TSC calibrated to 2397.223 MHz, so the expected "average > latency" with with the default 0x100000 ns period was > > 0x100000 * 2.397223 - 0x100000 = 1465094.5044479999 I agree with your remove the logic in kvm_lapic_switch_to_hv_timer() since it is more closer to the expected "average latency" now. Regards, Wanpeng Li