On 09/28/2009 11:33 AM, Zhai, Edwin wrote:

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.


__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

Reply via email to