Hi Paolo,
2018-03-27 3:43 GMT+08:00 Eduardo Habkost <ehabkost@xxxxxxxxxx>:
> On Sun, Mar 25, 2018 at 11:33:01AM +0800, Wanpeng Li wrote:
>> 2018-03-24 4:18 GMT+08:00 Eduardo Habkost <ehabkost@xxxxxxxxxx>:
>> > On Fri, Mar 16, 2018 at 07:36:42AM -0700, Wanpeng Li wrote:
>> >> From: Wanpeng Li <wanpengli@xxxxxxxxxxx>
>> >>
>> >> This patch adds support for KVM_CAP_X86_DISABLE_EXITS. Provides userspace with
>> >> per-VM capability(KVM_CAP_X86_DISABLE_EXITS) to not intercept MWAIT/HLT/PAUSE
>> >> in order that to improve latency in some workloads.
>> >>
>> >> Cc: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>> >> Cc: Radim Krčmář <rkrcmar@xxxxxxxxxx>
>> >> Cc: Eduardo Habkost <ehabkost@xxxxxxxxxx>
>> >> Signed-off-by: Wanpeng Li <wanpengli@xxxxxxxxxxx>
>> >
>> >
>> > Thanks.
>> >
>> > Patch looks good (except for comment below), but I would like to
>> > see QEMU documentation mentioning what exactly are the practical
>> > consequences of setting "+kvm-hint-dedicated" (especially what
>> > could happen if people enable the flag without properly
>> > configuring vCPU pinning).
>> >
>> >
>> > [...]
>> >> + if (env->features[FEAT_KVM_HINTS] & KVM_HINTS_DEDICATED) {
>> >> + int disable_exits = kvm_check_extension(cs->kvm_state, KVM_CAP_X86_DISABLE_EXITS);
>> >> + if (disable_exits) {
>> >> + disable_exits &= (KVM_X86_DISABLE_EXITS_MWAIT |
>> >> + KVM_X86_DISABLE_EXITS_HLT |
>> >> + KVM_X86_DISABLE_EXITS_PAUSE);
>> >> + }
>> >
>> > Documentation/virtual/kvm/api.txt says that KVM_FEATURE_PV_UNHALT
>> > shouldn't be enabled if disabling HLT exits. This needs to be
>> > handled by QEMU.
>>
>> This is handled by KVM(in kvm_update_cpuid()) currently to avoid kvm
>> userspace doing something crazy.
>> https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=queue&id=caa057a2cad647fb368a12c8e6c410ac4c28e063
>
> This seems to disable kvm-pv-unhalt silently if
> KVM_X86_DISABLE_EXITS_HLT is enabled. We shouldn't do that if
> the user explicitly requested +kvm-pv-unhalt in the command-line.
>
>>
>> >
>> > Probably the simplest solution is to not allow kvm-hint-dedicated
>> > to be enabled if kvm-pv-unhalt is. This should be mentioned in
>> > QEMU documentation, also, especially considering that we might
>> > enable kvm-pv-unhalt by default in future QEMU versions.
>>
>> As Locking guy Waiman mentioned before:
>> > Generally speaking, unfair lock performs well for VMs with a small number of vCPUs. Native qspinlock may perform better than pvqspinlock if there is vCPU pinning and there is no vCPU over-commitment.
>> I think +kvm-hint-dedicated, -kvm-pv-unhalt is more suitable for vCPU
>> pinning and there is no vCPU over-commitment, on the contrary,
>> -kvm-hint-dedicated, +kvm-pv-unhalt is more prefer.
>
> Disabling kvm-pv-unhalt by default if only "-cpu
> ...,+kvm-hint-dedicated" is used makes sense. But we still need
> the system to not silently ignore options if
> "-cpu ...,+kvm-pv-unhalt,+kvm-hint-dedicated" is specified.
What's your opinion for these two comments from Eduardo?
Regards,
Wanpeng Li
