linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] srcu: API for barrier after srcu read unlock
@ 2013-11-04 20:36 Michael S. Tsirkin
  2013-11-04 20:36 ` [PATCH 2/2] kvm: optimize out smp_mb after srcu_read_unlock Michael S. Tsirkin
  2013-11-04 20:55 ` [PATCH 1/2] srcu: API for barrier after srcu read unlock Paul E. McKenney
  0 siblings, 2 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2013-11-04 20:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: Paul E. McKenney, Lai Jiangshan

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] kvm: optimize out smp_mb after srcu_read_unlock
  2013-11-04 20:36 [PATCH 1/2] srcu: API for barrier after srcu read unlock Michael S. Tsirkin
@ 2013-11-04 20:36 ` Michael S. Tsirkin
  2013-11-04 20:55 ` [PATCH 1/2] srcu: API for barrier after srcu read unlock Paul E. McKenney
  1 sibling, 0 replies; 5+ messages in thread
From: Michael S. Tsirkin @ 2013-11-04 20:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: Gleb Natapov, Paolo Bonzini, kvm

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock
  2013-11-04 20:36 [PATCH 1/2] srcu: API for barrier after srcu read unlock Michael S. Tsirkin
  2013-11-04 20:36 ` [PATCH 2/2] kvm: optimize out smp_mb after srcu_read_unlock Michael S. Tsirkin
@ 2013-11-04 20:55 ` Paul E. McKenney
  2013-11-06 15:00   ` Lai Jiangshan
  1 sibling, 1 reply; 5+ messages in thread
From: Paul E. McKenney @ 2013-11-04 20:55 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: linux-kernel, Lai Jiangshan

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
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock
  2013-11-04 20:55 ` [PATCH 1/2] srcu: API for barrier after srcu read unlock Paul E. McKenney
@ 2013-11-06 15:00   ` Lai Jiangshan
  2013-11-06 20:00     ` Paul E. McKenney
  0 siblings, 1 reply; 5+ messages in thread
From: Lai Jiangshan @ 2013-11-06 15:00 UTC (permalink / raw)
  To: paulmck; +Cc: Michael S. Tsirkin, linux-kernel

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/
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/2] srcu: API for barrier after srcu read unlock
  2013-11-06 15:00   ` Lai Jiangshan
@ 2013-11-06 20:00     ` Paul E. McKenney
  0 siblings, 0 replies; 5+ messages in thread
From: Paul E. McKenney @ 2013-11-06 20:00 UTC (permalink / raw)
  To: Lai Jiangshan; +Cc: Michael S. Tsirkin, linux-kernel

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/
> > 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-11-06 20:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-04 20:36 [PATCH 1/2] srcu: API for barrier after srcu read unlock Michael S. Tsirkin
2013-11-04 20:36 ` [PATCH 2/2] kvm: optimize out smp_mb after srcu_read_unlock Michael S. Tsirkin
2013-11-04 20:55 ` [PATCH 1/2] srcu: API for barrier after srcu read unlock Paul E. McKenney
2013-11-06 15:00   ` Lai Jiangshan
2013-11-06 20:00     ` Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).