> -----邮件原件-----
> 发件人: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> 发送时间: 2022年1月13日 17:34
> 收件人: Li,Rongqing <lirongqing@xxxxxxxxx>
> 抄送: Sean Christopherson <seanjc@xxxxxxxxxx>; pbonzini@xxxxxxxxxx;
> vkuznets@xxxxxxxxxx; wanpengli@xxxxxxxxxxx; jmattson@xxxxxxxxxx;
> tglx@xxxxxxxxxxxxx; bp@xxxxxxxxx; x86@xxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> joro@xxxxxxxxxx; Wang,Guangju <wangguangju@xxxxxxxxx>
> 主题: Re: 答复: [PATCH] KVM: X86: set vcpu preempted only if it is preempted
>
> 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*
>
Sorry
I first notice that kvm_vcpu_is_preempted() always return true from code review, even if vcpu is idle, think it is unreasonable, so have first patch.
After see feedback, do some tests, find the first patch will cause unixbench pipe performance degrading in one copy mode, which prove what your said, kvm_vcpu_is_preempted return true nearly always, which makes unixbench two thread running in same one vcpu sometime, so less wakeup, less rescheduling ipi
See kvm_vcpu_is_preempted() works only if guest has not nopvspin kernel cmdline and has not KVM_HINTS_REALTIME feature in kvm_spinlock_init, so there is new patch
Thanks
-LI
> > 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
