Re: [PATCH v2 2/3] KVM: Optimize vcpu->requests slow path slightly

[Marcelo Tosatti <mtosatti@redhat.com>]

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.