diff mbox series

[v4,3/3] build: add option to enable wait until equal

Message ID 20210707054840.1608425-4-ruifeng.wang@arm.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series [v4,1/3] spinlock: use wfe to reduce contention on aarch64 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot success github build: passed
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Ruifeng Wang July 7, 2021, 5:48 a.m. UTC 
  Introduce a meson option 'use_wfe' to select wait until equal method.
The default is disable. Traditional polling loop is used.
When enabled, architecture specific mechanism is relied on to do the
wait.

Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 config/arm/meson.build | 2 +-
 meson_options.txt      | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon July 7, 2021, 6:32 a.m. UTC | #1 
07/07/2021 07:48, Ruifeng Wang:
> Introduce a meson option 'use_wfe' to select wait until equal method.
> The default is disable. Traditional polling loop is used.
> When enabled, architecture specific mechanism is relied on to do the
> wait.

Why do we need an option?
Can it be automatic to enable it when supported?
Ruifeng Wang July 7, 2021, 6:46 a.m. UTC | #2 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, July 7, 2021 2:32 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: Jan Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com; Bruce
> Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [PATCH v4 3/3] build: add option to enable wait until equal
> 
> 07/07/2021 07:48, Ruifeng Wang:
> > Introduce a meson option 'use_wfe' to select wait until equal method.
> > The default is disable. Traditional polling loop is used.
> > When enabled, architecture specific mechanism is relied on to do the
> > wait.
> 
> Why do we need an option?
> Can it be automatic to enable it when supported?
> 
The problem is inconsistency in performance on different Arm platforms. We had decided that each platform needs to enable it after some testing.
Bruce Richardson July 7, 2021, 12:27 p.m. UTC | #3 
On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, July 7, 2021 2:32 PM
> > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Cc: Jan Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com; Bruce
> > Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> > david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>
> > Subject: Re: [PATCH v4 3/3] build: add option to enable wait until equal
> > 
> > 07/07/2021 07:48, Ruifeng Wang:
> > > Introduce a meson option 'use_wfe' to select wait until equal method.
> > > The default is disable. Traditional polling loop is used.
> > > When enabled, architecture specific mechanism is relied on to do the
> > > wait.
> > 
> > Why do we need an option?
> > Can it be automatic to enable it when supported?
> > 
> The problem is inconsistency in performance on different Arm platforms. We had decided that each platform needs to enable it after some testing.
> 
Can that not be done via variables in the cross-file for the builds, or via
automatic detection if it's a native build? Is it likely that individual
users of DPDK will be knowledgable enough to use this option correctly?
Jerin Jacob July 7, 2021, 12:36 p.m. UTC | #4 
On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Wednesday, July 7, 2021 2:32 PM
> > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > Cc: Jan Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com; Bruce
> > > Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> > > david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com>
> > > Subject: Re: [PATCH v4 3/3] build: add option to enable wait until equal
> > >
> > > 07/07/2021 07:48, Ruifeng Wang:
> > > > Introduce a meson option 'use_wfe' to select wait until equal method.
> > > > The default is disable. Traditional polling loop is used.
> > > > When enabled, architecture specific mechanism is relied on to do the
> > > > wait.
> > >
> > > Why do we need an option?
> > > Can it be automatic to enable it when supported?
> > >
> > The problem is inconsistency in performance on different Arm platforms. We had decided that each platform needs to enable it after some testing.
> >
> Can that not be done via variables in the cross-file for the builds, or via
> automatic detection if it's a native build? Is it likely that individual
> users of DPDK will be knowledgable enough to use this option correctly?

