[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Paolo Bonzini]

lwsync orders loads with respect to other loads, and stores with respect
to other stores.  eieio instead only orders stores.  Use them to
implement rmb/wmb/smp_wmb.
---
 urcu/arch/ppc.h |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
index a03d688..3a6c702 100644
--- a/urcu/arch/ppc.h
+++ b/urcu/arch/ppc.h
@@ -32,7 +32,14 @@ extern "C" {
 /* Include size of POWER5+ L3 cache lines: 256 bytes */
 #define CAA_CACHE_LINE_SIZE	256
 
-#define cmm_mb()    asm volatile("sync":::"memory")
+#define cmm_mb()	 asm volatile("sync":::"memory")
+#define cmm_rmb()	 asm volatile("lwsync":::"memory")
+#define cmm_wmb()	 asm volatile("lwsync":::"memory")
+
+/* eieio is good for a write memory barrier, assuming we don't
+ * need to order cacheable and non-cacheable stores with respect
+ * to each other.  */
+#define cmm_smp_wmb()    asm volatile("eieio":::"memory")
 
 #define mftbl()						\
 	({ 						\
-- 
1.7.6

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> lwsync orders loads with respect to other loads, and stores with respect
> to other stores.  eieio instead only orders stores.  Use them to
> implement rmb/wmb/smp_wmb.
> ---
>  urcu/arch/ppc.h |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
> index a03d688..3a6c702 100644
> --- a/urcu/arch/ppc.h
> +++ b/urcu/arch/ppc.h
> @@ -32,7 +32,14 @@ extern "C" {
>  /* Include size of POWER5+ L3 cache lines: 256 bytes */
>  #define CAA_CACHE_LINE_SIZE	256
>  
> -#define cmm_mb()    asm volatile("sync":::"memory")
> +#define cmm_mb()	 asm volatile("sync":::"memory")
> +#define cmm_rmb()	 asm volatile("lwsync":::"memory")
> +#define cmm_wmb()	 asm volatile("lwsync":::"memory")

I don't think lwsync orders non-cacheable memory operations. Therefore,
is it the right choice for cmm_rmb/cmm_wmb ?

It might be a good choice for cmm_smp_rmb/cmm_smp_wmb though.

> +
> +/* eieio is good for a write memory barrier, assuming we don't
> + * need to order cacheable and non-cacheable stores with respect
> + * to each other.  */
> +#define cmm_smp_wmb()    asm volatile("eieio":::"memory")

For this one, I wonder which of lwsync or eieio perform best ? But the
question that arises is whether lwsync is available on all PowerPC
flavors, and if not, which. We may want to have a configure/compile
option to specialize the output generated depending on the powerpc
flavor.

If eieio is slower than lwsync on recent powerpc, then slowing down new
generations for speedup of older generations seems like a global loss.

Thanks,

Mathieu

>  
>  #define mftbl()						\
>  	({ 						\
> -- 
> 1.7.6
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Paolo Bonzini]

On 09/20/2011 06:31 PM, Mathieu Desnoyers wrote:
> I don't think lwsync orders non-cacheable memory operations. Therefore,
> is it the right choice for cmm_rmb/cmm_wmb ?

I think you're right. "eieio;lwsync" is good for rmb, lwsync is good for 
wmb/smp_rmb/smp_wmb.

Paolo

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> On 09/20/2011 06:31 PM, Mathieu Desnoyers wrote:
>> I don't think lwsync orders non-cacheable memory operations. Therefore,
>> is it the right choice for cmm_rmb/cmm_wmb ?
>
> I think you're right. "eieio;lwsync" is good for rmb, lwsync is good for  
> wmb/smp_rmb/smp_wmb.

I'm not convinced that the "eieio; lwsync" combo would provide the
ordering we're looking for for cmm_rmb(). AFAIK, eieio orders,
separately, a) cacheable stores and b) loads and stores to non-cacheable
memory. AFAIK, lwsync orders cacheable memory ops, but not loads with
respect to previous stores. So basically, this combo lacks ordering of
non-cacheable memory accesses with respect to cachable memory accesses.

Why would lwsync be good for cmm_wmb ? Does it order non-cacheable
writes ?

I'd be tempted to stick to "sync" for both cmm_rmb() and cmm_wmb().

Now about cmm_smp_rmb(), lwsync seems like a good choice. For
cmm_smp_wmb(), lwsync would be appropriate too I think in the general
case. We could have specific compile options for older architectures
that lack lwsync support that uses eieio instead, but this would be just
an optimisation, because lwsync AFAIK falls back to "sync" on these
older archs.

It would be really good to have the double-check of a PowerPC expert on
this before we weaken any of these primitives though.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Paolo Bonzini]

On 09/20/2011 06:51 PM, Mathieu Desnoyers wrote:
>> >  I think you're right. "eieio;lwsync" is good for rmb, lwsync is good for
>> >  wmb/smp_rmb/smp_wmb.
> I'm not convinced that the "eieio; lwsync" combo would provide the
> ordering we're looking for for cmm_rmb(). AFAIK, eieio orders,
> separately, a) cacheable stores and b) loads and stores to non-cacheable
> memory. AFAIK, lwsync orders cacheable memory ops, but not loads with
> respect to previous stores. So basically, this combo lacks ordering of
> non-cacheable memory accesses with respect to cachable memory accesses.

