Hi Jia, > -----Original Message----- > From: Jia He [mailto:hejia...@gmail.com] > Sent: Friday, November 10, 2017 2:06 AM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; Jianbo Liu > <jianbo....@arm.com> > Cc: Richardson, Bruce <bruce.richard...@intel.com>; > jerin.ja...@caviumnetworks.com; dev@dpdk.org; olivier.m...@6wind.com; > hemant.agra...@nxp.com; jia...@hxt-semitech.com > Subject: Re: [PATCH 1/3] eal/arm64: remove the braces {} for dmb(),dsb() > > > > 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")
Fine by me. Konstantin > Another choice is adding the do/while. > > @Ananyev @Jianbo > Any thoughts? > > -- > Cheers, > Jia
