Re: [dpdk-dev] [PATCH v3 5/5] config: add WFE config entry for aarch64

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

> 
> On Tue, 23 Jul 2019 23:43:46 +0800
> Gavin Hu <gavin.hu@arm.com> wrote:
> 
> > Add the RTE_USE_WFE configuration entry for aarch64, disabled by default.
> > It can be enabled selectively based on the performance benchmarking.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> >  config/arm/meson.build     | 1 +
> >  config/common_armv8a_linux | 6 ++++++
> >  2 files changed, 7 insertions(+)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 979018e..496813a 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -116,6 +116,7 @@ impl_dpaa = ['NXP DPAA', flags_dpaa,
> > machine_args_generic]
> >  impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]
> >
> >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > +dpdk_conf.set('RTE_USE_WFE', 0)
> >
> >  if not dpdk_conf.get('RTE_ARCH_64')
> >  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64) diff --git
> > a/config/common_armv8a_linux b/config/common_armv8a_linux index
> > 481712e..48c7ab5 100644
> > --- a/config/common_armv8a_linux
> > +++ b/config/common_armv8a_linux
> > @@ -12,6 +12,12 @@ CONFIG_RTE_ARCH_64=y
> >
> >  CONFIG_RTE_FORCE_INTRINSICS=y
> >
> > +# Use WFE instructions to implement the rte_wait_for_equal_xxx APIs,
> > +# calling these APIs put the cores enter low power state while
> > +waiting # for the memory address to be become equal to the expected value.
> > +# This is supported only by aarch64.
> > +CONFIG_RTE_USE_WFE=n
> > +
> >  # Maximum available cache line size in arm64 implementations.
> >  # Setting to maximum available cache line size in generic config  #
> > to address minimum DMA alignment across all arm64 implementations.
> 
> Introducing config options is a maintenance nightmare.
> How are distributions supposed to ship a package?
> Does full regression test get done on both options?
> 
> The user should not be able to change this.
Agree with these concerns here. In our tests, we are finding that this patch does not result in performance improvements on all micro-architectures. May be these micro-architectures will evolve in the future knowing that WFE is being used in DPDK. But at this point, it does not make sense to enable this by default. This means additional testing/regression with the flag enabled. We could add this to Travis build (Travis yml file).

Currently, this patch will address use cases where the target hardware/environment is known during compilation.