Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S943085AbcJZNcl (ORCPT ); Wed, 26 Oct 2016 09:32:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60460 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S941357AbcJZNcg (ORCPT ); Wed, 26 Oct 2016 09:32:36 -0400 Date: Wed, 26 Oct 2016 15:32:32 +0200 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Wanpeng Li Cc: Paolo Bonzini , "linux-kernel@vger.kernel.org" , kvm , Yunhong Jiang , Wanpeng Li Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Message-ID: <20161026133231.GA3452@potion> References: <1477304593-3453-1-git-send-email-wanpeng.li@hotmail.com> <20161024150323.GB2247@potion> <20161024152737.GB3197@potion> <20161025114334.GD3197@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.31]); Wed, 26 Oct 2016 13:32:35 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2364 Lines: 72 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? > 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. 5k cycles is a huge difference, though ... You tested the original kvm_lapic_switch_to_hv_timer(), with fixed advance_periodic_target_expiration()? >> 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 The expected value is pretty close to what I actually measured: >> [...] >> If I run the test with [6/5], it gets sane numbers: >> >> hlt average latency = 1465107 >> pause average latency = 1465093