Yeah, better safe than sorry.

> Why would lwsync be good for cmm_wmb ? Does it order non-cacheable
> writes ?

The manuals say non-cacheable writes are always ordered.

Paolo

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Paul E. McKenney]

On Wed, Sep 21, 2011 at 09:20:08AM +0200, Paolo Bonzini wrote:
> On 09/20/2011 06:51 PM, Mathieu Desnoyers wrote:
> >>>  I think you're right. "eieio;lwsync" is good for rmb, lwsync is good for
> >>>  wmb/smp_rmb/smp_wmb.
> >I'm not convinced that the "eieio; lwsync" combo would provide the
> >ordering we're looking for for cmm_rmb(). AFAIK, eieio orders,
> >separately, a) cacheable stores and b) loads and stores to non-cacheable
> >memory. AFAIK, lwsync orders cacheable memory ops, but not loads with
> >respect to previous stores. So basically, this combo lacks ordering of
> >non-cacheable memory accesses with respect to cachable memory accesses.
> 
> Yeah, better safe than sorry.
> 
> >Why would lwsync be good for cmm_wmb ? Does it order non-cacheable
> >writes ?
> 
> The manuals say non-cacheable writes are always ordered.

But at best only against other non-cacheable writes.  Again, they can
slip out of lock-based critical sections.

							Thanx, Paul

[ltt-dev] [URCU PATCH] cmm: provide lightweight rmb/wmb on PPC

[Paul E. McKenney]

On Tue, Sep 20, 2011 at 06:39:33PM +0200, Paolo Bonzini wrote:
> On 09/20/2011 06:31 PM, Mathieu Desnoyers wrote:
> >I don't think lwsync orders non-cacheable memory operations. Therefore,
> >is it the right choice for cmm_rmb/cmm_wmb ?
> 
> I think you're right. "eieio;lwsync" is good for rmb, lwsync is good
> for wmb/smp_rmb/smp_wmb.

But eieio;lwsync won't order non-cacheable memory operations against
cacheable memory operations.  This means that your MMIO accesses can
slip out of your lock-based critical section, which is a very bad
thing indeed.

For general-purpose use, I strongly recommend full "sync" for mb(),
rmb(), and wmb().  For memory-only use, the developer should not
be using these, but smp_mb(), smp_rmb(), and smp_wmb() instead.

							Thanx, Paul

[ltt-dev] [PATCH v2] cmm: provide lightweight smp_rmb/smp_wmb on PPC

[Paolo Bonzini]

lwsync orders loads in cacheable memory with respect to other loads,
and stores in cacheable memory with respect to other stores.  Use it
to implement smp_rmb/smp_wmb.