+1 to add this in cross-file instead of the top of config option as
scope if is only for arm64 builds.
Ruifeng Wang July 8, 2021, 6:25 a.m. UTC | #5 
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Wednesday, July 7, 2021 8:37 PM
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; thomas@monjalon.net; Jan
> Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com; dev@dpdk.org;
> david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v4 3/3] build: add option to enable wait until
> equal
> 
> On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Wednesday, July 7, 2021 2:32 PM
> > > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > > Cc: Jan Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com;
> > > > Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> > > > david.marchand@redhat.com; nd <nd@arm.com>; Honnappa
> Nagarahalli
> > > > <Honnappa.Nagarahalli@arm.com>
> > > > Subject: Re: [PATCH v4 3/3] build: add option to enable wait until
> > > > equal
> > > >
> > > > 07/07/2021 07:48, Ruifeng Wang:
> > > > > Introduce a meson option 'use_wfe' to select wait until equal method.
> > > > > The default is disable. Traditional polling loop is used.
> > > > > When enabled, architecture specific mechanism is relied on to do
> > > > > the wait.
> > > >
> > > > Why do we need an option?
> > > > Can it be automatic to enable it when supported?
> > > >
> > > The problem is inconsistency in performance on different Arm platforms.
> We had decided that each platform needs to enable it after some testing.
> > >
> > Can that not be done via variables in the cross-file for the builds,
> > or via automatic detection if it's a native build? Is it likely that
> > individual users of DPDK will be knowledgable enough to use this option
> correctly?
> 
> +1 to add this in cross-file instead of the top of config option as
> scope if is only for arm64 builds.

Currently this option is in config/arm/meson.build (flags_common). SoCs can build
with this option enabled/disabled. And the ability is available for both native build
and cross build as cross build also goes through meson.build. 
If a SoC needs to enable the option by default, an entry and be added to the SoC flags.

The key difference here is whether this option need to be exposed to the top level config.
Jerin Jacob July 8, 2021, 6:32 a.m. UTC | #6 
On Thu, Jul 8, 2021 at 11:55 AM Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:
>
> > -----Original Message-----
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > Sent: Wednesday, July 7, 2021 8:37 PM
> > To: Bruce Richardson <bruce.richardson@intel.com>
> > Cc: Ruifeng Wang <Ruifeng.Wang@arm.com>; thomas@monjalon.net; Jan
> > Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com; dev@dpdk.org;
> > david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 3/3] build: add option to enable wait until
> > equal
> >
> > On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > > > -----Original Message-----
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > Sent: Wednesday, July 7, 2021 2:32 PM
> > > > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > > > Cc: Jan Viktorin <viktorin@rehivetech.com>; jerinj@marvell.com;
> > > > > Bruce Richardson <bruce.richardson@intel.com>; dev@dpdk.org;
> > > > > david.marchand@redhat.com; nd <nd@arm.com>; Honnappa
> > Nagarahalli
> > > > > <Honnappa.Nagarahalli@arm.com>
> > > > > Subject: Re: [PATCH v4 3/3] build: add option to enable wait until
> > > > > equal
> > > > >
> > > > > 07/07/2021 07:48, Ruifeng Wang:
> > > > > > Introduce a meson option 'use_wfe' to select wait until equal method.
> > > > > > The default is disable. Traditional polling loop is used.
> > > > > > When enabled, architecture specific mechanism is relied on to do
> > > > > > the wait.
> > > > >
> > > > > Why do we need an option?
> > > > > Can it be automatic to enable it when supported?
> > > > >
> > > > The problem is inconsistency in performance on different Arm platforms.
> > We had decided that each platform needs to enable it after some testing.
> > > >
> > > Can that not be done via variables in the cross-file for the builds,
> > > or via automatic detection if it's a native build? Is it likely that
> > > individual users of DPDK will be knowledgable enough to use this option
> > correctly?
> >
> > +1 to add this in cross-file instead of the top of config option as
> > scope if is only for arm64 builds.
>
> Currently this option is in config/arm/meson.build (flags_common). SoCs can build
> with this option enabled/disabled. And the ability is available for both native build
> and cross build as cross build also goes through meson.build.
> If a SoC needs to enable the option by default, an entry and be added to the SoC flags.
>
> The key difference here is whether this option need to be exposed to the top level config.

