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:In general, they are read only except that experienced user may try different parameter for perf tuning.+#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).__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.For different spinlock in different OS, and for different workloads, we need different parameter for tuning. It's similar as the enable_ept.I'm not even sure they should be parameters.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.Isn't trace_kvm_exit(exit_reason, ...) enough? We can tell the PLE vmexit from other vmexits.Please add a tracepoint for this (since it can cause significant change in behaviour),+ set_current_state(TASK_INTERRUPTIBLE); + schedule_hrtimeout(&expires, HRTIMER_MODE_ABS); +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).
Attachment:
kvm_ple_hrtimer_v3.patch
Description: Binary data
