On 11/9/2017 5:38 PM, Ananyev, Konstantin Wrote:
-----Original Message-----
From: Jianbo Liu [mailto:jianbo....@arm.com]
Sent: Thursday, November 9, 2017 4:56 AM
To: Jia He <hejia...@gmail.com>
Cc: Richardson, Bruce <bruce.richard...@intel.com>;
jerin.ja...@caviumnetworks.com; dev@dpdk.org; olivier.m...@6wind.com;
Ananyev, Konstantin <konstantin.anan...@intel.com>; hemant.agra...@nxp.com;
jia...@hxt-semitech.com
Subject: Re: [PATCH 1/3] eal/arm64: remove the braces {} for dmb(),dsb()
The 11/09/2017 12:43, Jia He wrote:
Hi Jianbo
On 11/9/2017 11:21 AM, Jianbo Liu Wrote:
The 11/09/2017 11:14, Jia He wrote:
On 11/9/2017 9:22 AM, Jia He Wrote:
Hi Bruce
On 11/8/2017 6:28 PM, Bruce Richardson Wrote:
On Wed, Nov 08, 2017 at 06:17:10AM +0000, Jia He wrote:
for the code as follows:
if (condition)
rte_smp_rmb();
else
rte_smp_wmb();
Without this patch, compiler will report this error:
error: 'else' without a previous 'if'
Signed-off-by: Jia He <hejia...@gmail.com>
Signed-off-by: jia...@hxt-semitech.com
---
lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git
a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
index 0b70d62..38c3393 100644
--- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
+++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h
@@ -43,8 +43,8 @@ extern "C" {
#include "generic/rte_atomic.h"
-#define dsb(opt) { asm volatile("dsb " #opt : : : "memory"); }
-#define dmb(opt) { asm volatile("dmb " #opt : : : "memory"); }
+#define dsb(opt) asm volatile("dsb " #opt : : : "memory");
+#define dmb(opt) asm volatile("dmb " #opt : : : "memory");
Need to remove the trailing ";" I too I think.
Alternatively, to keep the braces, the standard practice is to use
do { ... } while(0)
If trailing ";" is not removed
the code:
if (condition)
rte_smp_rmb();
else
anything();
Sorry, why not use two different functions as your conditions passed in
are fixed in the calling functions.
Do you mean to split update_tail() into update_tail_enqueue() and
update_tail_dequeue()?
Yes. So it's not need to change dsb/dmb.
That's a good idea - but you still might hit the same problem in
Some different place in future...
Why not to convert these macros into 'always_inline' functions then?
Konstantin
It makes things more complex
opt needs to be redefined with types
such as : __attribute__((always_inline)) void dsb( char* opt)
and the input paramenter shoud be
#define sy "sy"
#define ld "ld"
And the "#" in asm codes needs to be considerred more.
IMO, the kernel way is simple and clean, isn't it?
#define dmb(opt) asm volatile("dmb " #opt : : : "memory")
Another choice is adding the do/while.
@Ananyev @Jianbo
Any thoughts?
--
Cheers,
Jia
