From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v2 2/3] KVM: Optimize vcpu->requests slow path slightly
Date: Wed, 30 May 2012 17:03:34 -0300
Message-ID: <20120530200334.GA23297@amt.cnet>
References: <1337521768-14182-1-git-send-email-avi@redhat.com>
 <1337521768-14182-3-git-send-email-avi@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:24277 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753267Ab2E3UJJ (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 30 May 2012 16:09:09 -0400
Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4UK99tV020502
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <kvm@vger.kernel.org>; Wed, 30 May 2012 16:09:09 -0400
Content-Disposition: inline
In-Reply-To: <1337521768-14182-3-git-send-email-avi@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Sun, May 20, 2012 at 04:49:27PM +0300, Avi Kivity wrote:
> Instead of using a atomic operation per active request, use just one
> to get all requests at once, then check them with local ops.  This
> probably isn't any faster, since simultaneous requests are rare, but
> it does reduce code size.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>
> ---
>  arch/x86/kvm/x86.c |   33 ++++++++++++++++++---------------
>  1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 953e692..c0209eb 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5232,55 +5232,58 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
>  	bool req_int_win = !irqchip_in_kernel(vcpu->kvm) &&
>  		vcpu->run->request_interrupt_window;
>  	bool req_immediate_exit = 0;
> +	ulong reqs;
>  
>  	if (unlikely(req_int_win))
>  		kvm_make_request(KVM_REQ_EVENT, vcpu);
>  
>  	if (vcpu->requests) {
> -		if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu))
> +		reqs = xchg(&vcpu->requests, 0UL);
> +
> +		if (test_bit(KVM_REQ_MMU_RELOAD, &reqs))
>  			kvm_mmu_unload(vcpu);
> -		if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
> +		if (test_bit(KVM_REQ_MIGRATE_TIMER, &reqs))
>  			__kvm_migrate_timers(vcpu);
> -		if (kvm_check_request(KVM_REQ_CLOCK_UPDATE, vcpu)) {
> +		if (test_bit(KVM_REQ_CLOCK_UPDATE, &reqs)) {
>  			r = kvm_guest_time_update(vcpu);
>  			if (unlikely(r))
>  				goto out;
>  		}

Bailing out loses requests in "reqs". 

Caching the requests makes the following type of sequence behave strangely

req = xchg(&vcpu->requests);
if request is set
    request handler
        ...
        set REQ_EVENT
        ...

prepare for guest entry
vcpu->requests set
    bail

The code is more straightforward as it is.