When a vCPU runs on a nohz_full core, the hrtimer used by
the lapic emulation code can be migrated to another core.
When this happens, it's possible to observe milisecond
latency when delivering timer IRQs to KVM guests.
The huge latency is mainly due to the fact that
apic_timer_fn() expects to run during a kvm exit. It
sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
entry. However, if the timer fires on a different core,
we have to wait until the next kvm exit for the guest
to see KVM_REQ_PENDING_TIMER set.
This problem became visible after commit 9642d18ee. This
commit changed the timer migration code to always attempt
to migrate timers away from nohz_full cores. While it's
discussable if this is correct/desirable (I don't think
it is), it's clear that the lapic emulation code has
a requirement on firing the hrtimer in the same core
where it was started. This is achieved by making the
hrtimer pinned.
Lastly, note that KVM has code to migrate timers when a
vCPU is scheduled to run in different core. However, this
forced migration may fail. When this happens, we can have
the same problem. If we want 100% correctness, we'll have
to modify apic_timer_fn() to cause a kvm exit when it runs
on a different core than the vCPU. Not sure if this is
possible.
Here's a reproducer for the issue being fixed:
1. Set all cores but core0 to be nohz_full cores
2. Start a guest with a single vCPU
3. Trace apic_timer_fn() and kvm_inject_apic_timer_irqs()
You'll see that apic_timer_fn() will run in core0 while
kvm_inject_apic_timer_irqs() runs in a different core. If
you get both on core0, try running a program that takes 100%
of the CPU and pin it to core0 to force the vCPU out.
Signed-off-by: Luiz Capitulino <[email protected]>
---
arch/x86/kvm/lapic.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 443d2a5..1a2da0e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1369,7 +1369,7 @@ static void start_apic_timer(struct kvm_lapic *apic)
hrtimer_start(&apic->lapic_timer.timer,
ktime_add_ns(now, apic->lapic_timer.period),
- HRTIMER_MODE_ABS);
+ HRTIMER_MODE_ABS_PINNED);
apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
PRIx64 ", "
@@ -1402,7 +1402,7 @@ static void start_apic_timer(struct kvm_lapic *apic)
expire = ktime_add_ns(now, ns);
expire = ktime_sub_ns(expire, lapic_timer_advance_ns);
hrtimer_start(&apic->lapic_timer.timer,
- expire, HRTIMER_MODE_ABS);
+ expire, HRTIMER_MODE_ABS_PINNED);
} else
apic_timer_expired(apic);
@@ -1868,7 +1868,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu)
apic->vcpu = vcpu;
hrtimer_init(&apic->lapic_timer.timer, CLOCK_MONOTONIC,
- HRTIMER_MODE_ABS);
+ HRTIMER_MODE_ABS_PINNED);
apic->lapic_timer.timer.function = apic_timer_fn;
/*
@@ -2003,7 +2003,7 @@ void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
timer = &vcpu->arch.apic->lapic_timer.timer;
if (hrtimer_cancel(timer))
- hrtimer_start_expires(timer, HRTIMER_MODE_ABS);
+ hrtimer_start_expires(timer, HRTIMER_MODE_ABS_PINNED);
}
/*
--
2.1.0
On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
> When a vCPU runs on a nohz_full core, the hrtimer used by
> the lapic emulation code can be migrated to another core.
> When this happens, it's possible to observe milisecond
> latency when delivering timer IRQs to KVM guests.
>
> The huge latency is mainly due to the fact that
> apic_timer_fn() expects to run during a kvm exit. It
> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
> entry. However, if the timer fires on a different core,
> we have to wait until the next kvm exit for the guest
> to see KVM_REQ_PENDING_TIMER set.
>
> This problem became visible after commit 9642d18ee. This
> commit changed the timer migration code to always attempt
> to migrate timers away from nohz_full cores. While it's
> discussable if this is correct/desirable (I don't think
> it is), it's clear that the lapic emulation code has
> a requirement on firing the hrtimer in the same core
> where it was started. This is achieved by making the
> hrtimer pinned.
Given that delivering a timer to a guest seems to
involve trapping from the guest to the host, anyway,
I don't see a downside to your patch.
If that is ever changed (eg. allowing delivery of
a timer interrupt to a VCPU without trapping to the
host), we may want to revisit this.
Until then...
Acked-by: Rik van Riel <[email protected]>
--
All Rights Reversed.
On 2016/4/5 5:00, Rik van Riel wrote:
> On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
>> When a vCPU runs on a nohz_full core, the hrtimer used by
>> the lapic emulation code can be migrated to another core.
>> When this happens, it's possible to observe milisecond
>> latency when delivering timer IRQs to KVM guests.
>>
>> The huge latency is mainly due to the fact that
>> apic_timer_fn() expects to run during a kvm exit. It
>> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
>> entry. However, if the timer fires on a different core,
>> we have to wait until the next kvm exit for the guest
>> to see KVM_REQ_PENDING_TIMER set.
>>
>> This problem became visible after commit 9642d18ee. This
>> commit changed the timer migration code to always attempt
>> to migrate timers away from nohz_full cores. While it's
>> discussable if this is correct/desirable (I don't think
>> it is), it's clear that the lapic emulation code has
>> a requirement on firing the hrtimer in the same core
>> where it was started. This is achieved by making the
>> hrtimer pinned.
>
> Given that delivering a timer to a guest seems to
> involve trapping from the guest to the host, anyway,
> I don't see a downside to your patch.
>
> If that is ever changed (eg. allowing delivery of
> a timer interrupt to a VCPU without trapping to the
> host), we may want to revisit this.
Posted interrupt helps in this case. Currently, KVM doesn't use PI for
lapic timer is due to same affinity for lapic timer and VCPU. Now, we
can change to use PI for lapic timer. The only concern is what's
frequency of timer migration in upstream Linux? If it is frequently,
will it bring additional cost?
BTW, in what case the migration of timers during VCPU scheduling will fail?
--
best regards
yang
On 04/04/2016 22:46, Luiz Capitulino wrote:
> When a vCPU runs on a nohz_full core, the hrtimer used by
> the lapic emulation code can be migrated to another core.
> When this happens, it's possible to observe milisecond
> latency when delivering timer IRQs to KVM guests.
>
> The huge latency is mainly due to the fact that
> apic_timer_fn() expects to run during a kvm exit. It
> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
> entry. However, if the timer fires on a different core,
> we have to wait until the next kvm exit for the guest
> to see KVM_REQ_PENDING_TIMER set.
>
> This problem became visible after commit 9642d18ee. This
> commit changed the timer migration code to always attempt
> to migrate timers away from nohz_full cores. While it's
> discussable if this is correct/desirable (I don't think
> it is), it's clear that the lapic emulation code has
> a requirement on firing the hrtimer in the same core
> where it was started. This is achieved by making the
> hrtimer pinned.
>
> Lastly, note that KVM has code to migrate timers when a
> vCPU is scheduled to run in different core. However, this
> forced migration may fail. When this happens, we can have
> the same problem. If we want 100% correctness, we'll have
> to modify apic_timer_fn() to cause a kvm exit when it runs
> on a different core than the vCPU. Not sure if this is
> possible.
>
> Here's a reproducer for the issue being fixed:
>
> 1. Set all cores but core0 to be nohz_full cores
> 2. Start a guest with a single vCPU
> 3. Trace apic_timer_fn() and kvm_inject_apic_timer_irqs()
>
> You'll see that apic_timer_fn() will run in core0 while
> kvm_inject_apic_timer_irqs() runs in a different core. If
> you get both on core0, try running a program that takes 100%
> of the CPU and pin it to core0 to force the vCPU out.
>
> Signed-off-by: Luiz Capitulino <[email protected]>
> ---
> arch/x86/kvm/lapic.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 443d2a5..1a2da0e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -1369,7 +1369,7 @@ static void start_apic_timer(struct kvm_lapic *apic)
>
> hrtimer_start(&apic->lapic_timer.timer,
> ktime_add_ns(now, apic->lapic_timer.period),
> - HRTIMER_MODE_ABS);
> + HRTIMER_MODE_ABS_PINNED);
>
> apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
> PRIx64 ", "
> @@ -1402,7 +1402,7 @@ static void start_apic_timer(struct kvm_lapic *apic)
> expire = ktime_add_ns(now, ns);
> expire = ktime_sub_ns(expire, lapic_timer_advance_ns);
> hrtimer_start(&apic->lapic_timer.timer,
> - expire, HRTIMER_MODE_ABS);
> + expire, HRTIMER_MODE_ABS_PINNED);
> } else
> apic_timer_expired(apic);
>
> @@ -1868,7 +1868,7 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu)
> apic->vcpu = vcpu;
>
> hrtimer_init(&apic->lapic_timer.timer, CLOCK_MONOTONIC,
> - HRTIMER_MODE_ABS);
> + HRTIMER_MODE_ABS_PINNED);
> apic->lapic_timer.timer.function = apic_timer_fn;
>
> /*
> @@ -2003,7 +2003,7 @@ void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu)
>
> timer = &vcpu->arch.apic->lapic_timer.timer;
> if (hrtimer_cancel(timer))
> - hrtimer_start_expires(timer, HRTIMER_MODE_ABS);
> + hrtimer_start_expires(timer, HRTIMER_MODE_ABS_PINNED);
> }
>
> /*
>
Queued for 4.6.0-rc3, thanks.
Paolo
On Tue, 5 Apr 2016 14:18:01 +0800
Yang Zhang <[email protected]> wrote:
> On 2016/4/5 5:00, Rik van Riel wrote:
> > On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
> >> When a vCPU runs on a nohz_full core, the hrtimer used by
> >> the lapic emulation code can be migrated to another core.
> >> When this happens, it's possible to observe milisecond
> >> latency when delivering timer IRQs to KVM guests.
> >>
> >> The huge latency is mainly due to the fact that
> >> apic_timer_fn() expects to run during a kvm exit. It
> >> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
> >> entry. However, if the timer fires on a different core,
> >> we have to wait until the next kvm exit for the guest
> >> to see KVM_REQ_PENDING_TIMER set.
> >>
> >> This problem became visible after commit 9642d18ee. This
> >> commit changed the timer migration code to always attempt
> >> to migrate timers away from nohz_full cores. While it's
> >> discussable if this is correct/desirable (I don't think
> >> it is), it's clear that the lapic emulation code has
> >> a requirement on firing the hrtimer in the same core
> >> where it was started. This is achieved by making the
> >> hrtimer pinned.
> >
> > Given that delivering a timer to a guest seems to
> > involve trapping from the guest to the host, anyway,
> > I don't see a downside to your patch.
> >
> > If that is ever changed (eg. allowing delivery of
> > a timer interrupt to a VCPU without trapping to the
> > host), we may want to revisit this.
>
>
> Posted interrupt helps in this case. Currently, KVM doesn't use PI for
> lapic timer is due to same affinity for lapic timer and VCPU. Now, we
> can change to use PI for lapic timer. The only concern is what's
> frequency of timer migration in upstream Linux? If it is frequently,
> will it bring additional cost?
I can't answer this questions.
> BTW, in what case the migration of timers during VCPU scheduling will fail?
For hrtimers (which is the lapic emulation case), it only succeeds if
the destination core has a hrtimer expiring before the hrtimer being
migrated.
Also, if the hrtimer callback function is already running (that is,
the timer fired already) it's not migrated either. But I _guess_ this
case doesn't affect KVM (and there's no much do about it anyways).
2016-04-05 14:18+0800, Yang Zhang:
> On 2016/4/5 5:00, Rik van Riel wrote:
>>Given that delivering a timer to a guest seems to
>>involve trapping from the guest to the host, anyway,
>>I don't see a downside to your patch.
>>
>>If that is ever changed (eg. allowing delivery of
>>a timer interrupt to a VCPU without trapping to the
>>host), we may want to revisit this.
>
> Posted interrupt helps in this case. Currently, KVM doesn't use PI for lapic
> timer is due to same affinity for lapic timer and VCPU. Now, we can change
> to use PI for lapic timer. The only concern is what's frequency of timer
> migration in upstream Linux? If it is frequently, will it bring additional
> cost?
It's a scheduler bug if the timer migration frequency would matter. :)
Additional costs arise when the timer and VCPU are on two different
CPUs. (e.g. if both CPUs are in deep C-state, we wasted one wakeup;
the timer would sometimes needs to send an interrupt.)
Fine tuned KVM could benefit from having the lapic timer backend on a
different physical core, but the general case would need some experience
to decide.
I think that we'd still want to have timer interrupts on the same
physical core if the host didn't have PI, and the fraction of timers
that can be injected without a guest entry is important to decide
whether PI can make the effort worthwhile.
The biggest benefit might come from handling multiple lapic timers in
one host interrupt.
On 2016/4/5 23:54, Radim Krčmář wrote:
> 2016-04-05 14:18+0800, Yang Zhang:
>> On 2016/4/5 5:00, Rik van Riel wrote:
>>> Given that delivering a timer to a guest seems to
>>> involve trapping from the guest to the host, anyway,
>>> I don't see a downside to your patch.
>>>
>>> If that is ever changed (eg. allowing delivery of
>>> a timer interrupt to a VCPU without trapping to the
>>> host), we may want to revisit this.
>>
>> Posted interrupt helps in this case. Currently, KVM doesn't use PI for lapic
>> timer is due to same affinity for lapic timer and VCPU. Now, we can change
>> to use PI for lapic timer. The only concern is what's frequency of timer
>> migration in upstream Linux? If it is frequently, will it bring additional
>> cost?
>
> It's a scheduler bug if the timer migration frequency would matter. :)
> Additional costs arise when the timer and VCPU are on two different
> CPUs. (e.g. if both CPUs are in deep C-state, we wasted one wakeup;
> the timer would sometimes needs to send an interrupt.)
Yes, it's possible. But the premise is VCPU is pinned to other CPU.
Normally, the VCPU will wake up on the same CPU where timer interrupt is
stay if CPU is idle.
>
> Fine tuned KVM could benefit from having the lapic timer backend on a
> different physical core, but the general case would need some experience
> to decide.
>
> I think that we'd still want to have timer interrupts on the same
> physical core if the host didn't have PI, and the fraction of timers
> that can be injected without a guest entry is important to decide
> whether PI can make the effort worthwhile.
Agree. I can do some experiences to see how much improvement we can get.
>
> The biggest benefit might come from handling multiple lapic timers in
> one host interrupt.
This is should be another story.We need to align multiple lapic timers
into one timer firstly.:)
--
best regards
yang
2016-04-05 20:40 GMT+08:00 Luiz Capitulino <[email protected]>:
> On Tue, 5 Apr 2016 14:18:01 +0800
> Yang Zhang <[email protected]> wrote:
>
>> On 2016/4/5 5:00, Rik van Riel wrote:
>> > On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
>> >> When a vCPU runs on a nohz_full core, the hrtimer used by
>> >> the lapic emulation code can be migrated to another core.
>> >> When this happens, it's possible to observe milisecond
>> >> latency when delivering timer IRQs to KVM guests.
>> >>
>> >> The huge latency is mainly due to the fact that
>> >> apic_timer_fn() expects to run during a kvm exit. It
>> >> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
>> >> entry. However, if the timer fires on a different core,
>> >> we have to wait until the next kvm exit for the guest
>> >> to see KVM_REQ_PENDING_TIMER set.
>> >>
>> >> This problem became visible after commit 9642d18ee. This
>> >> commit changed the timer migration code to always attempt
>> >> to migrate timers away from nohz_full cores. While it's
>> >> discussable if this is correct/desirable (I don't think
>> >> it is), it's clear that the lapic emulation code has
>> >> a requirement on firing the hrtimer in the same core
>> >> where it was started. This is achieved by making the
>> >> hrtimer pinned.
>> >
>> > Given that delivering a timer to a guest seems to
>> > involve trapping from the guest to the host, anyway,
>> > I don't see a downside to your patch.
>> >
>> > If that is ever changed (eg. allowing delivery of
>> > a timer interrupt to a VCPU without trapping to the
>> > host), we may want to revisit this.
>>
>>
>> Posted interrupt helps in this case. Currently, KVM doesn't use PI for
>> lapic timer is due to same affinity for lapic timer and VCPU. Now, we
>> can change to use PI for lapic timer. The only concern is what's
>> frequency of timer migration in upstream Linux? If it is frequently,
>> will it bring additional cost?
>
> I can't answer this questions.
>
>> BTW, in what case the migration of timers during VCPU scheduling will fail?
>
> For hrtimers (which is the lapic emulation case), it only succeeds if
> the destination core has a hrtimer expiring before the hrtimer being
> migrated.
Interesting, did you figure out why this happen? Actually the clock
event device will be reprogrammed if the expire time of the new
enqueued hrtimer is earlier than the left most(earliest expire time)
hrtimer in hrtimer rb tree.
Regards,
Wanpeng Li
>
> Also, if the hrtimer callback function is already running (that is,
> the timer fired already) it's not migrated either. But I _guess_ this
> case doesn't affect KVM (and there's no much do about it anyways).
On Fri, 22 Apr 2016 07:12:51 +0800
Wanpeng Li <[email protected]> wrote:
> 2016-04-05 20:40 GMT+08:00 Luiz Capitulino <[email protected]>:
> > On Tue, 5 Apr 2016 14:18:01 +0800
> > Yang Zhang <[email protected]> wrote:
> >
> >> On 2016/4/5 5:00, Rik van Riel wrote:
> >> > On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
> >> >> When a vCPU runs on a nohz_full core, the hrtimer used by
> >> >> the lapic emulation code can be migrated to another core.
> >> >> When this happens, it's possible to observe milisecond
> >> >> latency when delivering timer IRQs to KVM guests.
> >> >>
> >> >> The huge latency is mainly due to the fact that
> >> >> apic_timer_fn() expects to run during a kvm exit. It
> >> >> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
> >> >> entry. However, if the timer fires on a different core,
> >> >> we have to wait until the next kvm exit for the guest
> >> >> to see KVM_REQ_PENDING_TIMER set.
> >> >>
> >> >> This problem became visible after commit 9642d18ee. This
> >> >> commit changed the timer migration code to always attempt
> >> >> to migrate timers away from nohz_full cores. While it's
> >> >> discussable if this is correct/desirable (I don't think
> >> >> it is), it's clear that the lapic emulation code has
> >> >> a requirement on firing the hrtimer in the same core
> >> >> where it was started. This is achieved by making the
> >> >> hrtimer pinned.
> >> >
> >> > Given that delivering a timer to a guest seems to
> >> > involve trapping from the guest to the host, anyway,
> >> > I don't see a downside to your patch.
> >> >
> >> > If that is ever changed (eg. allowing delivery of
> >> > a timer interrupt to a VCPU without trapping to the
> >> > host), we may want to revisit this.
> >>
> >>
> >> Posted interrupt helps in this case. Currently, KVM doesn't use PI for
> >> lapic timer is due to same affinity for lapic timer and VCPU. Now, we
> >> can change to use PI for lapic timer. The only concern is what's
> >> frequency of timer migration in upstream Linux? If it is frequently,
> >> will it bring additional cost?
> >
> > I can't answer this questions.
> >
> >> BTW, in what case the migration of timers during VCPU scheduling will fail?
> >
> > For hrtimers (which is the lapic emulation case), it only succeeds if
> > the destination core has a hrtimer expiring before the hrtimer being
> > migrated.
>
> Interesting, did you figure out why this happen? Actually the clock
> event device will be reprogrammed if the expire time of the new
> enqueued hrtimer is earlier than the left most(earliest expire time)
> hrtimer in hrtimer rb tree.
Unless the code has changed very recently, what you describe is
what happens when queueing a hrtimer in the same core. Migrating a
hrtimer to a different core is a different case.
>
> Regards,
> Wanpeng Li
>
> >
> > Also, if the hrtimer callback function is already running (that is,
> > the timer fired already) it's not migrated either. But I _guess_ this
> > case doesn't affect KVM (and there's no much do about it anyways).
>
2016-04-22 21:12 GMT+08:00 Luiz Capitulino <[email protected]>:
> On Fri, 22 Apr 2016 07:12:51 +0800
> Wanpeng Li <[email protected]> wrote:
>
>> 2016-04-05 20:40 GMT+08:00 Luiz Capitulino <[email protected]>:
>> > On Tue, 5 Apr 2016 14:18:01 +0800
>> > Yang Zhang <[email protected]> wrote:
>> >
>> >> On 2016/4/5 5:00, Rik van Riel wrote:
>> >> > On Mon, 2016-04-04 at 16:46 -0400, Luiz Capitulino wrote:
>> >> >> When a vCPU runs on a nohz_full core, the hrtimer used by
>> >> >> the lapic emulation code can be migrated to another core.
>> >> >> When this happens, it's possible to observe milisecond
>> >> >> latency when delivering timer IRQs to KVM guests.
>> >> >>
>> >> >> The huge latency is mainly due to the fact that
>> >> >> apic_timer_fn() expects to run during a kvm exit. It
>> >> >> sets KVM_REQ_PENDING_TIMER and let it be handled on kvm
>> >> >> entry. However, if the timer fires on a different core,
>> >> >> we have to wait until the next kvm exit for the guest
>> >> >> to see KVM_REQ_PENDING_TIMER set.
>> >> >>
>> >> >> This problem became visible after commit 9642d18ee. This
>> >> >> commit changed the timer migration code to always attempt
>> >> >> to migrate timers away from nohz_full cores. While it's
>> >> >> discussable if this is correct/desirable (I don't think
>> >> >> it is), it's clear that the lapic emulation code has
>> >> >> a requirement on firing the hrtimer in the same core
>> >> >> where it was started. This is achieved by making the
>> >> >> hrtimer pinned.
>> >> >
>> >> > Given that delivering a timer to a guest seems to
>> >> > involve trapping from the guest to the host, anyway,
>> >> > I don't see a downside to your patch.
>> >> >
>> >> > If that is ever changed (eg. allowing delivery of
>> >> > a timer interrupt to a VCPU without trapping to the
>> >> > host), we may want to revisit this.
>> >>
>> >>
>> >> Posted interrupt helps in this case. Currently, KVM doesn't use PI for
>> >> lapic timer is due to same affinity for lapic timer and VCPU. Now, we
>> >> can change to use PI for lapic timer. The only concern is what's
>> >> frequency of timer migration in upstream Linux? If it is frequently,
>> >> will it bring additional cost?
>> >
>> > I can't answer this questions.
>> >
>> >> BTW, in what case the migration of timers during VCPU scheduling will fail?
>> >
>> > For hrtimers (which is the lapic emulation case), it only succeeds if
>> > the destination core has a hrtimer expiring before the hrtimer being
>> > migrated.
>>
>> Interesting, did you figure out why this happen? Actually the clock
>> event device will be reprogrammed if the expire time of the new
>> enqueued hrtimer is earlier than the left most(earliest expire time)
>> hrtimer in hrtimer rb tree.
>
> Unless the code has changed very recently, what you describe is
> what happens when queueing a hrtimer in the same core. Migrating a
> hrtimer to a different core is a different case.
You are right!
Regards,
Wanpeng Li