From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Longpeng (Mike)" Subject: Re: [RFC] KVM: optimize the kvm_vcpu_on_spin Date: Mon, 31 Jul 2017 21:01:59 +0800 Message-ID: <597F2A47.4020209@huawei.com> References: <1501309377-195256-1-git-send-email-longpeng2@huawei.com> <597F1DAE.4020809@huawei.com> <20170731143155.30a1b0dd@gondolin> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Cc: , , , , , , , , , , , , To: Cornelia Huck , David Hildenbrand Return-path: In-Reply-To: <20170731143155.30a1b0dd@gondolin> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 2017/7/31 20:31, Cornelia Huck wrote: > On Mon, 31 Jul 2017 20:08:14 +0800 > "Longpeng (Mike)" wrote: > >> Hi David, >> >> On 2017/7/31 19:31, David Hildenbrand wrote: > >>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >>>> index 648b34c..f8f0d74 100644 >>>> --- a/include/linux/kvm_host.h >>>> +++ b/include/linux/kvm_host.h >>>> @@ -272,6 +272,9 @@ struct kvm_vcpu { >>>> } spin_loop; >>>> #endif >>>> bool preempted; >>>> + /* If vcpu is in kernel-mode when preempted */ >>>> + bool in_kernmode; >>>> + >>> >>> Why do you have to store that ... >>> >> >>> [...]> + me->in_kernmode = kvm_arch_vcpu_spin_kernmode(me); >>>> kvm_vcpu_set_in_spin_loop(me, true); >>>> /* >>>> * We boost the priority of a VCPU that is runnable but not >>>> @@ -2351,6 +2353,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me) >>>> continue; >>>> if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu)) >>>> continue; >>>> + if (me->in_kernmode && !vcpu->in_kernmode) >>> >>> Wouldn't it be easier to simply have >>> >>> in_kernel = kvm_arch_vcpu_in_kernel(me); >>> ... >>> if (in_kernel && !kvm_arch_vcpu_in_kernel(vcpu)) >>> ... >>> >> >> I'm not sure whether the operation of get the vcpu's priority-level is >> expensive on all architectures, so I record it in kvm_sched_out() for >> minimal the extra cycles cost in kvm_vcpu_on_spin(). > > As it is now, this handling looks a bit inconsistent. You only update > the field on sched-out via preemption _or_ if kvm_vcpu_on_spin is > called for the vcpu. In most contexts, this field will have stale > content. > > Also, would checking for kernel mode be more expensive than the various > other checks already done in this function? > > [I like David's suggestion.] > Hi Cornelia & David, I'll take your suggestion, thanks :) >> >>>> + continue; >>>> if (!kvm_vcpu_eligible_for_directed_yield(vcpu)) >>>> continue; >>>> > > . > -- Regards, Longpeng(Mike)