The heavy-weight sync is still used for the "full" rmb/wmb operations,
as well as for smp_mb.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 urcu/arch/ppc.h |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
index a03d688..05f7db6 100644
--- a/urcu/arch/ppc.h
+++ b/urcu/arch/ppc.h
@@ -32,7 +32,15 @@ extern "C" {
 /* Include size of POWER5+ L3 cache lines: 256 bytes */
 #define CAA_CACHE_LINE_SIZE	256
 
-#define cmm_mb()    asm volatile("sync":::"memory")
+#define cmm_mb()         asm volatile("sync":::"memory")
+
+/* lwsync does not preserve ordering of cacheable vs. non-cacheable
+ * accesses, but it is good when MMIO is not in use.  An eieio+lwsync
+ * pair is also not enough for rmb, because it will order cacheable
+ * and non-cacheable memory operations separately---i.e. not the latter
+ * against the former.  */
+#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
+#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")
 
 #define mftbl()						\
 	({ 						\
-- 
1.7.6

[ltt-dev] [PATCH v2] cmm: provide lightweight smp_rmb/smp_wmb on PPC

[Mathieu Desnoyers]

* Paolo Bonzini (pbonzini at redhat.com) wrote:
> lwsync orders loads in cacheable memory with respect to other loads,
> and stores in cacheable memory with respect to other stores.  Use it
> to implement smp_rmb/smp_wmb.
> 
> The heavy-weight sync is still used for the "full" rmb/wmb operations,
> as well as for smp_mb.

[ Edit by Mathieu Desnoyers: rephrased the comments around the memory 
  barriers. ]


+/*
+ * Use sync for all cmm_mb/rmb/wmb barriers because lwsync does not
+ * preserve ordering of cacheable vs. non-cacheable accesses, so it
+ * should not be used to order with respect to MMIO operations.  An
+ * eieio+lwsync pair is also not enough for cmm_rmb, because it will
+ * order cacheable and non-cacheable memory operations separately---i.e.
+ * not the latter against the former.
+ */
+#define cmm_mb()         asm volatile("sync":::"memory")
+
+/*
+ * lwsync orders loads in cacheable memory with respect to other loads,
+ * and stores in cacheable memory with respect to other stores.
+ * Therefore, use it for barriers ordering accesses to cacheable memory
+ * only.
+ */
+#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
+#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")


Merged, thanks!

Mathieu

> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  urcu/arch/ppc.h |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
> index a03d688..05f7db6 100644
> --- a/urcu/arch/ppc.h
> +++ b/urcu/arch/ppc.h
> @@ -32,7 +32,15 @@ extern "C" {
>  /* Include size of POWER5+ L3 cache lines: 256 bytes */
>  #define CAA_CACHE_LINE_SIZE	256
>  
> -#define cmm_mb()    asm volatile("sync":::"memory")
> +#define cmm_mb()         asm volatile("sync":::"memory")
> +
> +/* lwsync does not preserve ordering of cacheable vs. non-cacheable
> + * accesses, but it is good when MMIO is not in use.  An eieio+lwsync
> + * pair is also not enough for rmb, because it will order cacheable
> + * and non-cacheable memory operations separately---i.e. not the latter
> + * against the former.  */
> +#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
> +#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")
>  
>  #define mftbl()						\
>  	({ 						\
> -- 
> 1.7.6
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH v2] cmm: provide lightweight smp_rmb/smp_wmb on PPC

[Paul E. McKenney]

On Thu, Sep 22, 2011 at 10:42:52AM +0200, Paolo Bonzini wrote:
> lwsync orders loads in cacheable memory with respect to other loads,
> and stores in cacheable memory with respect to other stores.  Use it
> to implement smp_rmb/smp_wmb.
> 
> The heavy-weight sync is still used for the "full" rmb/wmb operations,
> as well as for smp_mb.
> 
> Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> ---
>  urcu/arch/ppc.h |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
> index a03d688..05f7db6 100644
> --- a/urcu/arch/ppc.h
> +++ b/urcu/arch/ppc.h
> @@ -32,7 +32,15 @@ extern "C" {
>  /* Include size of POWER5+ L3 cache lines: 256 bytes */
>  #define CAA_CACHE_LINE_SIZE	256
> 
> -#define cmm_mb()    asm volatile("sync":::"memory")
> +#define cmm_mb()         asm volatile("sync":::"memory")
> +
> +/* lwsync does not preserve ordering of cacheable vs. non-cacheable
> + * accesses, but it is good when MMIO is not in use.  An eieio+lwsync
> + * pair is also not enough for rmb, because it will order cacheable
> + * and non-cacheable memory operations separately---i.e. not the latter
> + * against the former.  */
> +#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
> +#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")

This works for recent Power hardware, and I see no reason to care about
stuff old enough to lack lwsync.  I must defer to others on embedded
PowerPC.

							Thanx, Paul

[ltt-dev] [PATCH v2] cmm: provide lightweight smp_rmb/smp_wmb on PPC

[Mathieu Desnoyers]

* Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> On Thu, Sep 22, 2011 at 10:42:52AM +0200, Paolo Bonzini wrote:
> > lwsync orders loads in cacheable memory with respect to other loads,
> > and stores in cacheable memory with respect to other stores.  Use it
> > to implement smp_rmb/smp_wmb.
> > 
> > The heavy-weight sync is still used for the "full" rmb/wmb operations,
> > as well as for smp_mb.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> > ---
> >  urcu/arch/ppc.h |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
> > index a03d688..05f7db6 100644
> > --- a/urcu/arch/ppc.h
> > +++ b/urcu/arch/ppc.h
> > @@ -32,7 +32,15 @@ extern "C" {
> >  /* Include size of POWER5+ L3 cache lines: 256 bytes */
> >  #define CAA_CACHE_LINE_SIZE	256
> > 
> > -#define cmm_mb()    asm volatile("sync":::"memory")
> > +#define cmm_mb()         asm volatile("sync":::"memory")
> > +
> > +/* lwsync does not preserve ordering of cacheable vs. non-cacheable
> > + * accesses, but it is good when MMIO is not in use.  An eieio+lwsync
> > + * pair is also not enough for rmb, because it will order cacheable
> > + * and non-cacheable memory operations separately---i.e. not the latter
> > + * against the former.  */
> > +#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
> > +#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")
> 
> This works for recent Power hardware, and I see no reason to care about
> stuff old enough to lack lwsync.  I must defer to others on embedded
> PowerPC.

commit e62b2f86c5ec06ed41d33ed578e66fad426ff215
Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
Date:   Thu Sep 22 11:00:14 2011 -0400

    powerpc: use __NO_LWSYNC__ check to use appropriate lwsync/sync opcode
    
    We already used it in uatomic code, move it to arch ppc.
    
    Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>

it's fixed now ;)

Thanks,

Mathieu


> 
> 							Thanx, Paul
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

[ltt-dev] [PATCH v2] cmm: provide lightweight smp_rmb/smp_wmb on PPC

[Paul E. McKenney]

On Thu, Sep 22, 2011 at 10:57:38AM -0400, Mathieu Desnoyers wrote:
> * Paul E. McKenney (paulmck at linux.vnet.ibm.com) wrote:
> > On Thu, Sep 22, 2011 at 10:42:52AM +0200, Paolo Bonzini wrote:
> > > lwsync orders loads in cacheable memory with respect to other loads,
> > > and stores in cacheable memory with respect to other stores.  Use it
> > > to implement smp_rmb/smp_wmb.
> > > 
> > > The heavy-weight sync is still used for the "full" rmb/wmb operations,
> > > as well as for smp_mb.
> > > 
> > > Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
> > > ---
> > >  urcu/arch/ppc.h |   10 +++++++++-
> > >  1 files changed, 9 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/urcu/arch/ppc.h b/urcu/arch/ppc.h
> > > index a03d688..05f7db6 100644
> > > --- a/urcu/arch/ppc.h
> > > +++ b/urcu/arch/ppc.h
> > > @@ -32,7 +32,15 @@ extern "C" {
> > >  /* Include size of POWER5+ L3 cache lines: 256 bytes */
> > >  #define CAA_CACHE_LINE_SIZE	256
> > > 
> > > -#define cmm_mb()    asm volatile("sync":::"memory")
> > > +#define cmm_mb()         asm volatile("sync":::"memory")
> > > +
> > > +/* lwsync does not preserve ordering of cacheable vs. non-cacheable
> > > + * accesses, but it is good when MMIO is not in use.  An eieio+lwsync
> > > + * pair is also not enough for rmb, because it will order cacheable
> > > + * and non-cacheable memory operations separately---i.e. not the latter
> > > + * against the former.  */
> > > +#define cmm_smp_rmb()    asm volatile("lwsync":::"memory")
> > > +#define cmm_smp_wmb()    asm volatile("lwsync":::"memory")
> > 
> > This works for recent Power hardware, and I see no reason to care about
> > stuff old enough to lack lwsync.  I must defer to others on embedded
> > PowerPC.
> 
> commit e62b2f86c5ec06ed41d33ed578e66fad426ff215
> Author: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> Date:   Thu Sep 22 11:00:14 2011 -0400
> 
>     powerpc: use __NO_LWSYNC__ check to use appropriate lwsync/sync opcode
>     
>     We already used it in uatomic code, move it to arch ppc.
>     
>     Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> 
> it's fixed now ;)

Very good!  ;-)

							Thanx, Paul

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > 							Thanx, Paul
> > 
> 
> -- 
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com