[dpdk-dev,1/2] eal:Introduce rte_dma_wmb/rte_dma_rmb.

Message ID BLU436-SMTP81994005EFD015E8B339BDBFAB0@phx.gbl (mailing list archive)
State Changes Requested, archived
Headers

Commit Message

WangDong June 28, 2015, 3:23 p.m. UTC
  These macro can be used to replace current PMD's compiler memory barrier (volatile varible) and rte_wmb.
In x86, they implement to compiler memory barrier.
In power, they implement to processor memory barrier.

---
 .../common/include/arch/ppc_64/rte_atomic.h        |  4 ++++
 .../common/include/arch/x86/rte_atomic.h           |  4 ++++
 lib/librte_eal/common/include/generic/rte_atomic.h | 25 ++++++++++++++++++++++
 3 files changed, 33 insertions(+)
  

Comments

Ananyev, Konstantin July 2, 2015, 3:51 p.m. UTC | #1
Hi Dong,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of WangDong
> Sent: Sunday, June 28, 2015 4:23 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 1/2] eal:Introduce rte_dma_wmb/rte_dma_rmb.
> 
> These macro can be used to replace current PMD's compiler memory barrier (volatile varible) and rte_wmb.
> In x86, they implement to compiler memory barrier.
> In power, they implement to processor memory barrier.
> 
> ---
>  .../common/include/arch/ppc_64/rte_atomic.h        |  4 ++++
>  .../common/include/arch/x86/rte_atomic.h           |  4 ++++
>  lib/librte_eal/common/include/generic/rte_atomic.h | 25 ++++++++++++++++++++++
>  3 files changed, 33 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> index fb7af2b..8f4129d 100644
> --- a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> @@ -72,6 +72,10 @@ extern "C" {
>   */
>  #define	rte_rmb() {asm volatile("sync" : : : "memory"); }
> 
> +#define rte_dma_wmb() {asm volatile("sync" : : : "memory"); }
> +
> +#define rte_dma_rmb() {asm volatile("sync" : : : "memory");

As a nit, probably better:

+#define rte_dma_wmb()  rte_rmb()
+#define rte_dma_rmb()    rte_wmb()

Here?

Konstantin

> +
>  /*------------------------- 16 bit atomic operations -------------------------*/
>  /* To be compatible with Power7, use GCC built-in functions for 16 bit
>   * operations */
> diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> index e93e8ee..7cfbe8f 100644
> --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> @@ -53,6 +53,10 @@ extern "C" {
> 
>  #define	rte_rmb() _mm_lfence()
> 
> +#define rte_dma_wmb() rte_compiler_barrier()
> +
> +#define rte_dma_rmb() rte_compiler_barrier()
> +
>  /*------------------------- 16 bit atomic operations -------------------------*/
> 
>  #ifndef RTE_FORCE_INTRINSICS
> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> index 6c7581a..a51eeee 100644
> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> @@ -72,6 +72,31 @@ static inline void rte_wmb(void);
>   */
>  static inline void rte_rmb(void);
> 
> +/**
> + * Write memory barrier for DMA.
> + *
> + * Be used in PMD, unlike rte_wmb() which use processor memory barrier,
> + * this memory barrier focus on performance, if compiler memory barrier
> + * is sufficient for guarantee memory ordering, this function will
> + * use compiler memory barrier.
> + *
> + * This function is architecture dependent.
> + */
> +static inline void rte_dma_wmb(void);
> +
> +/**
> + * Read memory barrier for DMA.
> + *
> + * Be used in PMD, unlike rte_rmb() which use processor memory barrier,
> + * this memory barrier focus on performance, if compiler memory barrier
> + * is sufficient for guarantee memory ordering, this function will
> + * use compiler memory barrier.
> + *
> + * This function is architecture dependent.
> + */
> +static inline void rte_dma_rmb(void);
> +
> +
>  #endif /* __DOXYGEN__ */
> 
>  /**
> --
> 2.1.0
  
Ananyev, Konstantin July 2, 2015, 4:51 p.m. UTC | #2
> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, July 02, 2015 4:52 PM
> To: 'WangDong'; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/2] eal:Introduce rte_dma_wmb/rte_dma_rmb.
> 
> Hi Dong,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of WangDong
> > Sent: Sunday, June 28, 2015 4:23 PM
> > To: dev@dpdk.org
> > Subject: [dpdk-dev] [PATCH 1/2] eal:Introduce rte_dma_wmb/rte_dma_rmb.
> >
> > These macro can be used to replace current PMD's compiler memory barrier (volatile varible) and rte_wmb.
> > In x86, they implement to compiler memory barrier.
> > In power, they implement to processor memory barrier.
> >
> > ---
> >  .../common/include/arch/ppc_64/rte_atomic.h        |  4 ++++
> >  .../common/include/arch/x86/rte_atomic.h           |  4 ++++
> >  lib/librte_eal/common/include/generic/rte_atomic.h | 25 ++++++++++++++++++++++
> >  3 files changed, 33 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> > index fb7af2b..8f4129d 100644
> > --- a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> > +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
> > @@ -72,6 +72,10 @@ extern "C" {
> >   */
> >  #define	rte_rmb() {asm volatile("sync" : : : "memory"); }
> >
> > +#define rte_dma_wmb() {asm volatile("sync" : : : "memory"); }
> > +
> > +#define rte_dma_rmb() {asm volatile("sync" : : : "memory");
> 
> As a nit, probably better:
> 
> +#define rte_dma_wmb()  rte_rmb()
> +#define rte_dma_rmb()    rte_wmb()
> 
> Here?
> 
> Konstantin

BTW, forgot to mention, git am threw a warning when I applied tha patch:

$ git am patchv1/5884
Applying: eal:Introduce rte_dma_wmb/rte_dma_rmb.
/local/kananye1/dpdk.org-asmb1/.git/rebase-apply/patch:17: trailing whitespace.
#define rte_dma_rmb() {asm volatile("sync" : : : "memory");
warning: 1 line adds whitespace errors.
 
Konstantin


> 
> > +
> >  /*------------------------- 16 bit atomic operations -------------------------*/
> >  /* To be compatible with Power7, use GCC built-in functions for 16 bit
> >   * operations */
> > diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > index e93e8ee..7cfbe8f 100644
> > --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
> > @@ -53,6 +53,10 @@ extern "C" {
> >
> >  #define	rte_rmb() _mm_lfence()
> >
> > +#define rte_dma_wmb() rte_compiler_barrier()
> > +
> > +#define rte_dma_rmb() rte_compiler_barrier()
> > +
> >  /*------------------------- 16 bit atomic operations -------------------------*/
> >
> >  #ifndef RTE_FORCE_INTRINSICS
> > diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> > index 6c7581a..a51eeee 100644
> > --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> > +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> > @@ -72,6 +72,31 @@ static inline void rte_wmb(void);
> >   */
> >  static inline void rte_rmb(void);
> >
> > +/**
> > + * Write memory barrier for DMA.
> > + *
> > + * Be used in PMD, unlike rte_wmb() which use processor memory barrier,
> > + * this memory barrier focus on performance, if compiler memory barrier
> > + * is sufficient for guarantee memory ordering, this function will
> > + * use compiler memory barrier.
> > + *
> > + * This function is architecture dependent.
> > + */
> > +static inline void rte_dma_wmb(void);
> > +
> > +/**
> > + * Read memory barrier for DMA.
> > + *
> > + * Be used in PMD, unlike rte_rmb() which use processor memory barrier,
> > + * this memory barrier focus on performance, if compiler memory barrier
> > + * is sufficient for guarantee memory ordering, this function will
> > + * use compiler memory barrier.
> > + *
> > + * This function is architecture dependent.
> > + */
> > +static inline void rte_dma_rmb(void);
> > +
> > +
> >  #endif /* __DOXYGEN__ */
> >
> >  /**
> > --
> > 2.1.0
  
WangDong July 6, 2015, 3:48 p.m. UTC | #3
Hi Konstantin,
Yes, it's better to use rte_rmb/rte_wmb instead of their source code.
Actually, the warning is a bug, it lose a "}" of rte_dma_rmb(), I didn't 
test it on PowerPC, so the bug didn't be found out before I submit the 
patch, sorry.

I'll resubmit this patch.

Dong

> Hi Dong,
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of WangDong
>> Sent: Sunday, June 28, 2015 4:23 PM
>> To: dev@dpdk.org
>> Subject: [dpdk-dev] [PATCH 1/2] eal:Introduce rte_dma_wmb/rte_dma_rmb.
>>
>> These macro can be used to replace current PMD's compiler memory barrier (volatile varible) and rte_wmb.
>> In x86, they implement to compiler memory barrier.
>> In power, they implement to processor memory barrier.
>>
>> ---
>>   .../common/include/arch/ppc_64/rte_atomic.h        |  4 ++++
>>   .../common/include/arch/x86/rte_atomic.h           |  4 ++++
>>   lib/librte_eal/common/include/generic/rte_atomic.h | 25 ++++++++++++++++++++++
>>   3 files changed, 33 insertions(+)
>>
>> diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
>> index fb7af2b..8f4129d 100644
>> --- a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
>> +++ b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
>> @@ -72,6 +72,10 @@ extern "C" {
>>    */
>>   #define	rte_rmb() {asm volatile("sync" : : : "memory"); }
>>
>> +#define rte_dma_wmb() {asm volatile("sync" : : : "memory"); }
>> +
>> +#define rte_dma_rmb() {asm volatile("sync" : : : "memory");
>
> As a nit, probably better:
>
> +#define rte_dma_wmb()  rte_rmb()
> +#define rte_dma_rmb()    rte_wmb()
>
> Here?
>
> Konstantin
>
>> +
>>   /*------------------------- 16 bit atomic operations -------------------------*/
>>   /* To be compatible with Power7, use GCC built-in functions for 16 bit
>>    * operations */
>> diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
>> index e93e8ee..7cfbe8f 100644
>> --- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
>> +++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
>> @@ -53,6 +53,10 @@ extern "C" {
>>
>>   #define	rte_rmb() _mm_lfence()
>>
>> +#define rte_dma_wmb() rte_compiler_barrier()
>> +
>> +#define rte_dma_rmb() rte_compiler_barrier()
>> +
>>   /*------------------------- 16 bit atomic operations -------------------------*/
>>
>>   #ifndef RTE_FORCE_INTRINSICS
>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
>> index 6c7581a..a51eeee 100644
>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
>> @@ -72,6 +72,31 @@ static inline void rte_wmb(void);
>>    */
>>   static inline void rte_rmb(void);
>>
>> +/**
>> + * Write memory barrier for DMA.
>> + *
>> + * Be used in PMD, unlike rte_wmb() which use processor memory barrier,
>> + * this memory barrier focus on performance, if compiler memory barrier
>> + * is sufficient for guarantee memory ordering, this function will
>> + * use compiler memory barrier.
>> + *
>> + * This function is architecture dependent.
>> + */
>> +static inline void rte_dma_wmb(void);
>> +
>> +/**
>> + * Read memory barrier for DMA.
>> + *
>> + * Be used in PMD, unlike rte_rmb() which use processor memory barrier,
>> + * this memory barrier focus on performance, if compiler memory barrier
>> + * is sufficient for guarantee memory ordering, this function will
>> + * use compiler memory barrier.
>> + *
>> + * This function is architecture dependent.
>> + */
>> +static inline void rte_dma_rmb(void);
>> +
>> +
>>   #endif /* __DOXYGEN__ */
>>
>>   /**
>> --
>> 2.1.0
>
  

Patch

diff --git a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
index fb7af2b..8f4129d 100644
--- a/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/ppc_64/rte_atomic.h
@@ -72,6 +72,10 @@  extern "C" {
  */
 #define	rte_rmb() {asm volatile("sync" : : : "memory"); }
 
+#define rte_dma_wmb() {asm volatile("sync" : : : "memory"); }
+
+#define rte_dma_rmb() {asm volatile("sync" : : : "memory"); 
+
 /*------------------------- 16 bit atomic operations -------------------------*/
 /* To be compatible with Power7, use GCC built-in functions for 16 bit
  * operations */
diff --git a/lib/librte_eal/common/include/arch/x86/rte_atomic.h b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
index e93e8ee..7cfbe8f 100644
--- a/lib/librte_eal/common/include/arch/x86/rte_atomic.h
+++ b/lib/librte_eal/common/include/arch/x86/rte_atomic.h
@@ -53,6 +53,10 @@  extern "C" {
 
 #define	rte_rmb() _mm_lfence()
 
+#define rte_dma_wmb() rte_compiler_barrier()
+
+#define rte_dma_rmb() rte_compiler_barrier()
+
 /*------------------------- 16 bit atomic operations -------------------------*/
 
 #ifndef RTE_FORCE_INTRINSICS
diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
index 6c7581a..a51eeee 100644
--- a/lib/librte_eal/common/include/generic/rte_atomic.h
+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
@@ -72,6 +72,31 @@  static inline void rte_wmb(void);
  */
 static inline void rte_rmb(void);
 
+/**
+ * Write memory barrier for DMA.
+ *
+ * Be used in PMD, unlike rte_wmb() which use processor memory barrier,
+ * this memory barrier focus on performance, if compiler memory barrier
+ * is sufficient for guarantee memory ordering, this function will
+ * use compiler memory barrier.
+ *
+ * This function is architecture dependent.
+ */
+static inline void rte_dma_wmb(void);
+
+/**
+ * Read memory barrier for DMA.
+ *
+ * Be used in PMD, unlike rte_rmb() which use processor memory barrier,
+ * this memory barrier focus on performance, if compiler memory barrier
+ * is sufficient for guarantee memory ordering, this function will
+ * use compiler memory barrier.
+ *
+ * This function is architecture dependent.
+ */
+static inline void rte_dma_rmb(void);
+
+
 #endif /* __DOXYGEN__ */
 
 /**