[dpdk-dev] [PATCH v13 2/5] eal: add the APIs to wait until equal

[Ananyev, Konstantin]

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Friday, November 8, 2019 5:00 PM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Cc: David Marchand <david.marchand at redhat.com>; dev at dpdk.org; nd at arm.com; Gavin Hu <gavin.hu at arm.com>; Mcnamara, John
> <john.mcnamara at intel.com>; Kovacevic, Marko <marko.kovacevic at intel.com>; Jerin Jacob <jerinj at marvell.com>; Jan Viktorin
> <viktorin at rehivetech.com>
> Subject: Re: [PATCH v13 2/5] eal: add the APIs to wait until equal
> 
> 08/11/2019 17:38, Ananyev, Konstantin:
> > > From: Gavin Hu <gavin.hu at arm.com>
> > > +static __rte_always_inline void
> > > +rte_wait_until_equal_64(volatile uint64_t *addr, uint64_t expected,
> > > +		int memorder)
> > > +{
> > > +	uint64_t value;
> > > +
> > > +	assert(memorder == __ATOMIC_ACQUIRE || memorder == __ATOMIC_RELAXED);
> > > +
> > > +	/*
> > > +	 * Atomic exclusive load from addr, it returns the 64-bit content of
> > > +	 * *addr while making it 'monitored',when it is written by someone
> > > +	 * else, the 'monitored' state is cleared and a event is generated
> > > +	 * implicitly to exit WFE.
> > > +	 */
> > > +#define __LOAD_EXC_64(src, dst, memorder) {              \
> > > +	if (memorder == __ATOMIC_RELAXED) {              \
> > > +		asm volatile("ldxr %x[tmp], [%x[addr]]"  \
> > > +			: [tmp] "=&r" (dst)              \
> > > +			: [addr] "r"(src)                \
> > > +			: "memory");                     \
> > > +	} else {                                         \
> > > +		asm volatile("ldaxr %x[tmp], [%x[addr]]" \
> > > +			: [tmp] "=&r" (dst)              \
> > > +			: [addr] "r"(src)                \
> > > +			: "memory");                     \
> > > +	} }
> > > +
> > > +	__LOAD_EXC_64(addr, value, memorder)
> > > +	if (value != expected) {
> > > +		__SEVL()
> > > +		do {
> > > +			__WFE()
> > > +			__LOAD_EXC_64(addr, value, memorder)
> > > +		} while (value != expected);
> > > +	}
> > > +}
> > > +#undef __LOAD_EXC_64
> > > +
> > > +#undef __SEVL
> > > +#undef __WFE
> >
> > Personally I don't see how these define/undef are better then inline functions...
> 
> The benefit it to not pollute the API namespace
> with some functions which are used only locally.
> After using a macro, it disappears with #undef.
> 
> > Again I suppose they might be re-used in future some other ARM specific low-level primitvies.
> 
> Do this low-level routines _LOAD_EXC_ need to be exposed in EAL for re-use?

About load_exc don't know for sure...
Though as I can see wfe/sevl are used here:
http://patchwork.dpdk.org/patch/61779/
Might be it is possible to reuse these functions/macros...
But again, I am not arm expert, would be interested to know what arm guys think.

> 
> > My preference would be to keep them as inline function - much cleaner code.
> > But as I don't develop for/use, I wouldn't insist and let you and arm guys to decide.
> 
>