Skip to contentSkip to conversation

KVM: busy-spin detector

RFC PATCH In review Marcelo Tosatti 4 participants 7 messages Jun 11, 2010 – Jun 16, 2010

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

RFC PATCH 1/1 KVM: busy-spin detector

The following patch implements a simple busy-spin detector. It considers a vcpu as busy-spinning if there are two consecutive exits due to external interrupt on the same RIP, and sleeps for 100us in that case.

It is very likely that if the vcpu is making progress it will either exit for other reasons or change RIP.

The percentage numbers below represent improvement in kernel build time in comparison with mainline (RHEL 5.4 guest).

make -j16, 8 cpu host:
smp 16: 3%
smp 18: 10%
smp 32: 14%

smp 4, make -j4, pinned to 2 cpus:
4%

smp 8, make -j8, pinned to 2 cpus:
5%

arch/x86/include/asm/kvm_host.h +4 -0
Index: kvm/arch/x86/include/asm/kvm_host.h
===================================================================
@@ -301,6 +301,8 @@ struct kvm_vcpu_arch {
 		unsigned long mmu_seq;
 	} update_pte;
 
+	unsigned long last_rip;
+
 	struct fpu guest_fpu;
 
 	gva_t mmio_fault_cr2;

@@ -653,6 +655,8 @@ void kvm_disable_tdp(void);
 int complete_pio(struct kvm_vcpu *vcpu);
 bool kvm_check_iopl(struct kvm_vcpu *vcpu);
 
+void kvm_detect_spin(struct kvm_vcpu *vcpu);
+
 struct kvm_memory_slot *gfn_to_memslot_unaliased(struct kvm *kvm, gfn_t gfn);
 
 static inline struct kvm_mmu_page *page_header(hpa_t shadow_page)
