Skip to contentSkip to conversation

kvm: x86: make lapic hrtimer pinned

PATCH Applied Luiz Capitulino 6 participants 10 messages Apr 4, 2016 – Apr 23, 2016

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 kvm: x86: make lapic hrtimer pinned

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.

---
 arch/x86/kvm/lapic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kvm/lapic.c +4 -4
index 443d2a5..1a2da0e 100644
@@ -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);
 }
 
 /*
Signature · 1 line
2.1.0
Acked-by on v1 1/1
#1 lore raw
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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...

Signature · 1 line
All Rights Reversed.
#2 lore raw
Quoted 22 lines · Rik van Riel
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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.

Quoted 7 lines

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?

Signature · 2 lines
best regards
yang
#3 lore raw
Quoted 32 lines · On Tue, 5 Apr 2016 14:18:01 +0800 Yang Zhang <[email protected]> wrote:
Quoted 22 lines · Rik van Riel
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 1 line

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).

#4 lore raw

2016-04-05 20:40 GMT+08:00 Luiz Capitulino <[email protected]>:

Quoted 41 lines
Quoted 32 lines · On Tue, 5 Apr 2016 14:18:01 +0800 Yang Zhang <[email protected]> wrote:
Quoted 22 lines · Rik van Riel
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 5 lines

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?

Quoted 7 lines

I can't answer this questions.

Quoted 1 line

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

Quoted 4 lines

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).

#5 lore raw
Quoted 46 lines · 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]>:

Quoted 45 lines
Quoted 32 lines · On Tue, 5 Apr 2016 14:18:01 +0800 Yang Zhang <[email protected]> wrote:
Quoted 22 lines · Rik van Riel
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 1 line

BTW, in what case the migration of timers during VCPU scheduling will fail?

Quoted 8 lines
Quoted 3 lines

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.

Quoted 9 lines

Regards,
Wanpeng Li

Quoted 4 lines

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).

#6 lore raw

2016-04-22 21:12 GMT+08:00 Luiz Capitulino <[email protected]>:

Quoted 53 lines
Quoted 50 lines · 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]>:

Quoted 48 lines
Quoted 32 lines · On Tue, 5 Apr 2016 14:18:01 +0800 Yang Zhang <[email protected]> wrote:
Quoted 22 lines · Rik van Riel
Quoted 12 lines · Luiz Capitulino

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.

Quoted 8 lines

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.

Quoted 7 lines

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.

Quoted 5 lines

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.

Quoted 1 line

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.

Quoted 8 lines
Quoted 4 lines

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

#7 lore raw
In reply to Yang Zhang

2016-04-05 14:18+0800, Yang Zhang:

Quoted 9 lines · Rik van Riel
Quoted 7 lines

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.

Quoted 5 lines

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.

#8 lore raw
Quoted 16 lines · Radim Krčmář

2016-04-05 14:18+0800, Yang Zhang:

Quoted 9 lines · Rik van Riel
Quoted 7 lines

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.

Quoted 5 lines

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?

Quoted 4 lines

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.

Quoted 9 lines

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.

Quoted 3 lines

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.:)

Signature · 2 lines
best regards
yang
maintainer Applied
#9 lore raw
In reply to Luiz Capitulino
Quoted 78 lines · Luiz Capitulino

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.

---
 arch/x86/kvm/lapic.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kvm/lapic.c +3 -3
index 443d2a5..1a2da0e 100644
@@ -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;
 
 	/*

arch/x86/kvm/lapic.c void __kvm_migrate_apic_timer(struct kvm_vcpu *vcpu) · line 2009

Quoted 8 lines
arch/x86/kvm/lapic.c +1 -1
@@ -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