Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1676307 > unrolled thread
| Started by | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| First post | 2017-06-28 03:30 +0200 |
| Last post | 2017-06-29 14:20 +0200 |
| Articles | 15 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-28 03:30 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-28 14:20 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-28 16:00 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-28 16:10 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-28 16:10 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-28 16:30 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-28 16:40 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-29 05:50 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-29 10:00 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-29 10:20 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-29 10:50 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-29 13:50 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-29 14:00 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Wanpeng Li <kernellwp@gmail.com> - 2017-06-29 14:10 +0200
Re: [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay Paolo Bonzini <pbonzini@redhat.com> - 2017-06-29 14:20 +0200
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-28 03:30 +0200 |
| Subject | [PATCH v3] KVM: LAPIC: Fix lapic timer injection delay |
| Message-ID | <tX9XP-bC-3@gated-at.bofh.it> |
From: Wanpeng Li <wanpeng.li@hotmail.com>
If the TSC deadline timer is programmed really close to the deadline or
even in the past, the computation in vmx_set_hv_timer will program the
absolute target tsc value to vmcs preemption timer field w/ delta == 0,
then plays a vmentry and an upcoming vmx preemption timer fire vmexit
dance, the lapic timer injection is delayed due to this duration. Actually
the lapic timer which is emulated by hrtimer can handle this correctly.
This patch fixes it by firing the lapic timer and injecting a timer interrupt
immediately during the next vmentry if the TSC deadline timer is programmed
really close to the deadline or even in the past. This saves ~300 cycles on
the tsc_deadline_timer test of apic.flat.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v2 -> v3:
* update the patch description
arch/x86/kvm/lapic.c | 7 ++++++-
arch/x86/kvm/vmx.c | 3 +++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d24c874..08cf73d 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1504,12 +1504,17 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
+ int ret = 0;
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;
+ }
} else {
apic->lapic_timer.hv_timer_in_use = true;
hrtimer_cancel(&apic->lapic_timer.timer);
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 2e906cf..2008e9b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11149,6 +11149,9 @@ static int vmx_set_hv_timer(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc)
u64 guest_tscl = kvm_read_l1_tsc(vcpu, tscl);
u64 delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl;
+ if (delta_tsc == 0)
+ return 1;
+
/* Convert to host delta tsc if tsc scaling is enabled */
if (vcpu->arch.tsc_scaling_ratio != kvm_default_tsc_scaling_ratio &&
u64_shl_div_u64(delta_tsc,
--
2.7.4
[toc] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-28 14:20 +0200 |
| Message-ID | <tXk6S-6H0-5@gated-at.bofh.it> |
| In reply to | #1676307 |
On 28/06/2017 03:29, Wanpeng Li wrote:
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
> + int ret = 0;
>
> 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.
You could use something like this:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d24c8742d9b0..15b751aa7625 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
+ bool need_cancel = apic->lapic_timer.hv_timer_in_use;
+ if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
+ int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
+ if (r >= 0) {
+ need_cancel = false;
+ apic->lapic_timer.hv_timer_in_use = true;
+ hrtimer_cancel(&apic->lapic_timer.timer);
- if ((atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic)) ||
- kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
- if (apic->lapic_timer.hv_timer_in_use)
- cancel_hv_timer(apic);
- } else {
- apic->lapic_timer.hv_timer_in_use = true;
- hrtimer_cancel(&apic->lapic_timer.timer);
-
- /* In case the sw timer triggered in the window */
- if (atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic))
- cancel_hv_timer(apic);
+ /* In case the sw timer triggered in the window */
+ if (atomic_read(&apic->lapic_timer.pending) &&
+ !apic_lvtt_period(apic))
+ need_cancel = true;
+ else if (r)
+ kvm_lapic_expired_hv_timer(vcpu);
+ }
}
+
+ if (need_cancel)
+ cancel_hv_timer(apic);
+
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
apic->lapic_timer.hv_timer_in_use);
return apic->lapic_timer.hv_timer_in_use;
but I'm afraid of introducing a mutual recursion between
start_hv_timer and kvm_lapic_expired_hv_timer.
Paolo
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-28 16:00 +0200 |
| Message-ID | <tXlFE-7uy-3@gated-at.bofh.it> |
| In reply to | #1676609 |
2017-06-28 20:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 28/06/2017 03:29, Wanpeng Li wrote:
>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>> + int ret = 0;
>>
>> 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, there is no hrtimer start
for the next period in start_sw_period(). If it is also buggy?
Regards,
Wanpeng Li
>
> You could use something like this:
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d24c8742d9b0..15b751aa7625 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
> static bool start_hv_timer(struct kvm_lapic *apic)
> {
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
> + if (r >= 0) {
> + need_cancel = false;
> + apic->lapic_timer.hv_timer_in_use = true;
> + hrtimer_cancel(&apic->lapic_timer.timer);
>
> - if ((atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic)) ||
> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> - if (apic->lapic_timer.hv_timer_in_use)
> - cancel_hv_timer(apic);
> - } else {
> - apic->lapic_timer.hv_timer_in_use = true;
> - hrtimer_cancel(&apic->lapic_timer.timer);
> -
> - /* In case the sw timer triggered in the window */
> - if (atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic))
> - cancel_hv_timer(apic);
> + /* In case the sw timer triggered in the window */
> + if (atomic_read(&apic->lapic_timer.pending) &&
> + !apic_lvtt_period(apic))
> + need_cancel = true;
> + else if (r)
> + kvm_lapic_expired_hv_timer(vcpu);
> + }
> }
> +
> + if (need_cancel)
> + cancel_hv_timer(apic);
> +
> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
> apic->lapic_timer.hv_timer_in_use);
> return apic->lapic_timer.hv_timer_in_use;
>
> but I'm afraid of introducing a mutual recursion between
> start_hv_timer and kvm_lapic_expired_hv_timer.
>
> Paolo
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-28 16:10 +0200 |
| Message-ID | <tXlPj-7Na-3@gated-at.bofh.it> |
| In reply to | #1676674 |
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
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-28 16:10 +0200 |
| Message-ID | <tXlPk-7Na-17@gated-at.bofh.it> |
| In reply to | #1676688 |
2017-06-28 22:00 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> 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? ;)
Indeed. :)
>
>> , 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.
Ah, yes, I miss the apic_lvtt_oneshot() check here.
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-28 16:30 +0200 |
| Message-ID | <tXm8F-7TU-1@gated-at.bofh.it> |
| In reply to | #1676609 |
2017-06-28 20:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 28/06/2017 03:29, Wanpeng Li wrote:
>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>> + int ret = 0;
>>
>> 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.
>
> You could use something like this:
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d24c8742d9b0..15b751aa7625 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
> static bool start_hv_timer(struct kvm_lapic *apic)
> {
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
> + if (r >= 0) {
> + need_cancel = false;
> + apic->lapic_timer.hv_timer_in_use = true;
> + hrtimer_cancel(&apic->lapic_timer.timer);
>
> - if ((atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic)) ||
> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> - if (apic->lapic_timer.hv_timer_in_use)
> - cancel_hv_timer(apic);
> - } else {
> - apic->lapic_timer.hv_timer_in_use = true;
> - hrtimer_cancel(&apic->lapic_timer.timer);
> -
> - /* In case the sw timer triggered in the window */
> - if (atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic))
> - cancel_hv_timer(apic);
> + /* In case the sw timer triggered in the window */
> + if (atomic_read(&apic->lapic_timer.pending) &&
> + !apic_lvtt_period(apic))
> + need_cancel = true;
> + else if (r)
> + kvm_lapic_expired_hv_timer(vcpu);
> + }
> }
> +
> + if (need_cancel)
> + cancel_hv_timer(apic);
> +
> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
> apic->lapic_timer.hv_timer_in_use);
> return apic->lapic_timer.hv_timer_in_use;
>
> but I'm afraid of introducing a mutual recursion between
> start_hv_timer and kvm_lapic_expired_hv_timer.
We can just handle the apic timer oneshot/tscdeadline mode instead of
periodic mode just like which is emulated by hrtimer to avoid the
mutual recusion, what do you think?
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-28 16:40 +0200 |
| Message-ID | <tXmim-7Xs-19@gated-at.bofh.it> |
| In reply to | #1676707 |
On 28/06/2017 16:27, Wanpeng Li wrote:
> 2017-06-28 20:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>
>>
>> On 28/06/2017 03:29, Wanpeng Li wrote:
>>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>>> + int ret = 0;
>>>
>>> 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.
>>
>> You could use something like this:
>>
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index d24c8742d9b0..15b751aa7625 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
>> static bool start_hv_timer(struct kvm_lapic *apic)
>> {
>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
>> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
>> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
>> + if (r >= 0) {
>> + need_cancel = false;
>> + apic->lapic_timer.hv_timer_in_use = true;
>> + hrtimer_cancel(&apic->lapic_timer.timer);
>>
>> - if ((atomic_read(&apic->lapic_timer.pending) &&
>> - !apic_lvtt_period(apic)) ||
>> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
>> - if (apic->lapic_timer.hv_timer_in_use)
>> - cancel_hv_timer(apic);
>> - } else {
>> - apic->lapic_timer.hv_timer_in_use = true;
>> - hrtimer_cancel(&apic->lapic_timer.timer);
>> -
>> - /* In case the sw timer triggered in the window */
>> - if (atomic_read(&apic->lapic_timer.pending) &&
>> - !apic_lvtt_period(apic))
>> - cancel_hv_timer(apic);
>> + /* In case the sw timer triggered in the window */
>> + if (atomic_read(&apic->lapic_timer.pending) &&
>> + !apic_lvtt_period(apic))
>> + need_cancel = true;
>> + else if (r)
>> + kvm_lapic_expired_hv_timer(vcpu);
>> + }
>> }
>> +
>> + if (need_cancel)
>> + cancel_hv_timer(apic);
>> +
>> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
>> apic->lapic_timer.hv_timer_in_use);
>> return apic->lapic_timer.hv_timer_in_use;
>>
>> but I'm afraid of introducing a mutual recursion between
>> start_hv_timer and kvm_lapic_expired_hv_timer.
>
> We can just handle the apic timer oneshot/tscdeadline mode instead of
> periodic mode just like which is emulated by hrtimer to avoid the
> mutual recusion, what do you think?
In that case, set_hv_timer should probably always enable the preemption
timer. You can then cancel it if it returns 1 _and_ the APIC timer's
mode is oneshot/tscdeadline.
Paolo
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-29 05:50 +0200 |
| Message-ID | <tXyCR-2nE-3@gated-at.bofh.it> |
| In reply to | #1676725 |
2017-06-28 22:30 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 28/06/2017 16:27, Wanpeng Li wrote:
>> 2017-06-28 20:10 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>>
>>>
>>> On 28/06/2017 03:29, Wanpeng Li wrote:
>>>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>>>> + int ret = 0;
>>>>
>>>> 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.
>>>
>>> You could use something like this:
>>>
>>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>>> index d24c8742d9b0..15b751aa7625 100644
>>> --- a/arch/x86/kvm/lapic.c
>>> +++ b/arch/x86/kvm/lapic.c
>>> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
>>> static bool start_hv_timer(struct kvm_lapic *apic)
>>> {
>>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>>> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
>>> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
>>> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
>>> + if (r >= 0) {
>>> + need_cancel = false;
>>> + apic->lapic_timer.hv_timer_in_use = true;
>>> + hrtimer_cancel(&apic->lapic_timer.timer);
>>>
>>> - if ((atomic_read(&apic->lapic_timer.pending) &&
>>> - !apic_lvtt_period(apic)) ||
>>> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
>>> - if (apic->lapic_timer.hv_timer_in_use)
>>> - cancel_hv_timer(apic);
>>> - } else {
>>> - apic->lapic_timer.hv_timer_in_use = true;
>>> - hrtimer_cancel(&apic->lapic_timer.timer);
>>> -
>>> - /* In case the sw timer triggered in the window */
>>> - if (atomic_read(&apic->lapic_timer.pending) &&
>>> - !apic_lvtt_period(apic))
>>> - cancel_hv_timer(apic);
>>> + /* In case the sw timer triggered in the window */
>>> + if (atomic_read(&apic->lapic_timer.pending) &&
>>> + !apic_lvtt_period(apic))
>>> + need_cancel = true;
>>> + else if (r)
>>> + kvm_lapic_expired_hv_timer(vcpu);
>>> + }
>>> }
>>> +
>>> + if (need_cancel)
>>> + cancel_hv_timer(apic);
>>> +
>>> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
>>> apic->lapic_timer.hv_timer_in_use);
>>> return apic->lapic_timer.hv_timer_in_use;
>>>
>>> but I'm afraid of introducing a mutual recursion between
>>> start_hv_timer and kvm_lapic_expired_hv_timer.
>>
>> We can just handle the apic timer oneshot/tscdeadline mode instead of
>> periodic mode just like which is emulated by hrtimer to avoid the
>> mutual recusion, what do you think?
>
> In that case, set_hv_timer should probably always enable the preemption
> timer. You can then cancel it if it returns 1 _and_ the APIC timer's
> mode is oneshot/tscdeadline.
>
> Paolo
So how about something like this?
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d24c874..900ce86 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
+ bool need_cancel = apic->lapic_timer.hv_timer_in_use;
+ if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
+ int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
+ if (r >= 0) {
+ need_cancel = false;
+ apic->lapic_timer.hv_timer_in_use = true;
+ hrtimer_cancel(&apic->lapic_timer.timer);
+
+ /* In case the sw timer triggered in the window */
+ if (atomic_read(&apic->lapic_timer.pending) &&
+ !apic_lvtt_period(apic))
+ need_cancel = true;
+ else if (r && (apic_lvtt_oneshot(apic) ||
apic_lvtt_tscdeadline(apic)))
+ apic_timer_expired(apic);
+ }
+ }
- if ((atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic)) ||
- kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
- if (apic->lapic_timer.hv_timer_in_use)
- cancel_hv_timer(apic);
- } else {
- apic->lapic_timer.hv_timer_in_use = true;
- hrtimer_cancel(&apic->lapic_timer.timer);
+ if (need_cancel)
+ cancel_hv_timer(apic);
- /* In case the sw timer triggered in the window */
- if (atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic))
- cancel_hv_timer(apic);
- }
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
apic->lapic_timer.hv_timer_in_use);
return apic->lapic_timer.hv_timer_in_use;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4f616db..0a0e696 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11125,6 +11125,9 @@ static int vmx_set_hv_timer(struct kvm_vcpu
*vcpu, u64 guest_deadline_tsc)
u64 guest_tscl = kvm_read_l1_tsc(vcpu, tscl);
u64 delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl;
+ if (delta_tsc == 0)
+ return 1;
+
/* Convert to host delta tsc if tsc scaling is enabled */
if (vcpu->arch.tsc_scaling_ratio != kvm_default_tsc_scaling_ratio &&
u64_shl_div_u64(delta_tsc,
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-29 10:00 +0200 |
| Message-ID | <tXCwP-4ON-29@gated-at.bofh.it> |
| In reply to | #1677362 |
On 29/06/2017 05:42, Wanpeng Li wrote:
> So how about something like this?
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index d24c874..900ce86 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
> static bool start_hv_timer(struct kvm_lapic *apic)
> {
> u64 tscdeadline = apic->lapic_timer.tscdeadline;
> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
> + if (r >= 0) {
> + need_cancel = false;
> + apic->lapic_timer.hv_timer_in_use = true;
> + hrtimer_cancel(&apic->lapic_timer.timer);
> +
> + /* In case the sw timer triggered in the window */
> + if (atomic_read(&apic->lapic_timer.pending) &&
> + !apic_lvtt_period(apic))
> + need_cancel = true;
> + else if (r && (apic_lvtt_oneshot(apic) ||
> apic_lvtt_tscdeadline(apic)))
> + apic_timer_expired(apic);
> + }
> + }
>
> - if ((atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic)) ||
> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> - if (apic->lapic_timer.hv_timer_in_use)
> - cancel_hv_timer(apic);
> - } else {
> - apic->lapic_timer.hv_timer_in_use = true;
> - hrtimer_cancel(&apic->lapic_timer.timer);
> + if (need_cancel)
> + cancel_hv_timer(apic);
>
> - /* In case the sw timer triggered in the window */
> - if (atomic_read(&apic->lapic_timer.pending) &&
> - !apic_lvtt_period(apic))
> - cancel_hv_timer(apic);
> - }
> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
> apic->lapic_timer.hv_timer_in_use);
> return apic->lapic_timer.hv_timer_in_use;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4f616db..0a0e696 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -11125,6 +11125,9 @@ static int vmx_set_hv_timer(struct kvm_vcpu
> *vcpu, u64 guest_deadline_tsc)
> u64 guest_tscl = kvm_read_l1_tsc(vcpu, tscl);
> u64 delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl;
>
> + if (delta_tsc == 0)
> + return 1;
You still need to enable the preemption timer even if you return 1, so
in lapic.c it becomes
if (!apic_lvtt_period(apic)) {
if (r)
apic_timer_expired(apic);
if (atomic_read(&apic->lapic_timer.pending))
need_cancel = true;
}
Otherwise it looks good. Thanks for your persistence. :)
Paolo
> /* Convert to host delta tsc if tsc scaling is enabled */
> if (vcpu->arch.tsc_scaling_ratio != kvm_default_tsc_scaling_ratio &&
> u64_shl_div_u64(delta_tsc,
>
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-29 10:20 +0200 |
| Message-ID | <tXCQa-5a5-9@gated-at.bofh.it> |
| In reply to | #1677506 |
2017-06-29 15:55 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>> +
>> + /* In case the sw timer triggered in the window */
>> + if (atomic_read(&apic->lapic_timer.pending) &&
>> + !apic_lvtt_period(apic))
>> + need_cancel = true;
>> + else if (r && (apic_lvtt_oneshot(apic) ||
>> apic_lvtt_tscdeadline(apic)))
>> + apic_timer_expired(apic);
>> + }
>> + }
[...]
>
> You still need to enable the preemption timer even if you return 1, so
> in lapic.c it becomes
>
> if (!apic_lvtt_period(apic)) {
> if (r)
> apic_timer_expired(apic);
> if (atomic_read(&apic->lapic_timer.pending))
> need_cancel = true;
> }
I think the codes are more clear but the same as above. We didn't
program preemption timer vmcs field if delta == 0, so how to
understand "need to enable the preemption timer even if return 1"?
>
>
> Otherwise it looks good. Thanks for your persistence. :)
Thanks for your help and patient. :)
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-29 10:50 +0200 |
| Message-ID | <tXDjb-5jp-11@gated-at.bofh.it> |
| In reply to | #1677511 |
2017-06-29 16:17 GMT+08:00 Wanpeng Li <kernellwp@gmail.com>:
> 2017-06-29 15:55 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>>> +
>>> + /* In case the sw timer triggered in the window */
>>> + if (atomic_read(&apic->lapic_timer.pending) &&
>>> + !apic_lvtt_period(apic))
>>> + need_cancel = true;
>>> + else if (r && (apic_lvtt_oneshot(apic) ||
>>> apic_lvtt_tscdeadline(apic)))
>>> + apic_timer_expired(apic);
>>> + }
>>> + }
>
> [...]
>
>>
>> You still need to enable the preemption timer even if you return 1, so
>> in lapic.c it becomes
>>
>> if (!apic_lvtt_period(apic)) {
>> if (r)
>> apic_timer_expired(apic);
>> if (atomic_read(&apic->lapic_timer.pending))
>> need_cancel = true;
>> }
>
> I think the codes are more clear but the same as above. We didn't
> program preemption timer vmcs field if delta == 0, so how to
> understand "need to enable the preemption timer even if return 1"?
I guess you mean start_hv_timer() should return true, right?
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-29 13:50 +0200 |
| Message-ID | <tXG7o-71O-21@gated-at.bofh.it> |
| In reply to | #1677530 |
On 29/06/2017 10:44, Wanpeng Li wrote:
>>> You still need to enable the preemption timer even if you return 1, so
>>> in lapic.c it becomes
>>>
>>> if (!apic_lvtt_period(apic)) {
>>> if (r)
>>> apic_timer_expired(apic);
>>> if (atomic_read(&apic->lapic_timer.pending))
>>> need_cancel = true;
>>> }
>> I think the codes are more clear but the same as above. We didn't
>> program preemption timer vmcs field if delta == 0, so how to
>> understand "need to enable the preemption timer even if return 1"?
> I guess you mean start_hv_timer() should return true, right?
vmx.c's set_hv_timer callback should set the preemption timer execution
control. Otherwise, kvm_lapic_hv_timer_expired is again not called.
Paolo
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-29 14:00 +0200 |
| Message-ID | <tXGh3-774-5@gated-at.bofh.it> |
| In reply to | #1677646 |
2017-06-29 19:43 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 29/06/2017 10:44, Wanpeng Li wrote:
>>>> You still need to enable the preemption timer even if you return 1, so
>>>> in lapic.c it becomes
>>>>
>>>> if (!apic_lvtt_period(apic)) {
>>>> if (r)
>>>> apic_timer_expired(apic);
>>>> if (atomic_read(&apic->lapic_timer.pending))
>>>> need_cancel = true;
>>>> }
>>> I think the codes are more clear but the same as above. We didn't
>>> program preemption timer vmcs field if delta == 0, so how to
>>> understand "need to enable the preemption timer even if return 1"?
>> I guess you mean start_hv_timer() should return true, right?
>
> vmx.c's set_hv_timer callback should set the preemption timer execution
> control. Otherwise, kvm_lapic_hv_timer_expired is again not called.
I see, this should be set for apic timer periodic mode.
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Wanpeng Li <kernellwp@gmail.com> |
|---|---|
| Date | 2017-06-29 14:10 +0200 |
| Message-ID | <tXGqL-7pH-37@gated-at.bofh.it> |
| In reply to | #1677506 |
2017-06-29 15:55 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 29/06/2017 05:42, Wanpeng Li wrote:
>> So how about something like this?
>>
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index d24c874..900ce86 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -1504,21 +1504,26 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
>> static bool start_hv_timer(struct kvm_lapic *apic)
>> {
>> u64 tscdeadline = apic->lapic_timer.tscdeadline;
>> + bool need_cancel = apic->lapic_timer.hv_timer_in_use;
>> + if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
>> + int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
>> + if (r >= 0) {
>> + need_cancel = false;
>> + apic->lapic_timer.hv_timer_in_use = true;
>> + hrtimer_cancel(&apic->lapic_timer.timer);
>> +
>> + /* In case the sw timer triggered in the window */
>> + if (atomic_read(&apic->lapic_timer.pending) &&
>> + !apic_lvtt_period(apic))
>> + need_cancel = true;
>> + else if (r && (apic_lvtt_oneshot(apic) ||
>> apic_lvtt_tscdeadline(apic)))
>> + apic_timer_expired(apic);
>> + }
>> + }
>>
>> - if ((atomic_read(&apic->lapic_timer.pending) &&
>> - !apic_lvtt_period(apic)) ||
>> - kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
>> - if (apic->lapic_timer.hv_timer_in_use)
>> - cancel_hv_timer(apic);
>> - } else {
>> - apic->lapic_timer.hv_timer_in_use = true;
>> - hrtimer_cancel(&apic->lapic_timer.timer);
>> + if (need_cancel)
>> + cancel_hv_timer(apic);
>>
>> - /* In case the sw timer triggered in the window */
>> - if (atomic_read(&apic->lapic_timer.pending) &&
>> - !apic_lvtt_period(apic))
>> - cancel_hv_timer(apic);
>> - }
>> trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
>> apic->lapic_timer.hv_timer_in_use);
>> return apic->lapic_timer.hv_timer_in_use;
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 4f616db..0a0e696 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -11125,6 +11125,9 @@ static int vmx_set_hv_timer(struct kvm_vcpu
>> *vcpu, u64 guest_deadline_tsc)
>> u64 guest_tscl = kvm_read_l1_tsc(vcpu, tscl);
>> u64 delta_tsc = max(guest_deadline_tsc, guest_tscl) - guest_tscl;
>>
>> + if (delta_tsc == 0)
>> + return 1;
>
> You still need to enable the preemption timer even if you return 1, so
> in lapic.c it becomes
>
> if (!apic_lvtt_period(apic)) {
> if (r)
> apic_timer_expired(apic);
> if (atomic_read(&apic->lapic_timer.pending))
> need_cancel = true;
> }
As you point out, we should cancel the preemption timer if it returns
1 and the APIC timer's mode is oneshot/tscdeadline. How about
something like this:
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index d24c874..b801385 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1504,21 +1504,28 @@ static void cancel_hv_timer(struct kvm_lapic *apic)
static bool start_hv_timer(struct kvm_lapic *apic)
{
u64 tscdeadline = apic->lapic_timer.tscdeadline;
+ bool need_cancel = apic->lapic_timer.hv_timer_in_use;
+ if (!atomic_read(&apic->lapic_timer.pending) || apic_lvtt_period(apic)) {
+ int r = kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline);
+ if (r >= 0) {
+ need_cancel = false;
+ apic->lapic_timer.hv_timer_in_use = true;
+ hrtimer_cancel(&apic->lapic_timer.timer);
+
+ /* In case the sw timer triggered in the window */
+ if (!apic_lvtt_period(apic)) {
+ if (r || atomic_read(&apic->lapic_timer.pending)) {
+ need_cancel = true;
+ if (r)
+ apic_timer_expired(apic);
+ }
+ }
+ }
+ }
- if ((atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic)) ||
- kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
- if (apic->lapic_timer.hv_timer_in_use)
- cancel_hv_timer(apic);
- } else {
- apic->lapic_timer.hv_timer_in_use = true;
- hrtimer_cancel(&apic->lapic_timer.timer);
+ if (need_cancel)
+ cancel_hv_timer(apic);
- /* In case the sw timer triggered in the window */
- if (atomic_read(&apic->lapic_timer.pending) &&
- !apic_lvtt_period(apic))
- cancel_hv_timer(apic);
- }
trace_kvm_hv_timer_state(apic->vcpu->vcpu_id,
apic->lapic_timer.hv_timer_in_use);
return apic->lapic_timer.hv_timer_in_use;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ca5d2b9..b8bde13 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11134,7 +11134,11 @@ static int vmx_set_hv_timer(struct kvm_vcpu
*vcpu, u64 guest_deadline_tsc)
vmx->hv_deadline_tsc = tscl + delta_tsc;
vmcs_set_bits(PIN_BASED_VM_EXEC_CONTROL,
PIN_BASED_VMX_PREEMPTION_TIMER);
- return 0;
+
+ if (delta_tsc == 0)
+ return 1;
+ else
+ return 0;
}
static void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu)
Regards,
Wanpeng Li
[toc] | [prev] | [next] | [standalone]
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Date | 2017-06-29 14:20 +0200 |
| Message-ID | <tXGAp-7t8-3@gated-at.bofh.it> |
| In reply to | #1677661 |
On 29/06/2017 14:05, Wanpeng Li wrote:
> + /* In case the sw timer triggered in the window */
> + if (!apic_lvtt_period(apic)) {
> + if (r || atomic_read(&apic->lapic_timer.pending)) {
> + need_cancel = true;
> + if (r)
> + apic_timer_expired(apic);
> + }
> + }
Yes, that's equivalent. The compiler should thread the jumps as if it were:
if (r) {
apic_timer_expired(apic);
goto cancel_timer;
}
if (atomic_read(&apic->lapic_timer.pending))
goto cancel_timer;
so it produces pretty good code too.
Paolo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web