In the view of limiting top-level config options and it is specific to
Arm, I think, it better to
be a cross file only option.
Thomas Monjalon July 8, 2021, 7:32 a.m. UTC | #7 
08/07/2021 08:32, Jerin Jacob:
> On Thu, Jul 8, 2021 at 11:55 AM Ruifeng Wang <Ruifeng.Wang@arm.com> wrote:
> > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 07/07/2021 07:48, Ruifeng Wang:
> > > > > > > Introduce a meson option 'use_wfe' to select wait until equal method.
> > > > > > > The default is disable. Traditional polling loop is used.
> > > > > > > When enabled, architecture specific mechanism is relied on to do
> > > > > > > the wait.
> > > > > >
> > > > > > Why do we need an option?
> > > > > > Can it be automatic to enable it when supported?
> > > > > >
> > > > > The problem is inconsistency in performance on different Arm platforms.
> > > We had decided that each platform needs to enable it after some testing.
> > > > >
> > > > Can that not be done via variables in the cross-file for the builds,
> > > > or via automatic detection if it's a native build? Is it likely that
> > > > individual users of DPDK will be knowledgable enough to use this option
> > > correctly?
> > >
> > > +1 to add this in cross-file instead of the top of config option as
> > > scope if is only for arm64 builds.
> >
> > Currently this option is in config/arm/meson.build (flags_common). SoCs can build
> > with this option enabled/disabled. And the ability is available for both native build
> > and cross build as cross build also goes through meson.build.
> > If a SoC needs to enable the option by default, an entry and be added to the SoC flags.
> >
> > The key difference here is whether this option need to be exposed to the top level config.
> 
> In the view of limiting top-level config options and it is specific to
> Arm, I think, it better to
> be a cross file only option.

