Re: [PATCH 1/5] KVM: x86: avoid atomic operations on APICv vmentry

From: Michael S. Tsirkin
Date: Wed Oct 26 2016 - 17:42:59 EST


On Wed, Oct 26, 2016 at 09:53:45PM +0200, Radim KrÄmÃÅ wrote:
> 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@xxxxxxxxxx>
> > ---
> > 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.

smp_mb__after_atomic is empty on x86 so it's
a documentation thing, not a correctness thing anyway.

> > +
> > 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@xxxxxxxxxx>