Date: Wed, 26 Oct 2016 21:53:45 +0200
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        yang.zhang.wz@gmail.com, feng.wu@intel.com, mst@redhat.com
Subject: Re: [PATCH 1/5] KVM: x86: avoid atomic operations on APICv vmentry
Message-ID: <20161026195344.GB4212@potion>
References: <1476469291-5039-1-git-send-email-pbonzini@redhat.com>
 <1476469291-5039-2-git-send-email-pbonzini@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1476469291-5039-2-git-send-email-pbonzini@redhat.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1707
Lines: 48

2016-10-14 20:21+0200, Paolo Bonzini:
> On some benchmarks (e.g. netperf with ioeventfd disabled), APICv
> posted interrupts turn out to be slower than interrupt injection via
> KVM_REQ_EVENT.
> 
> This patch optimizes a bit the IRR update, avoiding expensive atomic
> operations in the common case where PI.ON=0 at vmentry or the PIR vector
> is mostly zero.  This saves at least 20 cycles (1%) per vmexit, as
> measured by kvm-unit-tests' inl_from_qemu test (20 runs):
> 
>               | enable_apicv=1  |  enable_apicv=0
>               | mean     stdev  |  mean     stdev
>     ----------|-----------------|------------------
>     before    | 5826     32.65  |  5765     47.09
>     after     | 5809     43.42  |  5777     77.02
> 
> Of course, any change in the right column is just placebo effect. :)
> The savings are bigger if interrupts are frequent.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -521,6 +521,12 @@ static inline void pi_set_sn(struct pi_desc *pi_desc)
>  			(unsigned long *)&pi_desc->control);
>  }
>  
> +static inline void pi_clear_on(struct pi_desc *pi_desc)
> +{
> +	clear_bit(POSTED_INTR_ON,
> +  		  (unsigned long *)&pi_desc->control);
   ^^
   bad whitespace.

> +}

We should add an explicit smp_mb__after_atomic() for extra correctness,
because clear_bit() does not guarantee a memory barrier and we must make
sure that pir reads can't be reordered before it.
x86 clear_bit() currently uses locked instruction, though.

> +
>  static inline int pi_test_on(struct pi_desc *pi_desc)
>  {
>  	return test_bit(POSTED_INTR_ON,

Other than that,

Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>