arch/x86/kvm/svm.c +3 -1
Index: kvm/arch/x86/kvm/svm.c
===================================================================
@@ -1558,8 +1558,10 @@ static int nmi_interception(struct vcpu_
 
 static int intr_interception(struct vcpu_svm *svm)
 {
+	if (!svm_has(SVM_FEATURE_PAUSE_FILTER))
+		kvm_detect_spin(&svm->vcpu);
 	++svm->vcpu.stat.irq_exits;
-	return 1;
+	return 2;
 }
 
 static int nop_on_interception(struct vcpu_svm *svm)
arch/x86/kvm/vmx.c +3 -1
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
@@ -3116,7 +3116,9 @@ static int handle_exception(struct kvm_v
 static int handle_external_interrupt(struct kvm_vcpu *vcpu)
 {
 	++vcpu->stat.irq_exits;
-	return 1;
+	if (!cpu_has_vmx_ple())
+		kvm_detect_spin(vcpu);
+	return 2;
 }
 
 static int handle_triple_fault(struct kvm_vcpu *vcpu)
arch/x86/kvm/x86.c +13 -0
Index: kvm/arch/x86/kvm/x86.c
===================================================================
@@ -4523,6 +4523,17 @@ static void inject_pending_event(struct 
 	}
 }
 
+void kvm_detect_spin(struct kvm_vcpu *vcpu)
+{
+	unsigned long rip = kvm_rip_read(vcpu);
+
+	if (vcpu->arch.last_rip == rip)
+		kvm_vcpu_on_spin(vcpu);
+
+	vcpu->arch.last_rip = rip;
+}
+EXPORT_SYMBOL_GPL(kvm_detect_spin);
+
 static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 {
 	int r;

@@ -4654,6 +4665,8 @@ static int vcpu_enter_guest(struct kvm_v
 	kvm_lapic_sync_from_vapic(vcpu);
 
 	r = kvm_x86_ops->handle_exit(vcpu);
+	if (r == 1)
+		vcpu->arch.last_rip = ~(0UL);
 out:
 	return r;
 }
#1 lore raw
Quoted 11 lines · Marcelo Tosatti

The following patch implements a simple busy-spin detector. It considers a vcpu as busy-spinning if there are two consecutive exits due to external interrupt on the same RIP, and sleeps for 100us in that case.

It is very likely that if the vcpu is making progress it will either exit for other reasons or change RIP.

The percentage numbers below represent improvement in kernel build time in comparison with mainline (RHEL 5.4 guest).

Interesting approach, is there a reason to tie it in with pause loop exits? Can't we do something more generic or even para-virtish.

Signature · 2 lines
	Three Cheers,
	Balbir
#2 lore raw

PLE-like design may be more generic than para-virtish when it comes to Windows guest.

Is this busy-spin actually a Lock Holder Preemption problem?

Regards,

HUANG, Zhiteng

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Balbir Singh
Sent: Friday, June 11, 2010 10:07 PM
To: Marcelo Tosatti
Cc: kvm
Subject: Re: [PATCH RFC] KVM: busy-spin detector

Quoted 11 lines · Marcelo Tosatti

The following patch implements a simple busy-spin detector. It considers a vcpu as busy-spinning if there are two consecutive exits due to external interrupt on the same RIP, and sleeps for 100us in that case.

It is very likely that if the vcpu is making progress it will either exit for other reasons or change RIP.

The percentage numbers below represent improvement in kernel build time in comparison with mainline (RHEL 5.4 guest).

Interesting approach, is there a reason to tie it in with pause loop exits? Can't we do something more generic or even para-virtish.

Signature · 4 lines
	Three Cheers,
	Balbir
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected] More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw
Quoted 2 lines · Huang, Zhiteng

PLE-like design may be more generic than para-virtish when it comes to Windows guest.

Hmm.. sounds reasonable

Quoted 2 lines

Is this busy-spin actually a Lock Holder Preemption problem?

Yep, I was hinting towards solving that problem.

Signature · 2 lines
	Three Cheers,
	Balbir
author
#4 lore raw
In reply to Balbir Singh
Quoted 10 lines · Balbir Singh
Quoted 8 lines · Marcelo Tosatti

The following patch implements a simple busy-spin detector. It considers a vcpu as busy-spinning if there are two consecutive exits due to external interrupt on the same RIP, and sleeps for 100us in that case.

It is very likely that if the vcpu is making progress it will either exit for other reasons or change RIP.

Quoted 6 lines
Quoted 3 lines

The percentage numbers below represent improvement in kernel build time in comparison with mainline (RHEL 5.4 guest).

Interesting approach, is there a reason to tie it in with pause loop exits?

Hum, i don't see any. PLE exits provide the same detection, but more accurately.

Quoted 1 line

Can't we do something more generic or even para-virtish.

This is pretty generic already? Or what do you mean?

The advantage is it does not require paravirt modifications in the guest (at the expense of guessing what the guest is doing).

#5 lore raw
Quoted 6 lines · Marcelo Tosatti
Quoted 2 lines

Interesting approach, is there a reason to tie it in with pause loop exits?

Hum, i don't see any. PLE exits provide the same detection, but more accurately.

Quoted 7 lines
Quoted 1 line

Can't we do something more generic or even para-virtish.

This is pretty generic already? Or what do you mean?

The advantage is it does not require paravirt modifications in the guest (at the expense of guessing what the guest is doing).

Agreed, but one needs to depend on newer hardware to get this feature to work.

Signature · 2 lines
	Three Cheers,
	Balbir
#6 lore raw
In reply to Marcelo Tosatti
Quoted 15 lines · Marcelo Tosatti

The following patch implements a simple busy-spin detector. It considers a vcpu as busy-spinning if there are two consecutive exits due to external interrupt on the same RIP, and sleeps for 100us in that case.

It is very likely that if the vcpu is making progress it will either exit for other reasons or change RIP.

The percentage numbers below represent improvement in kernel build time in comparison with mainline (RHEL 5.4 guest).

make -j16, 8 cpu host:
smp 16: 3%
smp 18: 10%
smp 32: 14%

Quoted 7 lines

smp 4, make -j4, pinned to 2 cpus:
4%

smp 8, make -j8, pinned to 2 cpus:
5%

The big problem here is not in the patch itself, but in kvm_vcpu_on_spin(). An un-overcommitted guest will trigger this on long spinlocks and go to sleep, and if the lock is released while we're still sleeping, we lose performance.

arch/x86/kvm/x86.c static int vcpu_enter_guest(struct kvm_v · line 4672

Quoted 10 lines
arch/x86/kvm/x86.c +2 -0
@@ -4654,6 +4665,8 @@ static int vcpu_enter_guest(struct kvm_v
  	kvm_lapic_sync_from_vapic(vcpu);
 
  	r = kvm_x86_ops->handle_exit(vcpu);
+	if (r == 1)
+		vcpu->arch.last_rip = ~(0UL);
  out:
  	return r;
  }

!= 2, no?

It's a little ugly to overload the return code like that. Perhaps:

+ vcpu->arch.save_last_rip = vcpu->arch.last_rip

+    vcpu->arch.last_rip = ~0UL;
     r = kvm_x86_ops->handle_exit(vcpu);

And kvm_detect_spin() can restore last_rip.

Signature · 1 line
error compiling committee.c: too many arguments to function