On Wed, Oct 30, 2013 at 09:09:29PM +0200, Michael S. Tsirkin wrote: > I noticed that srcu_read_lock/unlock both have a memory barrier, > so just by moving srcu_read_unlock earlier we can get rid of > one call to smp_mb(). > > Unsurprisingly, the gain is small but measureable using the unit test > microbenchmark: > before > vmcall 1407 > after > vmcall 1357 > > Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx> > > -- > > I didn't stress test this yet, sending out for early review/flames. > > Paul, could you review this patch please? > Documentation/memory-barriers.txt says that unlock has a weaker > uni-directional barrier, but in practice srcu_read_unlock calls > smp_mb(). > > Is it OK to rely on this? If not, can I add > smp_mb__after_srcu_read_unlock (making it an empty macro for now) > so we can avoid an actual extra smp_mb()? Please use smp_mb__after_srcu_read_unlock(). After all, it was not that long ago that srcu_read_unlock() contained no memory barriers, and perhaps some day it won't need to once again. Thanx, Paul > Thanks. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 8617c9d..a48fb36 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -5949,8 +5949,10 @@ restore: > > /* We should set ->mode before check ->requests, > * see the comment in make_all_cpus_request. > + * > + * srcu_read_unlock below acts as a memory barrier. > */ > - smp_mb(); > + srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > > local_irq_disable(); > > @@ -5960,12 +5962,11 @@ restore: > smp_wmb(); > local_irq_enable(); > preempt_enable(); > + vcpu->srcu_idx = srcu_read_lock(&vcpu->kvm->srcu); > r = 1; > goto cancel_injection; > } > > - srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx); > - > if (req_immediate_exit) > smp_send_reschedule(vcpu->cpu); > > > -- > MST > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
