From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f66.google.com (mail-pg0-f66.google.com [74.125.83.66]) by dpdk.org (Postfix) with ESMTP id 4DE481B62C for ; Fri, 10 Nov 2017 03:06:23 +0100 (CET) Received: by mail-pg0-f66.google.com with SMTP id 207so3459418pgc.12 for ; Thu, 09 Nov 2017 18:06:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=NnlqwucJ1cF3BuEKNqbDp6hCIjUkS9fAOnu/LNOgaow=; b=FDeD3cO3tiZM3Cp3ePsshjOifDAT26DdWhDZ0Th3jCB5TynyIAWEOUE4w58i6vo0oH NWzQcJYLpYAJ8CevKVOSIEESuwzCI8edQKzZAVet3jPpP5NObHYFZCh4ndNSzOjbmUHP H9uVlwzOi+DVd0HYeSkSlMJVVtkAWlP5WwS+PtG0hh3TsDIQqyeNzCTNxpLAlrFDfWoE tOl3JlUkhkcIZUbLJY7h5Mq0o5+y1DDf8ZQFO3JAgU0Ogl6zpGo4C8PgzvwfK95BWqrH BxXFpDtIyUJV65h69dXTNFpMg52wtwhE6PP6RoqPMnUoUazgMZsPxWMCzMU+rQYdR6jO PAyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=NnlqwucJ1cF3BuEKNqbDp6hCIjUkS9fAOnu/LNOgaow=; b=r9Z0DPcRYI3RYopVkG+8U2gs7Ob1YdfUk3UWNZP5QDD2/dJeU3doKxb90w+zrTPo4V UKF/U+eUsq5EE39mt4A1Ri7Wf30iypn4FrKmP7amzSkEVcnjt2qEjbvgXn+QGk8Au4CS ydcBtpcA/vlJXJ9SIpqmZPuemIVMv3GRvLfM8DLm4wUzWV5VAcuuG7pvVDNV7JY8+HDQ CBtDDNe7BPUYvC22/Hs7oDRbfmZp5A2cA0nmnth8+iqZpMb+f7WGd6P7bgFeFzwoteok Xy9qSsJs+jAAuDYWsufDCYUGeJ3ofMHZSC02XsOQacrs2N1KX9B6xBjlNC9Z6lgPKc3A 56mQ== X-Gm-Message-State: AJaThX7k70bvB5xPzi/D+hty2ELdldXkoLY+p6A43oos9cetcaAsykYu nCLBLEC+BzwsdRfpPvaca+8= X-Google-Smtp-Source: ABhQp+TTLZ73S15hbRf+UhxrC2Ok18d6etNF6/L0QTHxA9ZGNKCSlp25vPZeC+2jwxoZADWv4P4Bhg== X-Received: by 10.159.207.134 with SMTP id z6mr2363831plo.272.1510279582623; Thu, 09 Nov 2017 18:06:22 -0800 (PST) Received: from [0.0.0.0] (67.209.179.165.16clouds.com. [67.209.179.165]) by smtp.gmail.com with ESMTPSA id x185sm15984055pfd.116.2017.11.09.18.06.17 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Nov 2017 18:06:22 -0800 (PST) To: "Ananyev, Konstantin" , Jianbo Liu Cc: "Richardson, Bruce" , "jerin.jacob@caviumnetworks.com" , "dev@dpdk.org" , "olivier.matz@6wind.com" , "hemant.agrawal@nxp.com" , "jia.he@hxt-semitech.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> From: Jia He Message-ID: Date: Fri, 10 Nov 2017 10:06:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <2601191342CEEE43887BDE71AB9772585FABB311@irsmsx105.ger.corp.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/3] eal/arm64: remove the braces {} for dmb(), dsb() X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 10 Nov 2017 02:06:23 -0000 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@caviumnetworks.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? -- Cheers, Jia