From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jia He <hejianet@gmail.com>
Subject: Re: [PATCH 1/3] eal/arm64: remove the braces {} for
	dmb(), dsb()
Date: Thu, 9 Nov 2017 09:22:10 +0800
Message-ID: <9086316b-c16b-c42b-2d85-9b01fa2f66e1@gmail.com>
References: <1510121832-16439-1-git-send-email-hejianet@gmail.com>
 <20171108102814.GA7552@bricha3-MOBL3.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Cc: jerin.jacob@caviumnetworks.com, dev@dpdk.org, olivier.matz@6wind.com,
 konstantin.ananyev@intel.com, jianbo.liu@arm.com, hemant.agrawal@nxp.com,
 jia.he@hxt-semitech.com
To: Bruce Richardson <bruce.richardson@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pg0-f65.google.com (mail-pg0-f65.google.com [74.125.83.65])
 by dpdk.org (Postfix) with ESMTP id 998AD1B282
 for <dev@dpdk.org>; Thu,  9 Nov 2017 02:22:20 +0100 (CET)
Received: by mail-pg0-f65.google.com with SMTP id o7so3374792pgc.4
 for <dev@dpdk.org>; Wed, 08 Nov 2017 17:22:20 -0800 (PST)
In-Reply-To: <20171108102814.GA7552@bricha3-MOBL3.ger.corp.intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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 <hejianet@gmail.com>
>> 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();

will be like below after precompiling:
if (condition)
     asm volatile("dsb " "ld" : : : "memory");;
else
     anything();
Then, the same error - error: 'else' without a previous 'if'

If you choose do/while(0), yes, no errors from compiler.
But the checkpatch will report

WARNING: Single statement macros should not use a do {} while (0) loop
#11: FILE: lib/librte_eal/common/include/arch/arm/rte_atomic_64.h:46:
+#define dsb(opt) do { asm volatile("dsb " #opt : : : "memory"); } while (0)

I searched the kernel codes, the marco dsb() didn't use the do/while(0).
Which one do you think is better for dpdk?

Cheers,
Jia