Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 10/09/2009 12:03 PM, Zhai, Edwin wrote: 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). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Wed, Sep 30, 2009 at 01:22:49PM -0300, Marcelo Tosatti wrote:
On Wed, Sep 30, 2009 at 09:01:51AM +0800, Zhai, Edwin wrote:
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,
Avi Kivity wrote:
On 09/28/2009 11:33 AM, Zhai, Edwin wrote:
Avi Kivity wrote:
+#define KVM_VMX_DEFAULT_PLE_GAP41
+#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).
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.
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.
+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).
Updated version (also please send it separately from the vmx.c patch):
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 894a56e..43125dc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -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);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4d0dd39..e788d70 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -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(), 10UL);
+ 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;
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/30/2009 03:01 AM, Zhai, Edwin wrote: 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. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Wed, Sep 30, 2009 at 09:01:51AM +0800, Zhai, Edwin wrote: 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, Avi Kivity wrote: On 09/28/2009 11:33 AM, Zhai, Edwin wrote: Avi Kivity wrote: +#define KVM_VMX_DEFAULT_PLE_GAP41 +#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). 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. 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. +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? -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
Avi,
Any comments for this new patch?
Thanks,
Zhai, Edwin wrote:
Avi Kivity wrote:
+#define KVM_VMX_DEFAULT_PLE_GAP41
+#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).
In general, they are read only except that experienced user may try
different parameter for perf tuning.
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.
/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */
+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(), 100UL);
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.
+ 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.
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.
+ return 1;
+}
+
+/*
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/28/2009 11:33 AM, Zhai, Edwin wrote: Avi Kivity wrote: +#define KVM_VMX_DEFAULT_PLE_GAP41 +#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). 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. 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. +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). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/27/2009 04:53 PM, Joerg Roedel wrote: 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. 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 -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
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, Avi Kivity wrote: On 09/28/2009 11:33 AM, Zhai, Edwin wrote: Avi Kivity wrote: +#define KVM_VMX_DEFAULT_PLE_GAP41 +#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). 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. 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. +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_ple_hrtimer_v3.patch Description: Binary data
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
Avi Kivity wrote:
+#define KVM_VMX_DEFAULT_PLE_GAP41
+#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).
In general, they are read only except that experienced user may try
different parameter for perf tuning.
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.
/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */
+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(), 100UL);
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.
+ 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.
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.
+ return 1;
+}
+
+/*
kvm_ple_hrtimer_v2.patch
Description: Binary data
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/25/2009 04:11 AM, Zhai, Edwin wrote:
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.
+/*
+ * 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
+ * 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_GAP41
+#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.
/*
+ * Indicate a busy-waiting vcpu in spinlock. We do not enable the PAUSE
+ * exiting, so only get here on cpu with PAUSE-Loop-Exiting.
+ */
+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(), 100UL);
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.
+ 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.
+ return 1;
+}
+
+/*
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/25/2009 11:43 PM, Joerg Roedel wrote: On Wed, Sep 23, 2009 at 05:09:38PM +0300, Avi Kivity wrote: 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. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Sun, Sep 27, 2009 at 10:31:21AM +0200, Avi Kivity wrote: On 09/25/2009 11:43 PM, Joerg Roedel wrote: On Wed, Sep 23, 2009 at 05:09:38PM +0300, Avi Kivity wrote: 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. 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/27/2009 03:46 PM, Joerg Roedel wrote: 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? yield() is a no-op in CFS. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Sun, Sep 27, 2009 at 03:47:55PM +0200, Avi Kivity wrote: On 09/27/2009 03:46 PM, Joerg Roedel wrote: 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? 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/27/2009 04:07 PM, Joerg Roedel wrote: On Sun, Sep 27, 2009 at 03:47:55PM +0200, Avi Kivity wrote: On 09/27/2009 03:46 PM, Joerg Roedel wrote: 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? 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. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Sun, Sep 27, 2009 at 04:18:00PM +0200, Avi Kivity wrote: On 09/27/2009 04:07 PM, Joerg Roedel wrote: On Sun, Sep 27, 2009 at 03:47:55PM +0200, Avi Kivity wrote: On 09/27/2009 03:46 PM, Joerg Roedel wrote: 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? 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. Only one runable task on each cpu is unlikely in a situation of high vcpu overcommit (where pause filtering matters). 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On Wed, Sep 23, 2009 at 05:09:38PM +0300, Avi Kivity wrote: 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 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
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
Avi Kivity wrote:
On 09/23/2009 05:04 PM, Zhai, Edwin wrote:
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.
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.
--
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 edwin.z...@intel.com
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 272514c..2b49454 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -56,6 +56,7 @@
#define SECONDARY_EXEC_ENABLE_VPID 0x0020
#define SECONDARY_EXEC_WBINVD_EXITING 0x0040
#define SECONDARY_EXEC_UNRESTRICTED_GUEST 0x0080
+#define SECONDARY_EXEC_PAUSE_LOOP_EXITING 0x0400
#define PIN_BASED_EXT_INTR_MASK 0x0001
@@ -144,6 +145,8 @@ enum vmcs_field {
VM_ENTRY_INSTRUCTION_LEN= 0x401a,
TPR_THRESHOLD = 0x401c,
SECONDARY_VM_EXEC_CONTROL = 0x401e,
+ PLE_GAP = 0x4020,
+ PLE_WINDOW = 0x4022,
VM_INSTRUCTION_ERROR= 0x4400,
VM_EXIT_REASON = 0x4402,
VM_EXIT_INTR_INFO = 0x4404,
@@ -248,6 +251,7 @@ enum vmcs_field {
#define EXIT_REASON_MSR_READ31
#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
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 3fe0d42..21dbfe9 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -61,6 +61,25 @@ module_param_named(unrestricted_guest,
static int __read_mostly emulate_invalid_guest_state = 0;
module_param(emulate_invalid_guest_state, bool, S_IRUGO);
+/*
+ * 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
+ * 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_GAP41
+#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);
+
struct vmcs {
u32
[PATCH] [RESEND] 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_ple_v2.patch Description: Binary data
Re: [PATCH] [RESEND] KVM:VMX: Add support for Pause-Loop Exiting
On 09/23/2009 05:04 PM, Zhai, Edwin wrote: 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. 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. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
