Skip to contentSkip to conversation

KVM:VMX: Add support for Pause-Loop Exiting

PATCH RESEND Applied Zhai, Edwin 4 participants 22 messages Sep 23, 2009 – Oct 12, 2009

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

Zhai, Edwin

author
#0 lore raw

PATCH RESEND 1/1 KVM:VMX: Add support for Pause-Loop Exiting

Avi,

This is the patch to enable PLE, which depends on the a small change of Linux scheduler (see http://lkml.org/lkml/2009/5/20/447).

According to our discussion last time, one missing part is that if PLE exit, pick up an unscheduled vcpu at random and schedule it. But further investigation found that:
1. KVM is hard to know the schedule state for each vcpu.
2. Linux scheduler has no existed API can be used to pull a specific task to this cpu, so we need more changes to the common scheduler. So I prefer current simple way: just give up current cpu time.

If no objection, I'll try to push common scheduler change first to linux.

Thanks,
edwin

KVM:VMX: Add support for Pause-Loop Exiting

New NHM processors will support Pause-Loop Exiting by adding 2 VM-execution control fields:
PLE_Gap - upper bound on the amount of time between two successive
executions of PAUSE in a loop.
PLE_Window - upper bound on the amount of time a guest is allowed to execute in
a PAUSE loop

If the time, between this execution of PAUSE and previous one, exceeds the PLE_Gap, processor consider this PAUSE belongs to a new loop. Otherwise, processor determins the the total execution time of this loop(since 1st PAUSE in this loop), and triggers a VM exit if total time exceeds the PLE_Window.
* Refer SDM volume 3b section 21.6.13 & 22.1.3.

Pause-Loop Exiting can be used to detect Lock-Holder Preemption, where one VP is sched-out after hold a spinlock, then other VPs for same lock are sched-in to waste the CPU time.

Our tests indicate that most spinlocks are held for less than 212 cycles. Performance tests show that with 2X LP over-commitment we can get +2% perf improvement for kernel build(Even more perf gain with more LPs).

Signed-off-by Zhai Edwin <[email protected]>
arch/x86/include/asm/vmx.h +4 -0
@@ -56,6 +56,7 @@
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING	0x00000400
 
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001

@@ -144,6 +145,8 @@ enum vmcs_field {
 	VM_ENTRY_INSTRUCTION_LEN        = 0x0000401a,
 	TPR_THRESHOLD                   = 0x0000401c,
 	SECONDARY_VM_EXEC_CONTROL       = 0x0000401e,
+	PLE_GAP                         = 0x00004020,
+	PLE_WINDOW                      = 0x00004022,
 	VM_INSTRUCTION_ERROR            = 0x00004400,
 	VM_EXIT_REASON                  = 0x00004402,
 	VM_EXIT_INTR_INFO               = 0x00004404,

@@ -248,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
arch/x86/kvm/vmx.c +50 -1

96 lines not on this page · raw message

#1 lore raw
Quoted 13 lines · Edwin

Avi,

This is the patch to enable PLE, which depends on the a small change of Linux scheduler (see http://lkml.org/lkml/2009/5/20/447).

According to our discussion last time, one missing part is that if PLE exit, pick up an unscheduled vcpu at random and schedule it. But further investigation found that:
1. KVM is hard to know the schedule state for each vcpu.
2. Linux scheduler has no existed API can be used to pull a specific task to this cpu, so we need more changes to the common scheduler. So I prefer current simple way: just give up current cpu time.

Quoted 2 lines

If no objection, I'll try to push common scheduler change first to linux.

We haven't sorted out what is the correct thing to do here. I think we should go for a directed yield, but until we have it, you can use hrtimers to sleep for 100 microseconds and hope the holding vcpu will get scheduled. Even if it doesn't, we're only wasting a few percent cpu time instead of spinning.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#2 lore raw

Avi,

hrtimer is used for sleep in attached patch, which have similar perf gain with previous one. Maybe we can check in this patch first, and turn to direct yield in future, as you suggested.

Thanks,
edwin

Quoted 22 lines · Avi Kivity
Quoted 13 lines · Edwin

Avi,

This is the patch to enable PLE, which depends on the a small change of Linux scheduler (see http://lkml.org/lkml/2009/5/20/447).

According to our discussion last time, one missing part is that if PLE exit, pick up an unscheduled vcpu at random and schedule it. But further investigation found that:
1. KVM is hard to know the schedule state for each vcpu.
2. Linux scheduler has no existed API can be used to pull a specific task to this cpu, so we need more changes to the common scheduler. So I prefer current simple way: just give up current cpu time.

Quoted 3 lines

If no objection, I'll try to push common scheduler change first to linux.

We haven't sorted out what is the correct thing to do here. I think we should go for a directed yield, but until we have it, you can use hrtimers to sleep for 100 microseconds and hope the holding vcpu will get scheduled. Even if it doesn't, we're only wasting a few percent cpu time instead of spinning.

Signature · 2 lines
best rgds,
edwin

KVM:VMX: Add support for Pause-Loop Exiting

New NHM processors will support Pause-Loop Exiting by adding 2 VM-execution control fields:
PLE_Gap - upper bound on the amount of time between two successive
executions of PAUSE in a loop.
PLE_Window - upper bound on the amount of time a guest is allowed to execute in
a PAUSE loop

If the time, between this execution of PAUSE and previous one, exceeds the PLE_Gap, processor consider this PAUSE belongs to a new loop. Otherwise, processor determins the the total execution time of this loop(since 1st PAUSE in this loop), and triggers a VM exit if total time exceeds the PLE_Window.
* Refer SDM volume 3b section 21.6.13 & 22.1.3.

Pause-Loop Exiting can be used to detect Lock-Holder Preemption, where one VP is sched-out after hold a spinlock, then other VPs for same lock are sched-in to waste the CPU time.

Our tests indicate that most spinlocks are held for less than 212 cycles. Performance tests show that with 2X LP over-commitment we can get +2% perf improvement for kernel build(Even more perf gain with more LPs).

Signed-off-by Zhai Edwin <[email protected]>
arch/x86/include/asm/vmx.h +4 -0
index 272514c..2b49454 100644
@@ -56,6 +56,7 @@
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING	0x00000400
 
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001

@@ -144,6 +145,8 @@ enum vmcs_field {
 	VM_ENTRY_INSTRUCTION_LEN        = 0x0000401a,
 	TPR_THRESHOLD                   = 0x0000401c,
 	SECONDARY_VM_EXEC_CONTROL       = 0x0000401e,
+	PLE_GAP                         = 0x00004020,
+	PLE_WINDOW                      = 0x00004022,
 	VM_INSTRUCTION_ERROR            = 0x00004400,
 	VM_EXIT_REASON                  = 0x00004402,
 	VM_EXIT_INTR_INFO               = 0x00004404,

@@ -248,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
arch/x86/kvm/vmx.c +56 -1

102 lines not on this page · raw message

#14 lore raw
In reply to Avi Kivity
Quoted 6 lines · Avi Kivity

We haven't sorted out what is the correct thing to do here. I think we should go for a directed yield, but until we have it, you can use hrtimers to sleep for 100 microseconds and hope the holding vcpu will get scheduled. Even if it doesn't, we're only wasting a few percent cpu time instead of spinning.

How do you plan to find out to which vcpu thread the current thread should yield?

Joerg

#3 lore raw
In reply to Zhai, Edwin
Quoted 13 lines · Edwin

Avi,

hrtimer is used for sleep in attached patch, which have similar perf gain with previous one. Maybe we can check in this patch first, and turn to direct yield in future, as you suggested.

arch/x86/kvm/vmx.c +8 -0
+/*
+ * These 2 parameters are used to config the controls for Pause-Loop Exiting:
+ * ple_gap:    upper bound on the amount of time between two successive
+ *             executions of PAUSE in a loop. Also indicate if ple enabled.
+ *             According to test, this time is usually small than 41 cycles.
+ * ple_window: upper bound on the amount of time a guest is allowed to execute
+ *             in a PAUSE loop. Tests indicate that most spinlocks are held for
+ *             less than 2^12 cycles

arch/x86/kvm/vmx.c module_param_named(unrestricted_guest, · line 81

Quoted 10 lines
arch/x86/kvm/vmx.c +10 -0
+ * Time is measured based on a counter that runs at the same rate as the TSC,
+ * refer SDM volume 3b section 21.6.13&  22.1.3.
+ */
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

I'm not even sure they should be parameters.

arch/x86/kvm/vmx.c out: · line 3411

Quoted 13 lines

/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */

arch/x86/kvm/vmx.c +8 -0
+static int handle_pause(struct kvm_vcpu *vcpu,
+				struct kvm_run *kvm_run)
+{
+	ktime_t expires;
+	skip_emulated_instruction(vcpu);
+
+	/* Sleep for 1 msec, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 1000000UL);

I think this should be much lower, 50-100us. Maybe this should be a parameter. With 1ms we losing significant cpu time if the congestion clears.

arch/x86/kvm/vmx.c out: · line 3414

Quoted 4 lines
arch/x86/kvm/vmx.c +3 -0
+	set_current_state(TASK_INTERRUPTIBLE);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour), and move the logic to kvm_main.c. It will be reused by the AMD implementation, possibly my software spinlock detector, paravirtualized spinlocks, and hopefully other architectures.

Quoted 5 lines
arch/x86/kvm/vmx.c +4 -0
+	return 1;
+}
+
+/*
Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#15 lore raw
In reply to Joerg Roedel
Quoted 11 lines · Joerg Roedel
Quoted 6 lines · Avi Kivity

We haven't sorted out what is the correct thing to do here. I think we should go for a directed yield, but until we have it, you can use hrtimers to sleep for 100 microseconds and hope the holding vcpu will get scheduled. Even if it doesn't, we're only wasting a few percent cpu time instead of spinning.

How do you plan to find out to which vcpu thread the current thread should yield?

We can't find exactly which vcpu, but we can:

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#16 lore raw
Quoted 15 lines · Avi Kivity
Quoted 11 lines · Joerg Roedel
Quoted 6 lines · Avi Kivity

We haven't sorted out what is the correct thing to do here. I think we should go for a directed yield, but until we have it, you can use hrtimers to sleep for 100 microseconds and hope the holding vcpu will get scheduled. Even if it doesn't, we're only wasting a few percent cpu time instead of spinning.

How do you plan to find out to which vcpu thread the current thread should yield?

We can't find exactly which vcpu, but we can:

Quoted 7 lines

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Ok, that makes sense. But before trying that we should probably try to call just yield() instead of schedule()? I remember someone from our team here at AMD did this for Xen a while ago and already had pretty good results with that. Xen has a completly other scheduler but maybe its worth trying?

Joerg

#17 lore raw
Quoted 11 lines · Joerg Roedel
Quoted 11 lines

We can't find exactly which vcpu, but we can:

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Quoted 6 lines

Ok, that makes sense. But before trying that we should probably try to call just yield() instead of schedule()? I remember someone from our team here at AMD did this for Xen a while ago and already had pretty good results with that. Xen has a completly other scheduler but maybe its worth trying?

yield() is a no-op in CFS.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#18 lore raw
Quoted 12 lines · Avi Kivity
Quoted 11 lines · Joerg Roedel
Quoted 11 lines

We can't find exactly which vcpu, but we can:

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Quoted 8 lines
Quoted 6 lines

Ok, that makes sense. But before trying that we should probably try to call just yield() instead of schedule()? I remember someone from our team here at AMD did this for Xen a while ago and already had pretty good results with that. Xen has a completly other scheduler but maybe its worth trying?

yield() is a no-op in CFS.

Hmm, true. At least when kernel.sched_compat_yield == 0, which it is on my distro.
If the scheduler would give us something like a real_yield() function which asumes kernel.sched_compat_yield = 1 might help. At least its better than sleeping for some random amount of time.

Joerg

#19 lore raw
Quoted 23 lines · Joerg Roedel
Quoted 21 lines · Avi Kivity
Quoted 19 lines · Joerg Roedel
Quoted 12 lines

We can't find exactly which vcpu, but we can:

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Ok, that makes sense. But before trying that we should probably try to call just yield() instead of schedule()? I remember someone from our team here at AMD did this for Xen a while ago and already had pretty good results with that. Xen has a completly other scheduler but maybe its worth trying?

Quoted 9 lines
Quoted 2 lines

yield() is a no-op in CFS.

Hmm, true. At least when kernel.sched_compat_yield == 0, which it is on my distro.
If the scheduler would give us something like a real_yield() function which asumes kernel.sched_compat_yield = 1 might help. At least its better than sleeping for some random amount of time.

Depends. If it's a global yield(), yes. If it's a local yield() that doesn't rebalance the runqueues we might be left with the spinning task re-running.

Also, if yield means "give up the reminder of our timeslice", then we potentially end up sleeping a much longer random amount of time. If we yield to another vcpu in the same guest we might not care, but if we yield to some other guest we're seriously penalizing ourselves.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#20 lore raw
Quoted 34 lines · Avi Kivity
Quoted 23 lines · Joerg Roedel
Quoted 21 lines · Avi Kivity
Quoted 19 lines · Joerg Roedel
Quoted 12 lines

We can't find exactly which vcpu, but we can:

- rule out threads that are not vcpus for this guest
- rule out threads that are already running

A major problem with sleep() is that it effectively reduces the vm priority relative to guests that don't have spinlock contention. By selecting a random nonrunnable vcpu belonging to this guest, we at least preserve the guest's timeslice.

Ok, that makes sense. But before trying that we should probably try to call just yield() instead of schedule()? I remember someone from our team here at AMD did this for Xen a while ago and already had pretty good results with that. Xen has a completly other scheduler but maybe its worth trying?

Quoted 9 lines
Quoted 2 lines

yield() is a no-op in CFS.

Hmm, true. At least when kernel.sched_compat_yield == 0, which it is on my distro.
If the scheduler would give us something like a real_yield() function which asumes kernel.sched_compat_yield = 1 might help. At least its better than sleeping for some random amount of time.

Quoted 3 lines

Depends. If it's a global yield(), yes. If it's a local yield() that doesn't rebalance the runqueues we might be left with the spinning task re-running.

Only one runable task on each cpu is unlikely in a situation of high vcpu overcommit (where pause filtering matters).

Quoted 4 lines

Also, if yield means "give up the reminder of our timeslice", then we potentially end up sleeping a much longer random amount of time. If we yield to another vcpu in the same guest we might not care, but if we yield to some other guest we're seriously penalizing ourselves.

I agree that a directed yield with possible rebalance would be good to have, but this is very intrusive to the scheduler code and I think we should at least try if this simpler approach already gives us good results.

Joerg

author
#4 lore raw
In reply to Avi Kivity
Quoted 10 lines · Avi Kivity
(quoted diff) +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 2 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

Quoted 2 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

Quoted 18 lines
Quoted 7 lines

/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */

Review

Quoted 8 lines
(quoted diff) +8 -0
+static int handle_pause(struct kvm_vcpu *vcpu,
+				struct kvm_run *kvm_run)
+{
+	ktime_t expires;
+	skip_emulated_instruction(vcpu);
+
+	/* Sleep for 1 msec, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 1000000UL);

I think this should be much lower, 50-100us. Maybe this should be a parameter. With 1ms we losing significant cpu time if the congestion clears.

I have made it a parameter with default value of 100 us.

Quoted 8 lines

Review

Quoted 6 lines
(quoted diff) +3 -0
+	set_current_state(TASK_INTERRUPTIBLE);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Quoted 2 lines

and move the logic to kvm_main.c. It will be reused by the AMD implementation, possibly my software spinlock detector, paravirtualized spinlocks, and hopefully other architectures.

Done.

Quoted 9 lines

Review

Quoted 7 lines
(quoted diff) +4 -0
+	return 1;
+}
+
+/*

KVM:VMX: Add support for Pause-Loop Exiting

New NHM processors will support Pause-Loop Exiting by adding 2 VM-execution control fields:
PLE_Gap - upper bound on the amount of time between two successive
executions of PAUSE in a loop.
PLE_Window - upper bound on the amount of time a guest is allowed to execute in
a PAUSE loop

If the time, between this execution of PAUSE and previous one, exceeds the PLE_Gap, processor consider this PAUSE belongs to a new loop. Otherwise, processor determins the the total execution time of this loop(since 1st PAUSE in this loop), and triggers a VM exit if total time exceeds the PLE_Window.
* Refer SDM volume 3b section 21.6.13 & 22.1.3.

Pause-Loop Exiting can be used to detect Lock-Holder Preemption, where one VP is sched-out after hold a spinlock, then other VPs for same lock are sched-in to waste the CPU time.

Our tests indicate that most spinlocks are held for less than 212 cycles. Performance tests show that with 2X LP over-commitment we can get +2% perf improvement for kernel build(Even more perf gain with more LPs).

Signed-off-by Zhai Edwin <[email protected]>
arch/x86/include/asm/vmx.h +4 -0
index 272514c..2b49454 100644
@@ -56,6 +56,7 @@
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING	0x00000400
 
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001

@@ -144,6 +145,8 @@ enum vmcs_field {
 	VM_ENTRY_INSTRUCTION_LEN        = 0x0000401a,
 	TPR_THRESHOLD                   = 0x0000401c,
 	SECONDARY_VM_EXEC_CONTROL       = 0x0000401e,
+	PLE_GAP                         = 0x00004020,
+	PLE_WINDOW                      = 0x00004022,
 	VM_INSTRUCTION_ERROR            = 0x00004400,
 	VM_EXIT_REASON                  = 0x00004402,
 	VM_EXIT_INTR_INFO               = 0x00004404,

@@ -248,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
arch/x86/kvm/vmx.c +57 -1

103 lines not on this page · raw message

include/linux/kvm_host.h +1 -0
index 0bf9ee9..3723d62 100644
@@ -287,6 +287,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
+void kvm_vcpu_sleep(struct kvm_vcpu *vcpu, unsigned int sleep_time);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
virt/kvm/kvm_main.c +11 -0
index e27b7a9..ff006ce 100644
@@ -1095,6 +1095,17 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+void kvm_vcpu_sleep(struct kvm_vcpu *vcpu, unsigned int sleep_time)
+{
+	/* Sleep for required time(us), and hope lock-holder got scheduled */
+	ktime_t expires;
+
+	expires = ktime_add_ns(ktime_get(), 1000UL * sleep_time);
+	set_current_state(TASK_INTERRUPTIBLE);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_sleep);
+
 static int kvm_vcpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct kvm_vcpu *vcpu = vma->vm_file->private_data;
author
#5 lore raw

Avi,
Any comments for this new patch?
Thanks,

Quoted 81 lines · Edwin
Quoted 10 lines · Avi Kivity
arch/x86/kvm/vmx.c +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 3 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

Quoted 3 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

Quoted 20 lines
Quoted 9 lines

/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */

arch/x86/kvm/vmx.c out:

Quoted 8 lines
arch/x86/kvm/vmx.c +8 -0
+static int handle_pause(struct kvm_vcpu *vcpu,
+				struct kvm_run *kvm_run)
+{
+	ktime_t expires;
+	skip_emulated_instruction(vcpu);
+
+	/* Sleep for 1 msec, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 1000000UL);

I think this should be much lower, 50-100us. Maybe this should be a parameter. With 1ms we losing significant cpu time if the congestion clears.

I have made it a parameter with default value of 100 us.

Quoted 10 lines

virt/kvm/kvm_main.c void kvm_resched(struct kvm_vcpu *vcpu) · line 1108

Quoted 8 lines
virt/kvm/kvm_main.c +3 -0
+	set_current_state(TASK_INTERRUPTIBLE);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Quoted 3 lines

and move the logic to kvm_main.c. It will be reused by the AMD implementation, possibly my software spinlock detector, paravirtualized spinlocks, and hopefully other architectures.

Done.

Quoted 11 lines
Quoted 9 lines
arch/x86/kvm/vmx.c +4 -0
+	return 1;
+}
+
+/*
#6 lore raw
In reply to Zhai, Edwin
Quoted 10 lines · Edwin
Quoted 8 lines · Avi Kivity
arch/x86/kvm/vmx.c +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 3 lines
Quoted 1 line

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

__read_mostly doesn't just mean it's read mostly. It also means it's read often. Otherwise it's just wasting space in hot cachelines.

Quoted 4 lines
Quoted 2 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

No, global parameters don't work for tuning workloads and guests since they cannot be modified on a per-guest basis. enable_ept is only useful for debugging and testing.

Quoted 8 lines
Quoted 6 lines

virt/kvm/kvm_main.c void kvm_resched(struct kvm_vcpu *vcpu) · line 1108

Quoted 4 lines
virt/kvm/kvm_main.c +3 -0
+    set_current_state(TASK_INTERRUPTIBLE);
+    schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Right. I thought of the software spinlock detector, but that's another problem.

I think you can drop the sleep_time parameter, it can be part of the function. Also kvm_vcpu_sleep() is confusing, we also sleep on halt. Please call it kvm_vcpu_on_spin() or something (since that's what the guest is doing).

Signature · 1 line
error compiling committee.c: too many arguments to function
#21 lore raw
In reply to Joerg Roedel
Quoted 10 lines · Joerg Roedel
Quoted 6 lines

Depends. If it's a global yield(), yes. If it's a local yield() that doesn't rebalance the runqueues we might be left with the spinning task re-running.

Only one runable task on each cpu is unlikely in a situation of high vcpu overcommit (where pause filtering matters).

I think even 2:1 overcommit can degrade performance terribly.

Quoted 10 lines
Quoted 5 lines

Also, if yield means "give up the reminder of our timeslice", then we potentially end up sleeping a much longer random amount of time. If we yield to another vcpu in the same guest we might not care, but if we yield to some other guest we're seriously penalizing ourselves.

I agree that a directed yield with possible rebalance would be good to have, but this is very intrusive to the scheduler code and I think we should at least try if this simpler approach already gives us good results.

No objection to trying. I'd like to see hrtimer sleep as a baseline since it doesn't require any core changes, and we can play with it as we add more core infrastructure:

- not sleeping if all vcpus are running
- true yield() instead of sleep
- directed yield
- cross cpu directed yield

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#7 lore raw
In reply to Avi Kivity

Avi,
I modify it according your comments. The only thing I want to keep is the module param ple_gap/window. Although they are not per-guest, they can be used to find the right value, and disable PLE for debug purpose.

Thanks,

Quoted 33 lines · Avi Kivity
Quoted 10 lines · Edwin
Quoted 8 lines · Avi Kivity
(quoted diff) +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 4 lines
Quoted 2 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

__read_mostly doesn't just mean it's read mostly. It also means it's read often. Otherwise it's just wasting space in hot cachelines.

Quoted 4 lines
Quoted 2 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

No, global parameters don't work for tuning workloads and guests since they cannot be modified on a per-guest basis. enable_ept is only useful for debugging and testing.

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
(quoted diff) +3 -0
+    set_current_state(TASK_INTERRUPTIBLE);
+    schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+
Quoted 10 lines
Quoted 4 lines
Quoted 2 lines

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Right. I thought of the software spinlock detector, but that's another problem.

I think you can drop the sleep_time parameter, it can be part of the function. Also kvm_vcpu_sleep() is confusing, we also sleep on halt. Please call it kvm_vcpu_on_spin() or something (since that's what the guest is doing).

KVM:VMX: Add support for Pause-Loop Exiting

New NHM processors will support Pause-Loop Exiting by adding 2 VM-execution control fields:
PLE_Gap - upper bound on the amount of time between two successive
executions of PAUSE in a loop.
PLE_Window - upper bound on the amount of time a guest is allowed to execute in
a PAUSE loop

If the time, between this execution of PAUSE and previous one, exceeds the PLE_Gap, processor consider this PAUSE belongs to a new loop. Otherwise, processor determins the the total execution time of this loop(since 1st PAUSE in this loop), and triggers a VM exit if total time exceeds the PLE_Window.
* Refer SDM volume 3b section 21.6.13 & 22.1.3.

Pause-Loop Exiting can be used to detect Lock-Holder Preemption, where one VP is sched-out after hold a spinlock, then other VPs for same lock are sched-in to waste the CPU time.

Our tests indicate that most spinlocks are held for less than 212 cycles. Performance tests show that with 2X LP over-commitment we can get +2% perf improvement for kernel build(Even more perf gain with more LPs).

Signed-off-by Zhai Edwin <[email protected]>
arch/x86/include/asm/vmx.h +4 -0
index 272514c..2b49454 100644
@@ -56,6 +56,7 @@
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING	0x00000400
 
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001

@@ -144,6 +145,8 @@ enum vmcs_field {
 	VM_ENTRY_INSTRUCTION_LEN        = 0x0000401a,
 	TPR_THRESHOLD                   = 0x0000401c,
 	SECONDARY_VM_EXEC_CONTROL       = 0x0000401e,
+	PLE_GAP                         = 0x00004020,
+	PLE_WINDOW                      = 0x00004022,
 	VM_INSTRUCTION_ERROR            = 0x00004400,
 	VM_EXIT_REASON                  = 0x00004402,
 	VM_EXIT_INTR_INFO               = 0x00004404,

@@ -248,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
arch/x86/kvm/vmx.c +51 -1

97 lines not on this page · raw message

include/linux/kvm_host.h +1 -0
index 0bf9ee9..0c86b1d 100644
@@ -287,6 +287,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
virt/kvm/kvm_main.c +11 -0
index e27b7a9..f36519b 100644
@@ -1095,6 +1095,17 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu)
+{
+	/* Sleep for 100 us, and hope lock-holder got scheduled */
+	ktime_t expires;
+
+	expires = ktime_add_ns(ktime_get(), 100000UL);
+	set_current_state(TASK_INTERRUPTIBLE);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
+
 static int kvm_vcpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct kvm_vcpu *vcpu = vma->vm_file->private_data;
#8 lore raw
Quoted 2 lines · Edwin

Avi,
I modify it according your comments. The only thing I want to keep is the module param ple_gap/window. Although they are not per-guest, they can be used to find the right value, and disable PLE for debug purpose.

Fair enough, ACK.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#9 lore raw
In reply to Zhai, Edwin
Quoted 5 lines · Edwin

Avi,
I modify it according your comments. The only thing I want to keep is the module param ple_gap/window. Although they are not per-guest, they can be used to find the right value, and disable PLE for debug purpose.

Quoted 58 lines

Thanks,

Quoted 54 lines · Avi Kivity
Quoted 10 lines · Edwin
Quoted 8 lines · Avi Kivity
arch/x86/kvm/vmx.c +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 7 lines
Quoted 4 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

__read_mostly doesn't just mean it's read mostly. It also means it's read often. Otherwise it's just wasting space in hot cachelines.

Quoted 6 lines
Quoted 2 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

No, global parameters don't work for tuning workloads and guests since they cannot be modified on a per-guest basis. enable_ept is only useful for debugging and testing.

Quoted 9 lines
Quoted 6 lines

virt/kvm/kvm_main.c void kvm_resched(struct kvm_vcpu *vcpu) · line 1108

Quoted 4 lines
virt/kvm/kvm_main.c +3 -0
+    set_current_state(TASK_INTERRUPTIBLE);
+    schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Right. I thought of the software spinlock detector, but that's another problem.

I think you can drop the sleep_time parameter, it can be part of the function. Also kvm_vcpu_sleep() is confusing, we also sleep on halt. Please call it kvm_vcpu_on_spin() or something (since that's what the guest is doing).

kvm_vcpu_on_spin() should add the vcpu to vcpu->wq (so a new pending interrupt wakes it up immediately).

Do you (and/or Mark) have any numbers for non-vcpu overcommited guests?

#10 lore raw
Quoted 60 lines · Marcelo Tosatti
Quoted 59 lines · Edwin

Avi,
I modify it according your comments. The only thing I want to keep is the module param ple_gap/window. Although they are not per-guest, they can be used to find the right value, and disable PLE for debug purpose.

Thanks,

Quoted 50 lines · Avi Kivity
Quoted 10 lines · Edwin
Quoted 8 lines · Avi Kivity
(quoted diff) +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 7 lines
Quoted 4 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

__read_mostly doesn't just mean it's read mostly. It also means it's read often. Otherwise it's just wasting space in hot cachelines.

Quoted 6 lines
Quoted 2 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

No, global parameters don't work for tuning workloads and guests since they cannot be modified on a per-guest basis. enable_ept is only useful for debugging and testing.

Quoted 9 lines
Quoted 6 lines

Review

Quoted 4 lines
(quoted diff) +3 -0
+    set_current_state(TASK_INTERRUPTIBLE);
+    schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Right. I thought of the software spinlock detector, but that's another problem.

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines

I think you can drop the sleep_time parameter, it can be part of the function. Also kvm_vcpu_sleep() is confusing, we also sleep on halt. Please call it kvm_vcpu_on_spin() or something (since that's what the guest is doing).

kvm_vcpu_on_spin() should add the vcpu to vcpu->wq (so a new pending interrupt wakes it up immediately).

Updated version (also please send it separately from the vmx.c patch):

include/linux/kvm_host.h +1 -0
index 894a56e..43125dc 100644
@@ -231,6 +231,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
virt/kvm/kvm_main.c +15 -0
index 4d0dd39..e788d70 100644
@@ -1479,6 +1479,21 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu)
+{
+	ktime_t expires;
+	DEFINE_WAIT(wait);
+
+	prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+
+	/* Sleep for 100 us, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 100000UL);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+
+	finish_wait(&vcpu->wq, &wait);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
+
 static int kvm_vcpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct kvm_vcpu *vcpu = vma->vm_file->private_data;
author
#11 lore raw

Tosatti,
See attached patch.

Avi,
Could you pls. do the check in if no any other comments. Thanks,

Quoted 106 lines · Marcelo Tosatti
Quoted 54 lines · Marcelo Tosatti
Quoted 52 lines · Edwin

Avi,
I modify it according your comments. The only thing I want to keep is the module param ple_gap/window. Although they are not per-guest, they can be used to find the right value, and disable PLE for debug purpose.

Thanks,

Quoted 44 lines · Avi Kivity
Quoted 11 lines · Edwin
Quoted 8 lines · Avi Kivity
(quoted diff) +7 -0
+#define KVM_VMX_DEFAULT_PLE_GAP    41
+#define KVM_VMX_DEFAULT_PLE_WINDOW 4096
+static int __read_mostly ple_gap = KVM_VMX_DEFAULT_PLE_GAP;
+module_param(ple_gap, int, S_IRUGO);
+
+static int __read_mostly ple_window = KVM_VMX_DEFAULT_PLE_WINDOW;
+module_param(ple_window, int, S_IRUGO);
Quoted 6 lines
Quoted 3 lines

Shouldn't be __read_mostly since they're read very rarely (__read_mostly should be for variables that are very often read, and rarely written).

In general, they are read only except that experienced user may try different parameter for perf tuning.

__read_mostly doesn't just mean it's read mostly. It also means it's read often. Otherwise it's just wasting space in hot cachelines.

Quoted 6 lines
Quoted 3 lines

I'm not even sure they should be parameters.

For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.

No, global parameters don't work for tuning workloads and guests since they cannot be modified on a per-guest basis. enable_ept is only useful for debugging and testing.

Quoted 10 lines
Quoted 7 lines

Review

Quoted 5 lines
(quoted diff) +3 -0
+    set_current_state(TASK_INTERRUPTIBLE);
+    schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+

Please add a tracepoint for this (since it can cause significant change in behaviour),

Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.

Quoted 7 lines
Quoted 4 lines
Quoted 4 lines

Right. I thought of the software spinlock detector, but that's another problem.

I think you can drop the sleep_time parameter, it can be part of the function. Also kvm_vcpu_sleep() is confusing, we also sleep on halt. Please call it kvm_vcpu_on_spin() or something (since that's what the guest is doing).

kvm_vcpu_on_spin() should add the vcpu to vcpu->wq (so a new pending interrupt wakes it up immediately).

Updated version (also please send it separately from the vmx.c patch):

include/linux/kvm_host.h +1 -0
index 894a56e..43125dc 100644
@@ -231,6 +231,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
virt/kvm/kvm_main.c +15 -0
index 4d0dd39..e788d70 100644
@@ -1479,6 +1479,21 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu)
+{
+	ktime_t expires;
+	DEFINE_WAIT(wait);
+
+	prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+
+	/* Sleep for 100 us, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 100000UL);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+
+	finish_wait(&vcpu->wq, &wait);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
+
 static int kvm_vcpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct kvm_vcpu *vcpu = vma->vm_file->private_data;
Signature · 2 lines
best rgds,
edwin

KVM:VMX: Add support for Pause-Loop Exiting

New NHM processors will support Pause-Loop Exiting by adding 2 VM-execution control fields:
PLE_Gap - upper bound on the amount of time between two successive
executions of PAUSE in a loop.
PLE_Window - upper bound on the amount of time a guest is allowed to execute in
a PAUSE loop

If the time, between this execution of PAUSE and previous one, exceeds the PLE_Gap, processor consider this PAUSE belongs to a new loop. Otherwise, processor determins the the total execution time of this loop(since 1st PAUSE in this loop), and triggers a VM exit if total time exceeds the PLE_Window.
* Refer SDM volume 3b section 21.6.13 & 22.1.3.

Pause-Loop Exiting can be used to detect Lock-Holder Preemption, where one VP is sched-out after hold a spinlock, then other VPs for same lock are sched-in to waste the CPU time.

Our tests indicate that most spinlocks are held for less than 212 cycles. Performance tests show that with 2X LP over-commitment we can get +2% perf improvement for kernel build(Even more perf gain with more LPs).

Signed-off-by Zhai Edwin <[email protected]>
arch/x86/include/asm/vmx.h +4 -0
index 272514c..2b49454 100644
@@ -56,6 +56,7 @@
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING	0x00000400
 
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001

@@ -144,6 +145,8 @@ enum vmcs_field {
 	VM_ENTRY_INSTRUCTION_LEN        = 0x0000401a,
 	TPR_THRESHOLD                   = 0x0000401c,
 	SECONDARY_VM_EXEC_CONTROL       = 0x0000401e,
+	PLE_GAP                         = 0x00004020,
+	PLE_WINDOW                      = 0x00004022,
 	VM_INSTRUCTION_ERROR            = 0x00004400,
 	VM_EXIT_REASON                  = 0x00004402,
 	VM_EXIT_INTR_INFO               = 0x00004404,

@@ -248,6 +251,7 @@ enum vmcs_field {
 #define EXIT_REASON_MSR_READ            31
 #define EXIT_REASON_MSR_WRITE           32
 #define EXIT_REASON_MWAIT_INSTRUCTION   36
+#define EXIT_REASON_PAUSE_INSTRUCTION   40
 #define EXIT_REASON_MCE_DURING_VMENTRY	 41
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
arch/x86/kvm/vmx.c +51 -1

97 lines not on this page · raw message

include/linux/kvm_host.h +1 -0
index b985a29..bd5a616 100644
@@ -286,6 +286,7 @@ int kvm_is_visible_gfn(struct kvm *kvm, gfn_t gfn);
 void mark_page_dirty(struct kvm *kvm, gfn_t gfn);
 
 void kvm_vcpu_block(struct kvm_vcpu *vcpu);
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu);
 void kvm_resched(struct kvm_vcpu *vcpu);
 void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
 void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
virt/kvm/kvm_main.c +15 -0
index c0a929f..c4289c0 100644
@@ -1108,6 +1108,21 @@ void kvm_resched(struct kvm_vcpu *vcpu)
 }
 EXPORT_SYMBOL_GPL(kvm_resched);
 
+void kvm_vcpu_on_spin(struct kvm_vcpu *vcpu)
+{
+	ktime_t expires;
+	DEFINE_WAIT(wait);
+
+	prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+
+	/* Sleep for 100 us, and hope lock-holder got scheduled */
+	expires = ktime_add_ns(ktime_get(), 100000UL);
+	schedule_hrtimeout(&expires, HRTIMER_MODE_ABS);
+
+	finish_wait(&vcpu->wq, &wait);
+}
+EXPORT_SYMBOL_GPL(kvm_vcpu_on_spin);
+
 static int kvm_vcpu_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 {
 	struct kvm_vcpu *vcpu = vma->vm_file->private_data;
#12 lore raw
Quoted 5 lines · Edwin

Tosatti,
See attached patch.

Avi,
Could you pls. do the check in if no any other comments.

Looks reasonable to me (Marcelo commits this week).

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