[PATCH 1/2] srcu: API for barrier after srcu read unlock

[PATCH 1/2] srcu: API for barrier after srcu read unlock

[Michael S. Tsirkin]

srcu read lock/unlock include a full memory barrier
but that's an implementation detail.
Add an API for make memory fencing explicit for
users that need this barrier, to make sure we
can change it as needed without breaking all users.

Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/linux/srcu.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index c114614..9b058ee 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -237,4 +237,18 @@ static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
 	__srcu_read_unlock(sp, idx);
 }
 
+/**
+ * smp_mb__after_srcu_read_unlock - ensure full ordering after srcu_read_unlock
+ *
+ * Converts the preceding srcu_read_unlock into a two-way memory barrier.
+ *
+ * Call this after srcu_read_unlock, to guarantee that all memory operations
+ * that occur after smp_mb__after_srcu_read_unlock will appear to happen after
+ * the preceding srcu_read_unlock.
+ */
+static inline void smp_mb__after_srcu_read_unlock(void)
+{
+	/* __srcu_read_unlock has smp_mb() internally so nothing to do here. */
+}
+
 #endif
-- 
MST

[PATCH 2/2] kvm: optimize out smp_mb after srcu_read_unlock

[Michael S. Tsirkin]

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() using smp_mb__after_srcu_read_unlock instead.

Unsurprisingly, the gain is small but measureable using the unit test
microbenchmark:
before
        vmcall in the ballpark of 1410 cycles
after
        vmcall in the ballpark of 1360 cycles

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/kvm/x86.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e5ca72a..d609dce 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5945,10 +5945,12 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 
 	vcpu->mode = IN_GUEST_MODE;
 
+	srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);
+
 	/* We should set ->mode before check ->requests,
 	 * see the comment in make_all_cpus_request.
 	 */
-	smp_mb();
+	smp_mb__after_srcu_read_unlock();
 
 	local_irq_disable();
 
@@ -5958,12 +5960,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		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

Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock

[Paul E. McKenney]

On Mon, Nov 04, 2013 at 10:36:17PM +0200, Michael S. Tsirkin wrote:
> srcu read lock/unlock include a full memory barrier
> but that's an implementation detail.
> Add an API for make memory fencing explicit for
> users that need this barrier, to make sure we
> can change it as needed without breaking all users.
> 
> Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> ---
>  include/linux/srcu.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> index c114614..9b058ee 100644
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -237,4 +237,18 @@ static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
>  	__srcu_read_unlock(sp, idx);
>  }
> 
> +/**
> + * smp_mb__after_srcu_read_unlock - ensure full ordering after srcu_read_unlock
> + *
> + * Converts the preceding srcu_read_unlock into a two-way memory barrier.
> + *
> + * Call this after srcu_read_unlock, to guarantee that all memory operations
> + * that occur after smp_mb__after_srcu_read_unlock will appear to happen after
> + * the preceding srcu_read_unlock.
> + */
> +static inline void smp_mb__after_srcu_read_unlock(void)
> +{
> +	/* __srcu_read_unlock has smp_mb() internally so nothing to do here. */
> +}
> +
>  #endif
> -- 
> MST
> 

Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock

[Lai Jiangshan]

On 11/05/2013 04:55 AM, Paul E. McKenney wrote:
> On Mon, Nov 04, 2013 at 10:36:17PM +0200, Michael S. Tsirkin wrote:
>> srcu read lock/unlock include a full memory barrier
>> but that's an implementation detail.
>> Add an API for make memory fencing explicit for
>> users that need this barrier, to make sure we
>> can change it as needed without breaking all users.
>>
>> Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

Acked-by: "Lai Jiangshan" <laijs@cn.fujitsu.com>

> 
>> ---
>>  include/linux/srcu.h | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
>> index c114614..9b058ee 100644
>> --- a/include/linux/srcu.h
>> +++ b/include/linux/srcu.h
>> @@ -237,4 +237,18 @@ static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
>>  	__srcu_read_unlock(sp, idx);
>>  }
>>
>> +/**
>> + * smp_mb__after_srcu_read_unlock - ensure full ordering after srcu_read_unlock
>> + *
>> + * Converts the preceding srcu_read_unlock into a two-way memory barrier.
>> + *
>> + * Call this after srcu_read_unlock, to guarantee that all memory operations
>> + * that occur after smp_mb__after_srcu_read_unlock will appear to happen after
>> + * the preceding srcu_read_unlock.
>> + */
>> +static inline void smp_mb__after_srcu_read_unlock(void)
>> +{
>> +	/* __srcu_read_unlock has smp_mb() internally so nothing to do here. */
>> +}
>> +
>>  #endif
>> -- 
>> MST
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock

[Paul E. McKenney]

On Wed, Nov 06, 2013 at 11:00:21PM +0800, Lai Jiangshan wrote:
> On 11/05/2013 04:55 AM, Paul E. McKenney wrote:
> > On Mon, Nov 04, 2013 at 10:36:17PM +0200, Michael S. Tsirkin wrote:
> >> srcu read lock/unlock include a full memory barrier
> >> but that's an implementation detail.
> >> Add an API for make memory fencing explicit for
> >> users that need this barrier, to make sure we
> >> can change it as needed without breaking all users.
> >>
> >> Acked-by: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> 
> Acked-by: "Lai Jiangshan" <laijs@cn.fujitsu.com>

Very good, queued for 3.14.

							Thanx, Paul

> >> ---
> >>  include/linux/srcu.h | 14 ++++++++++++++
> >>  1 file changed, 14 insertions(+)
> >>
> >> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> >> index c114614..9b058ee 100644
> >> --- a/include/linux/srcu.h
> >> +++ b/include/linux/srcu.h
> >> @@ -237,4 +237,18 @@ static inline void srcu_read_unlock(struct srcu_struct *sp, int idx)
> >>  	__srcu_read_unlock(sp, idx);
> >>  }
> >>
> >> +/**
> >> + * smp_mb__after_srcu_read_unlock - ensure full ordering after srcu_read_unlock
> >> + *
> >> + * Converts the preceding srcu_read_unlock into a two-way memory barrier.
> >> + *
> >> + * Call this after srcu_read_unlock, to guarantee that all memory operations
> >> + * that occur after smp_mb__after_srcu_read_unlock will appear to happen after
> >> + * the preceding srcu_read_unlock.
> >> + */
> >> +static inline void smp_mb__after_srcu_read_unlock(void)
> >> +{
> >> +	/* __srcu_read_unlock has smp_mb() internally so nothing to do here. */
> >> +}
> >> +
> >>  #endif
> >> -- 
> >> MST
> >>
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at  http://www.tux.org/lkml/
> > 
>