Re: [dpdk-dev] [EXT] [PATCH v3 3/3] eal/stack: enable lock-free stack for aarch64

["Phil Yang (Arm Technology China)" <Phil.Yang@arm.com>]

> -----Original Message-----
> From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> Sent: Friday, July 19, 2019 1:03 PM
> To: Eads, Gage <gage.eads@intel.com>; Phil Yang (Arm Technology China)
> <Phil.Yang@arm.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Gavin Hu (Arm Technology
> China) <Gavin.Hu@arm.com>; nd <nd@arm.com>
> Subject: RE: [EXT] [PATCH v3 3/3] eal/stack: enable lock-free stack for
> aarch64
> 
> > > Can we remove this all #ifdef clutter by adding the following
> > >
> > > $ git diff
> > > diff --git a/lib/librte_stack/rte_stack_lf.h
> > > b/lib/librte_stack/rte_stack_lf.h index f5581f0c2..46af08b83 100644
> > > --- a/lib/librte_stack/rte_stack_lf.h
> > > +++ b/lib/librte_stack/rte_stack_lf.h
> > > @@ -5,7 +5,7 @@
> > >  #ifndef _RTE_STACK_LF_H_
> > >  #define _RTE_STACK_LF_H_
> > >
> > > -#ifdef RTE_USE_C11_MEM_MODEL
> > > +#if defined (RTE_USE_C11_MEM_MODEL) &&
> > defined(RTE_ARCH_X86_64)
> > > &&
> > > +defined(RTE_ARCH_ARM64)
> >
> > I assume you meant (defined(RTE_ARCH_X86_64) ||
> > defined(RTE_ARCH_ARM64))?
> 
> Yup.
> 
> >
> > >  #include "rte_stack_lf_c11.h"
> > >  #else
> > >  #include "rte_stack_lf_generic.h"
> > >
> > >
> >
> > The ifdefs in those two headers prevent DPDK from trying to build
> > rte_atomic128_cmp_exchange() on architectures that don't implement it.
> So
> > the proposal wouldn't quite work, since rte_stack_lf_generic.h calls
> > rte_atomic128_cmp_exchange().
> >
> > Something like this could work:
> >
> > #if !(defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_ARM64)) #include
> > rte_stack_lf_stubs.h #else #ifdef RTE_USE_C11_MEM_MODEL #include
> > "rte_stack_lf_c11.h"
> > #else
> > #include "rte_stack_lf_generic.h"
> > #endif
> > #endif
> >
> > Where rte_stack_lf_stubs.h is a new header containing stub
> > implementations of __rte_stack_lf_count, __rte_stack_lf_push_elems,
> and
> > __rte_stack_lf_pop_elems. It still has some ifdef clutter, but less overall.
> 
> Agree. I prefer to take this route to reduce the ifdef clutter across generic
> and c11 files.

Got it. I will update it in the next version.

Thanks,
Phil Yang