+1, sorry for the late notice.
I would advocate to take this patch in 21.08-rc2.
Ruifeng Wang July 8, 2021, 9:21 a.m. UTC | #8 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, July 8, 2021 3:33 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; Jerin Jacob
> <jerinjacobk@gmail.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; Jan Viktorin
> <viktorin@rehivetech.com>; jerinj@marvell.com; dev@dpdk.org;
> david.marchand@redhat.com; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v4 3/3] build: add option to enable wait until
> equal
> 
> 08/07/2021 08:32, Jerin Jacob:
> > On Thu, Jul 8, 2021 at 11:55 AM Ruifeng Wang <Ruifeng.Wang@arm.com>
> wrote:
> > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > > On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > 07/07/2021 07:48, Ruifeng Wang:
> > > > > > > > Introduce a meson option 'use_wfe' to select wait until equal
> method.
> > > > > > > > The default is disable. Traditional polling loop is used.
> > > > > > > > When enabled, architecture specific mechanism is relied on
> > > > > > > > to do the wait.
> > > > > > >
> > > > > > > Why do we need an option?
> > > > > > > Can it be automatic to enable it when supported?
> > > > > > >
> > > > > > The problem is inconsistency in performance on different Arm
> platforms.
> > > > We had decided that each platform needs to enable it after some
> testing.
> > > > > >
> > > > > Can that not be done via variables in the cross-file for the
> > > > > builds, or via automatic detection if it's a native build? Is it
> > > > > likely that individual users of DPDK will be knowledgable enough
> > > > > to use this option
> > > > correctly?
> > > >
> > > > +1 to add this in cross-file instead of the top of config option
> > > > +as
> > > > scope if is only for arm64 builds.
> > >
> > > Currently this option is in config/arm/meson.build (flags_common).
> > > SoCs can build with this option enabled/disabled. And the ability is
> > > available for both native build and cross build as cross build also goes
> through meson.build.
> > > If a SoC needs to enable the option by default, an entry and be added to
> the SoC flags.
> > >
> > > The key difference here is whether this option need to be exposed to
> the top level config.
> >
> > In the view of limiting top-level config options and it is specific to
> > Arm, I think, it better to be a cross file only option.
> 
> +1, sorry for the late notice.
> I would advocate to take this patch in 21.08-rc2.
> 
If the decision is not to expose this option in top-level config, we can just drop 3/3 patch.
Do I need to send a new version?
Thomas Monjalon July 8, 2021, 10:28 a.m. UTC | #9 
08/07/2021 11:21, Ruifeng Wang:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 08/07/2021 08:32, Jerin Jacob:
> > > On Thu, Jul 8, 2021 at 11:55 AM Ruifeng Wang <Ruifeng.Wang@arm.com>
> > wrote:
> > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > On Wed, Jul 7, 2021 at 5:57 PM Bruce Richardson
> > > > > <bruce.richardson@intel.com> wrote:
> > > > > > On Wed, Jul 07, 2021 at 06:46:33AM +0000, Ruifeng Wang wrote:
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > 07/07/2021 07:48, Ruifeng Wang:
> > > > > > > > > Introduce a meson option 'use_wfe' to select wait until equal
> > method.
> > > > > > > > > The default is disable. Traditional polling loop is used.
> > > > > > > > > When enabled, architecture specific mechanism is relied on
> > > > > > > > > to do the wait.
> > > > > > > >
> > > > > > > > Why do we need an option?
> > > > > > > > Can it be automatic to enable it when supported?
> > > > > > > >
> > > > > > > The problem is inconsistency in performance on different Arm
> > platforms.
> > > > > We had decided that each platform needs to enable it after some
> > testing.
> > > > > > >
> > > > > > Can that not be done via variables in the cross-file for the
> > > > > > builds, or via automatic detection if it's a native build? Is it
> > > > > > likely that individual users of DPDK will be knowledgable enough
> > > > > > to use this option
> > > > > correctly?
> > > > >
> > > > > +1 to add this in cross-file instead of the top of config option
> > > > > +as
> > > > > scope if is only for arm64 builds.
> > > >
> > > > Currently this option is in config/arm/meson.build (flags_common).
> > > > SoCs can build with this option enabled/disabled. And the ability is
> > > > available for both native build and cross build as cross build also goes
> > through meson.build.
> > > > If a SoC needs to enable the option by default, an entry and be added to
> > the SoC flags.
> > > >
> > > > The key difference here is whether this option need to be exposed to
> > the top level config.
> > >
> > > In the view of limiting top-level config options and it is specific to
> > > Arm, I think, it better to be a cross file only option.
> > 
> > +1, sorry for the late notice.
> > I would advocate to take this patch in 21.08-rc2.
> > 
> If the decision is not to expose this option in top-level config, we can just drop 3/3 patch.
> Do I need to send a new version?

No it looks not needed to send a new version, I can take patches 1 & 2.
diff mbox series

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 9b147c0b93..9ea478fb77 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -18,7 +18,6 @@  flags_common = [
         #    ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
 
         ['RTE_SCHED_VECTOR', false],
-        ['RTE_ARM_USE_WFE', false],
         ['RTE_ARCH_ARM64', true],
         ['RTE_CACHE_LINE_SIZE', 128]
 ]
@@ -371,6 +370,7 @@  socs = {
 
 dpdk_conf.set('RTE_ARCH_ARM', 1)
 dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
+dpdk_conf.set('RTE_ARM_USE_WFE', get_option('use_wfe'))
 
 if dpdk_conf.get('RTE_ARCH_32')
     # armv7 build
diff --git a/meson_options.txt b/meson_options.txt
index 56bdfd0f0a..5012803c77 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -44,3 +44,5 @@  option('tests', type: 'boolean', value: true, description:
        'build unit tests')
 option('use_hpet', type: 'boolean', value: false, description:
        'use HPET timer in EAL')
+option('use_wfe', type: 'boolean', value: false, description:
+       'use Wait Until Equal for polling loops')