From: Wanpeng Li <kernellwp@gmail.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
kvm <kvm@vger.kernel.org>,
Yunhong Jiang <yunhong.jiang@intel.com>,
Wanpeng Li <wanpeng.li@hotmail.com>
Subject: Re: [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers
Date: Thu, 27 Oct 2016 10:11:26 +0800 [thread overview]
Message-ID: <CANRm+CwP+o0TKzhfftiMpUoesmQeAw1N=maXmVcjpFsUwLWPqg@mail.gmail.com> (raw)
In-Reply-To: <20161026133231.GA3452@potion>
2016-10-26 21:32 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
> 2016-10-26 14:02+0800, Wanpeng Li:
>> 2016-10-25 19:43 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>>> 2016-10-25 07:39+0800, Wanpeng Li:
>>>> 2016-10-24 23:27 GMT+08:00 Radim Krčmář <rkrcmar@redhat.com>:
>>>>> 2016-10-24 17:09+0200, Paolo Bonzini:
>>>>>> On 24/10/2016 17:03, Radim Krčmář wrote:
>>>>> [...]
>>>>>>
>>>>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>>>>>>
>>>>>> 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
next prev parent reply other threads:[~2016-10-27 2:11 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-24 10:23 [PATCH v3 0/5] KVM: LAPIC: Add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-24 10:23 ` [PATCH v3 1/5] KVM: LAPIC: extract start_sw_period() to handle periodic/oneshot mode Wanpeng Li
2016-10-24 10:23 ` [PATCH v3 2/5] KVM: LAPIC: guarantee the timer is in tsc-deadline mode Wanpeng Li
2016-10-24 10:23 ` [PATCH v3 3/5] KVM: LAPIC: introduce kvm_get_lapic_target_expiration_tsc() Wanpeng Li
2016-10-24 10:23 ` [PATCH v3 4/5] KVM: LAPIC: rename start/cancel_hv_tscdeadline to start/cancel_hv_timer Wanpeng Li
2016-10-24 10:23 ` [PATCH v3 5/5] KVM: LAPIC: add APIC Timer periodic/oneshot mode VMX preemption timer support Wanpeng Li
2016-10-24 14:50 ` Radim Krčmář
2016-10-24 23:33 ` Wanpeng Li
2016-10-24 15:03 ` [PATCH 6/5] KVM: x86: fix periodic lapic timer with hrtimers Radim Krčmář
2016-10-24 15:09 ` Paolo Bonzini
2016-10-24 15:27 ` Radim Krčmář
2016-10-24 23:39 ` Wanpeng Li
2016-10-25 11:43 ` Radim Krčmář
2016-10-25 11:55 ` Paolo Bonzini
2016-10-26 6:02 ` Wanpeng Li
2016-10-26 6:08 ` Wanpeng Li
2016-10-26 14:01 ` Radim Krčmář
2016-10-27 2:33 ` Wanpeng Li
2016-10-26 13:32 ` Radim Krčmář
2016-10-27 2:11 ` Wanpeng Li [this message]
2016-10-26 10:23 ` Wanpeng Li
2016-10-26 11:15 ` Paolo Bonzini
2016-10-26 11:26 ` Wanpeng Li
[not found] ` <SG2PR02MB1550E0FF04F2614BE0E262BC80A80@SG2PR02MB1550.apcprd02.prod.outlook.com>
2016-10-25 13:03 ` Radim Krčmář
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CANRm+CwP+o0TKzhfftiMpUoesmQeAw1N=maXmVcjpFsUwLWPqg@mail.gmail.com' \
--to=kernellwp@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=rkrcmar@redhat.com \
--cc=wanpeng.li@hotmail.com \
--cc=yunhong.jiang@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).