DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 3C495E7B67
Subject: Re: [RFC] KVM: optimize the kvm_vcpu_on_spin
To: "Longpeng(Mike)" <longpeng2@huawei.com>, pbonzini@redhat.com,
        rkrcmar@redhat.com
Cc: agraf@suse.com, borntraeger@de.ibm.com, cohuck@redhat.com,
        christoffer.dall@linaro.org, marc.zyngier@arm.com,
        james.hogan@imgtec.com, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org, weidong.huang@huawei.com,
        arei.gonglei@huawei.com, wangxinxin.wang@huawei.com,
        longpeng.mike@gmail.com
References: <1501309377-195256-1-git-send-email-longpeng2@huawei.com>
From: David Hildenbrand <david@redhat.com>
Organization: Red Hat GmbH
Message-ID: <bcbdafa9-349f-345e-dde1-28bfa1cf5826@redhat.com>
Date: Mon, 31 Jul 2017 13:31:45 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <1501309377-195256-1-git-send-email-longpeng2@huawei.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1889
Lines: 74

[no idea if this change makes sense (and especially if it has any bad
side effects), do you have performance numbers? I'll just have a look at
the general structure of the patch in the meanwhile]

> +bool kvm_arch_vcpu_spin_kernmode(struct kvm_vcpu *vcpu)

kvm_arch_vcpu_in_kernel() ?

> +{
> +	return kvm_x86_ops->get_cpl(vcpu) == 0;
> +}
> +
>  int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
>  {
>  	return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> 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))
...

> +				continue;
>  			if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
>  				continue;
>  
> @@ -4009,8 +4013,11 @@ static void kvm_sched_out(struct preempt_notifier *pn,
>  {
>  	struct kvm_vcpu *vcpu = preempt_notifier_to_vcpu(pn);
>  
> -	if (current->state == TASK_RUNNING)
> +	if (current->state == TASK_RUNNING) {
>  		vcpu->preempted = true;
> +		vcpu->in_kernmode = kvm_arch_vcpu_spin_kernmode(vcpu);
> +	}
> +

so you don't have to do this change, too.

>  	kvm_arch_vcpu_put(vcpu);
>  }
>  
> 


-- 

Thanks,

David