[dpdk-dev] [RFC 1/5] eal: add the APIs to wait until equal

Gavin Hu (Arm Technology China) Gavin.Hu at arm.com
Mon Jul 1 09:16:39 CEST 2019


Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Monday, July 1, 2019 4:28 AM
> To: Gavin Hu (Arm Technology China) <Gavin.Hu at arm.com>
> Cc: dev at dpdk.org; thomas at monjalon.net; jerinj at marvell.com;
> hemant.agrawal at nxp.com; bruce.richardson at intel.com;
> chaozhu at linux.vnet.ibm.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli at arm.com>; nd <nd at arm.com>
> Subject: Re: [dpdk-dev] [RFC 1/5] eal: add the APIs to wait until equal
> 
> On Mon,  1 Jul 2019 00:21:12 +0800
> Gavin Hu <gavin.hu at arm.com> wrote:
> 
> > +#ifdef RTE_USE_WFE
> > +#define rte_wait_until_equal_relaxed(addr, expected) do {\
> > +		typeof(*addr) tmp;  \
> > +		if (__builtin_constant_p((expected))) \
> > +			do { \
> > +				if (sizeof(*(addr)) == 16)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxrh  %w0, %1\n"  \
> > +						"cmp	%w0, %w2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r"(tmp)  \
> > +						: "Q"(*addr), "i"(expected)  \
> > +						: "cc", "memory");  \
> > +				else if (sizeof(*(addr)) == 32)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxr  %w0, %1\n"  \
> > +						"cmp	%w0, %w2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r"(tmp)  \
> > +						: "Q"(*addr), "i"(expected)  \
> > +						: "cc", "memory");  \
> > +				else if (sizeof(*(addr)) == 64)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxr  %x0, %1\n"  \
> > +						"cmp	%x0, %x2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r" (tmp)  \
> > +						: "Q"(*addr), "i"(expected)  \
> > +						: "cc", "memory"); \
> > +			} while (0); \
> > +		else \
> > +			do { \
> > +				if (sizeof(*(addr)) == 16)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxrh  %w0, %1\n"  \
> > +						"cmp	%w0, %w2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r"(tmp)  \
> > +						: "Q"(*addr), "r"(expected)  \
> > +						: "cc", "memory");  \
> > +				else if (sizeof(*(addr)) == 32)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxr  %w0, %1\n"  \
> > +						"cmp	%w0, %w2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r"(tmp)  \
> > +						: "Q"(*addr), "r"(expected)  \
> > +						: "cc", "memory");  \
> > +				else if (sizeof(*(addr)) == 64)\
> > +					asm volatile(  \
> > +						"sevl\n"  \
> > +						"1:	 wfe\n"  \
> > +						"ldxr  %x0, %1\n"  \
> > +						"cmp	%x0, %x2\n"  \
> > +						"bne	1b\n"  \
> > +						: "=&r" (tmp)  \
> > +						: "Q"(*addr), "r"(expected)  \
> > +						: "cc", "memory");  \
> > +		} while (0); \
> > +} while (0)
> 
> That is a hot mess.
> Macro's are harder to maintain and offer no benefit over inline functions.
During our internal review, I ever used C11 _Generic to generalize the API to take different types of arguments. 
That makes the API look much simpler and better, but it poses a hard requirement for C11 and gcc 4.9+.
That means, Gaetan's patch, as shown below, has to be reverted, otherwise there are compiling errors.
https://gcc.gnu.org/wiki/C11Status 
$ git show ea7726a6
commit ea7726a6ee4b2b63313c4a198522a8dcea70c13d
Author: Gaetan Rivet <gaetan.rivet at 6wind.com>
Date:   Thu Jul 20 14:27:53 2017 +0200

    net/failsafe: fix build on FreeBSD 10 with GCC 4.8

diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile
index 32aaaa2..d516d36 100644
--- a/drivers/net/failsafe/Makefile
+++ b/drivers/net/failsafe/Makefile
@@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_PMD_FAILSAFE) += failsafe_flow.c
 # No exported include files

 # Basic CFLAGS:
-CFLAGS += -std=c11 -Wextra
+CFLAGS += -std=gnu99 -Wextra


More information about the dev mailing list