[dpdk-dev] [PATCH v5 1/4] lib/rcu: add resource reclamation APIs

[Ananyev, Konstantin]

> 
> On Wed, Apr 22, 2020 at 10:42 AM David Marchand
> <david.marchand at redhat.com> wrote:
> >
> > On Wed, Apr 22, 2020 at 10:37 AM Ananyev, Konstantin
> > <konstantin.ananyev at intel.com> wrote:
> > >
> > > > Add resource reclamation APIs to make it simple for applications
> > > > and libraries to integrate rte_rcu library.
> > > >
> > > > Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> > > > Reviewed-by: Ola Liljedhal <ola.liljedhal at arm.com>
> > > > Reviewed-by: Ruifeng Wang <ruifeng.wang at arm.com>
> > > > Acked-by: Konstantin Ananyev <konstantin.ananyev at intel.com>
> > > > ---
> > > >  lib/librte_rcu/Makefile            |   2 +-
> > > >  lib/librte_rcu/meson.build         |   7 +
> > > >  lib/librte_rcu/rcu_qsbr_pvt.h      |  66 +++++++++
> > > >  lib/librte_rcu/rte_rcu_qsbr.c      | 227 ++++++++++++++++++++++++++++-
> > > >  lib/librte_rcu/rte_rcu_qsbr.h      | 194 +++++++++++++++++++++++-
> > > >  lib/librte_rcu/rte_rcu_version.map |   4 +
> > > >  lib/meson.build                    |   6 +-
> > > >  7 files changed, 501 insertions(+), 5 deletions(-)
> > > >  create mode 100644 lib/librte_rcu/rcu_qsbr_pvt.h
> > > >
> > > > diff --git a/lib/librte_rcu/Makefile b/lib/librte_rcu/Makefile
> > > > index 728669975..553bca2ef 100644
> > > > --- a/lib/librte_rcu/Makefile
> > > > +++ b/lib/librte_rcu/Makefile
> > > > @@ -7,7 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
> > > >  LIB = librte_rcu.a
> > > >
> > > >  CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
> > > > -LDLIBS += -lrte_eal
> > > > +LDLIBS += -lrte_eal -lrte_ring
> > > >
> > > >  EXPORT_MAP := rte_rcu_version.map
> > > >
> > > > diff --git a/lib/librte_rcu/meson.build b/lib/librte_rcu/meson.build
> > > > index c009ae4b7..3eb2ace17 100644
> > > > --- a/lib/librte_rcu/meson.build
> > > > +++ b/lib/librte_rcu/meson.build
> > > > @@ -3,3 +3,10 @@
> > > >
> > > >  sources = files('rte_rcu_qsbr.c')
> > > >  headers = files('rte_rcu_qsbr.h')
> > > > +
> > > > +# for clang 32-bit compiles we need libatomic for 64-bit atomic ops
> > > > +if cc.get_id() == 'clang' and dpdk_conf.get('RTE_ARCH_64') == false
> > > > +     ext_deps += cc.find_library('atomic')
> > > > +endif
> > > > +
> > >
> > > As a nit - as Pavan patch is already integrated into mainline,
> > > this is not necessary any more, I think.
> >
> > I can handle this.
> >
> >
> > > Also noticed that most of make builds failed due to dependency problem:
> > > http://mails.dpdk.org/archives/test-report/2020-April/127765.html
> > > I can't reproduce it locally, but my guess that we need to move rcu above
> > > ring in this mk file: mk/rte.app.mk
> > > Probably something like that:
> > > diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> > > index da12b9eec..8e5d023de 100644
> > > --- a/mk/rte.app.mk
> > > +++ b/mk/rte.app.mk
> > > @@ -91,13 +91,13 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_MEMPOOL)        += -lrte_mempool
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_STACK)          += -lrte_stack
> > >  _LDLIBS-$(CONFIG_RTE_DRIVER_MEMPOOL_RING)   += -lrte_mempool_ring
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_OCTEONTX2_MEMPOOL) += -lrte_mempool_octeontx2
> > > +_LDLIBS-$(CONFIG_RTE_LIBRTE_RCU)            += -lrte_rcu
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_RING)           += -lrte_ring
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_PCI)            += -lrte_pci
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lrte_eal
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)        += -lrte_cmdline
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
> > >  _LDLIBS-$(CONFIG_RTE_LIBRTE_SCHED)          += -lrte_sched
> > > -_LDLIBS-$(CONFIG_RTE_LIBRTE_RCU)            += -lrte_rcu
> >
> > No, just moving will not express a dependency.
> 
> Fixed with:
> 
> diff --git a/lib/Makefile b/lib/Makefile
> index 2cbb096f1..8bc0c2e4a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -118,6 +118,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TELEMETRY) += librte_telemetry
>  DEPDIRS-librte_telemetry := librte_eal librte_metrics librte_ethdev
>  DIRS-$(CONFIG_RTE_LIBRTE_RCU) += librte_rcu
> -DEPDIRS-librte_rcu := librte_eal
> +DEPDIRS-librte_rcu := librte_eal librte_ring

Right, totally forgot about that part.

> 
>  ifeq ($(CONFIG_RTE_EXEC_ENV_LINUX),y)
>  DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> diff --git a/lib/librte_rcu/meson.build b/lib/librte_rcu/meson.build
> index 3eb2ace17..09abc5204 100644
> --- a/lib/librte_rcu/meson.build
> +++ b/lib/librte_rcu/meson.build
> @@ -4,9 +4,4 @@
>  sources = files('rte_rcu_qsbr.c')
>  headers = files('rte_rcu_qsbr.h')
> 
> -# for clang 32-bit compiles we need libatomic for 64-bit atomic ops
> -if cc.get_id() == 'clang' and dpdk_conf.get('RTE_ARCH_64') == false
> -       ext_deps += cc.find_library('atomic')
> -endif
> -
>  deps += ['ring']
> 
> 
> --
> David Marchand