Avi Kivity wrote:
+#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).
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(), 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.
+ 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; +} + +/*
Attachment:
kvm_ple_hrtimer_v2.patch
Description: Binary data
