From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jianbo Liu Subject: Re: [PATCH 1/3] eal/arm64: remove the braces {} for dmb(), dsb() Date: Fri, 10 Nov 2017 11:09:51 +0800 Message-ID: <20171110030950.GC27104@arm.com> References: <1510121832-16439-1-git-send-email-hejianet@gmail.com> <20171108102814.GA7552@bricha3-MOBL3.ger.corp.intel.com> <9086316b-c16b-c42b-2d85-9b01fa2f66e1@gmail.com> <028263d0-44de-bd0c-c495-081588a0ad20@gmail.com> <20171109032145.GA26939@arm.com> <20171109045601.GA27104@arm.com> <2601191342CEEE43887BDE71AB9772585FABB311@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: "Ananyev, Konstantin" , "Richardson, Bruce" , "jerin.jacob@caviumnetworks.com" , "dev@dpdk.org" , "olivier.matz@6wind.com" , "hemant.agrawal@nxp.com" , "jia.he@hxt-semitech.com" To: Jia He Return-path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0057.outbound.protection.outlook.com [104.47.1.57]) by dpdk.org (Postfix) with ESMTP id EBD1E1B62C for ; Fri, 10 Nov 2017 04:11:04 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" The 11/10/2017 10:06, Jia He wrote: > > > On 11/9/2017 5:38 PM, Ananyev, Konstantin Wrote: > > > >>-----Original Message----- > >>From: Jianbo Liu [mailto:jianbo.liu@arm.com] > >>Sent: Thursday, November 9, 2017 4:56 AM > >>To: Jia He > >>Cc: Richardson, Bruce ; jerin.jacob@caviumn= etworks.com; dev@dpdk.org; olivier.matz@6wind.com; > >>Ananyev, Konstantin ; hemant.agrawal@nxp.= com; jia.he@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 > >>>>>>>>Signed-off-by: jia.he@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? > I think it's a good to keep your v4. Jianbo IMPORTANT NOTICE: The contents of this email and any attachments are confid= ential and may also be privileged. If you are not the intended recipient, p= lease notify the sender immediately and do not disclose the contents to any= other person, use it for any purpose, or store or copy the information in = any medium. Thank you.