Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752183AbdGaMcD (ORCPT ); Mon, 31 Jul 2017 08:32:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49592 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751784AbdGaMcC (ORCPT ); Mon, 31 Jul 2017 08:32:02 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com DA90FCC924 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=cohuck@redhat.com Date: Mon, 31 Jul 2017 14:31:55 +0200 From: Cornelia Huck To: "Longpeng (Mike)" Cc: David Hildenbrand , , , , , , , , , , , , , Subject: Re: [RFC] KVM: optimize the kvm_vcpu_on_spin Message-ID: <20170731143155.30a1b0dd@gondolin> In-Reply-To: <597F1DAE.4020809@huawei.com> References: <1501309377-195256-1-git-send-email-longpeng2@huawei.com> <597F1DAE.4020809@huawei.com> Organization: Red Hat GmbH MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 31 Jul 2017 12:32:02 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1827 Lines: 59 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.] > > >> + continue; > >> if (!kvm_vcpu_eligible_for_directed_yield(vcpu)) > >> continue; > >>