On Thu, Jan 13, 2022 at 04:52:40AM +0000, Li,Rongqing wrote:
> > > > On Wed, Jan 12, 2022 at 08:02:01PM +0800, Li RongQing wrote:
> > > > > vcpu can schedule out when run halt instruction, and set itself to
> > > > > INTERRUPTIBLE and switch to idle thread, vcpu should not be set
> > > > > preempted for this condition
> Is it possible if guest has KVM_HINTS_REALTIME feature, but its HLT instruction is emulated by KVM?
> If it is possible, this condition has been performance degradation, since vcpu_is_preempted is not __kvm_vcpu_is_preempted, will return false.
>
> Similar, guest has nopvspin, but HLT instruction is emulated;
>
> Should we adjust the setting of pv_ops.lock.vcpu_is_preempted as below
> And I see the performance boost when guest has nopvspin, but HLT instruction is emulated with below change
I'm a little confused; the initial patch explicitly avoided setting
preempted on HLT, while the below causes it to be set more.
That said; I don't object to this, but I'm not convinced it's right
either. If you have HINTS_REALTIME (horrible naming aside) this means
you have pinned vCPU and no overcommit, in which case setting preempted
makes no sense.
*confused*
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 59abbda..b061d17 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -1048,6 +1048,11 @@ void __init kvm_spinlock_init(void)
> return;
> }
>
> + if (kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
> + pv_ops.lock.vcpu_is_preempted =
> + PV_CALLEE_SAVE(__kvm_vcpu_is_preempted);
> + }
> +
> /*
> * Disable PV spinlocks and use native qspinlock when dedicated pCPUs
> * are available.
> @@ -1076,10 +1081,6 @@ void __init kvm_spinlock_init(void)
> pv_ops.lock.wait = kvm_wait;
> pv_ops.lock.kick = kvm_kick_cpu;
>
> - if (kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
> - pv_ops.lock.vcpu_is_preempted =
> - PV_CALLEE_SAVE(__kvm_vcpu_is_preempted);
> - }
> /*
> * When PV spinlock is enabled which is preferred over
> * virt_spin_lock(), virt_spin_lock_key's value is meaningless.
>
>
> -Li
