[v12,09/14] build: optional NUMA and cpu counts detection
Checks
Commit Message
Add an option to automatically discover the host's numa and cpu counts
and use those values for a non cross-build.
Give users the option to override the per-arch default values or values
from cross files by specifying them on the command line with -Dmax_lcores
and -Dmax_numa_nodes.
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
buildtools/get_cpu_count.py | 7 ++++++
buildtools/get_numa_count.py | 22 +++++++++++++++++
buildtools/meson.build | 2 ++
config/meson.build | 47 ++++++++++++++++++++++++++++++++++--
config/x86/meson.build | 2 ++
meson_options.txt | 8 +++---
6 files changed, 82 insertions(+), 6 deletions(-)
create mode 100644 buildtools/get_cpu_count.py
create mode 100644 buildtools/get_numa_count.py
Comments
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Saturday, November 14, 2020 2:16 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: bruce.richardson@intel.com; Ruifeng.Wang@arm.com;
> Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> ferruh.yigit@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> 13/11/2020 15:31, Juraj Linkeš:
> > Add an option to automatically discover the host's numa and cpu counts
> > and use those values for a non cross-build.
> > Give users the option to override the per-arch default values or
> > values from cross files by specifying them on the command line with
> > -Dmax_lcores and -Dmax_numa_nodes.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> [...]
> > create mode 100644 buildtools/get_cpu_count.py create mode 100644
> > buildtools/get_numa_count.py
>
> These new files should be added in the file MAINTAINERS.
Should I add a new section? I just add them under Build System, making Bruce the maintainer of these?
> The recommended pattern is to use - as word separator in script filenames.
>
Ok, I'll rename them.
> I'm also worried there is no more review on such general change, especially the
> description in meson_options.txt:
>
> > +option('max_lcores', type: 'integer', value: 0,
> > + description: 'maximum number of cores/threads supported by
> > +EAL. Set to positive integer to overwrite per-arch or cross-compilation
> defaults. Set to -1 to detect the number of cores on the build machine.')
> option('max_numa_nodes', type: 'integer', value: 0,
> > + description: 'maximum number of NUMA nodes supported by EAL. Set to
> positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to
> detect the number of numa
Honnappa reviewed this and Bruce also provided his comments and he's the only maintainer listed for meson_options.txt. Who else should review this?
16/11/2020 08:15, Juraj Linkeš:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 13/11/2020 15:31, Juraj Linkeš:
> > > Add an option to automatically discover the host's numa and cpu counts
> > > and use those values for a non cross-build.
> > > Give users the option to override the per-arch default values or
> > > values from cross files by specifying them on the command line with
> > > -Dmax_lcores and -Dmax_numa_nodes.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > [...]
> > > create mode 100644 buildtools/get_cpu_count.py create mode 100644
> > > buildtools/get_numa_count.py
> >
> > These new files should be added in the file MAINTAINERS.
>
> Should I add a new section? I just add them under Build System,
> making Bruce the maintainer of these?
If Bruce is OK, it looks fine.
> > The recommended pattern is to use - as word separator in script filenames.
> >
>
> Ok, I'll rename them.
>
> > I'm also worried there is no more review on such general change, especially the
> > description in meson_options.txt:
> >
> > > +option('max_lcores', type: 'integer', value: 0,
> > > + description: 'maximum number of cores/threads supported by
> > > +EAL. Set to positive integer to overwrite per-arch or cross-compilation
> > defaults. Set to -1 to detect the number of cores on the build machine.')
> > option('max_numa_nodes', type: 'integer', value: 0,
> > > + description: 'maximum number of NUMA nodes supported by EAL. Set to
> > positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to
> > detect the number of numa
>
> Honnappa reviewed this and Bruce also provided his comments
> and he's the only maintainer listed for meson_options.txt.
> Who else should review this?
Bruce did not ack.
I want to do careful reviews myself as well.
13/11/2020 15:31, Juraj Linkeš:
> +option('max_lcores', type: 'integer', value: 0,
> + description: 'maximum number of cores/threads supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of cores on the build machine.')
> +option('max_numa_nodes', type: 'integer', value: 0,
> + description: 'maximum number of NUMA nodes supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of numa nodes on the build machine.')
First comment: I don't like having so long description.
Second: I don't understand.
It is said the default value is 0 so I expect it means automatic detection.
But later it is said -1 is for detection. So ?
14/11/2020 14:16, Thomas Monjalon:
> I didn't think this series is changing options for all archs.
> It was supposed to be an Arm-only rework.
> Clearly it is too late for such change in 20.11.
I recommend splitting the series for 21.02.
Do not change global stuff in a series named "Arm build options rework".
It will be easier to absorb as smaller items. Step by step.
On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> 13/11/2020 15:31, Juraj Linkeš:
> > +option('max_lcores', type: 'integer', value: 0,
> > + description: 'maximum number of cores/threads supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of cores on the build machine.')
> > +option('max_numa_nodes', type: 'integer', value: 0,
> > + description: 'maximum number of NUMA nodes supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of numa nodes on the build machine.')
>
> First comment: I don't like having so long description.
> Second: I don't understand.
>
> It is said the default value is 0 so I expect it means automatic detection.
> But later it is said -1 is for detection. So ?
>
Zero is for the "per-arch or cross-compilation default". This was discussed
quite a bit in previous versions and this was te best compromise we could
come up with. Having a default of auto-detect is definitely not something I
think we should go with - just thinking of all the build CI jobs running on
2 or 4 core VMs! However, Juraj really felt there was value in having
auto-detection, so it's set as a -1 value, which I'm ok with.
As I flagged in a previous mail thread, the whole config of DPDK builds is
something we need to take a bigger look at in 21.02 and beyond.
/Bruce
16/11/2020 10:13, Bruce Richardson:
> On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > 13/11/2020 15:31, Juraj Linkeš:
> > > +option('max_lcores', type: 'integer', value: 0,
> > > + description: 'maximum number of cores/threads supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of cores on the build machine.')
> > > +option('max_numa_nodes', type: 'integer', value: 0,
> > > + description: 'maximum number of NUMA nodes supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of numa nodes on the build machine.')
> >
> > First comment: I don't like having so long description.
> > Second: I don't understand.
> >
> > It is said the default value is 0 so I expect it means automatic detection.
> > But later it is said -1 is for detection. So ?
> >
> Zero is for the "per-arch or cross-compilation default". This was discussed
> quite a bit in previous versions and this was te best compromise we could
> come up with. Having a default of auto-detect is definitely not something I
> think we should go with - just thinking of all the build CI jobs running on
> 2 or 4 core VMs! However, Juraj really felt there was value in having
> auto-detection, so it's set as a -1 value, which I'm ok with.
The problem is that I don't understand what 0 means.
> As I flagged in a previous mail thread, the whole config of DPDK builds is
> something we need to take a bigger look at in 21.02 and beyond.
Yes definitely.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, November 16, 2020 8:38 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>;
> Honnappa.Nagarahalli@arm.com
> Cc: dev@dpdk.org; bruce.richardson@intel.com; Ruifeng.Wang@arm.com;
> Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> ajit.khaparde@broadcom.com; ferruh.yigit@intel.com;
> david.marchand@redhat.com
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> 14/11/2020 14:16, Thomas Monjalon:
> > I didn't think this series is changing options for all archs.
> > It was supposed to be an Arm-only rework.
> > Clearly it is too late for such change in 20.11.
>
> I recommend splitting the series for 21.02.
> Do not change global stuff in a series named "Arm build options rework".
> It will be easier to absorb as smaller items. Step by step.
>
>
Yes, this makes sense. This commit first started with arm specific parts/considerations, went through a lot of discussion and ended up here. I'll send it separately.
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, November 16, 2020 10:24 AM
> To: Bruce Richardson <bruce.richardson@intel.com>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Ruifeng.Wang@arm.com;
> Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> ferruh.yigit@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> 16/11/2020 10:13, Bruce Richardson:
> > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > 13/11/2020 15:31, Juraj Linkeš:
> > > > +option('max_lcores', type: 'integer', value: 0,
> > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > +Set to positive integer to overwrite per-arch or cross-compilation
> defaults. Set to -1 to detect the number of cores on the build machine.')
> option('max_numa_nodes', type: 'integer', value: 0,
> > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > +build machine.')
> > >
> > > First comment: I don't like having so long description.
> > > Second: I don't understand.
> > >
> > > It is said the default value is 0 so I expect it means automatic detection.
> > > But later it is said -1 is for detection. So ?
> > >
> > Zero is for the "per-arch or cross-compilation default". This was
> > discussed quite a bit in previous versions and this was te best
> > compromise we could come up with. Having a default of auto-detect is
> > definitely not something I think we should go with - just thinking of
> > all the build CI jobs running on
> > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > auto-detection, so it's set as a -1 value, which I'm ok with.
>
> The problem is that I don't understand what 0 means.
>
There are three pieces of information which we need to convey:
1. The default value (0) indicates that per-arch or cross-compilation defaults will be used.
2. Positive integer values will be used instead of these defaults.
3. Detected values will be used for native build when the value is -1.
The description is a bit longer because of this and I've already tried to shorten it by combining 1 and 2 into one sentence. But maybe we can shorten it while making it a bit clearer. I'll think about it.
> > As I flagged in a previous mail thread, the whole config of DPDK
> > builds is something we need to take a bigger look at in 21.02 and beyond.
>
> Yes definitely.
>
>
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, November 16, 2020 8:20 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: bruce.richardson@intel.com; Ruifeng.Wang@arm.com;
> Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> ferruh.yigit@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> 16/11/2020 08:15, Juraj Linkeš:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 13/11/2020 15:31, Juraj Linkeš:
> > > > Add an option to automatically discover the host's numa and cpu
> > > > counts and use those values for a non cross-build.
> > > > Give users the option to override the per-arch default values or
> > > > values from cross files by specifying them on the command line
> > > > with -Dmax_lcores and -Dmax_numa_nodes.
> > > >
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > [...]
> > > > create mode 100644 buildtools/get_cpu_count.py create mode
> > > > 100644 buildtools/get_numa_count.py
> > >
> > > These new files should be added in the file MAINTAINERS.
> >
> > Should I add a new section? I just add them under Build System, making
> > Bruce the maintainer of these?
>
> If Bruce is OK, it looks fine.
>
Bruce, may I add the files to MAINTAINERS under your name?
On Wed, Nov 18, 2020 at 02:20:53PM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Monday, November 16, 2020 8:20 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: bruce.richardson@intel.com; Ruifeng.Wang@arm.com;
> > Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> > vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> > hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> > ferruh.yigit@intel.com; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> > detection
> >
> > 16/11/2020 08:15, Juraj Linkeš:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > Add an option to automatically discover the host's numa and cpu
> > > > > counts and use those values for a non cross-build.
> > > > > Give users the option to override the per-arch default values or
> > > > > values from cross files by specifying them on the command line
> > > > > with -Dmax_lcores and -Dmax_numa_nodes.
> > > > >
> > > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > [...]
> > > > > create mode 100644 buildtools/get_cpu_count.py create mode
> > > > > 100644 buildtools/get_numa_count.py
> > > >
> > > > These new files should be added in the file MAINTAINERS.
> > >
> > > Should I add a new section? I just add them under Build System, making
> > > Bruce the maintainer of these?
> >
> > If Bruce is OK, it looks fine.
> >
>
> Bruce, may I add the files to MAINTAINERS under your name?
Yes, that is ok for these simple scripts.
18/11/2020 15:19, Juraj Linkeš:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 16/11/2020 10:13, Bruce Richardson:
> > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > defaults. Set to -1 to detect the number of cores on the build machine.')
> > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > +build machine.')
> > > >
> > > > First comment: I don't like having so long description.
> > > > Second: I don't understand.
> > > >
> > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > But later it is said -1 is for detection. So ?
> > > >
> > > Zero is for the "per-arch or cross-compilation default". This was
> > > discussed quite a bit in previous versions and this was te best
> > > compromise we could come up with. Having a default of auto-detect is
> > > definitely not something I think we should go with - just thinking of
> > > all the build CI jobs running on
> > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > auto-detection, so it's set as a -1 value, which I'm ok with.
> >
> > The problem is that I don't understand what 0 means.
> >
>
> There are three pieces of information which we need to convey:
> 1. The default value (0) indicates that per-arch or cross-compilation defaults will be used.
> 2. Positive integer values will be used instead of these defaults.
Where these positive values come from?
> 3. Detected values will be used for native build when the value is -1.
Why not detect for any native build set up with 0 (default)?
On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> 18/11/2020 15:19, Juraj Linkeš:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 16/11/2020 10:13, Bruce Richardson:
> > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > > defaults. Set to -1 to detect the number of cores on the build machine.')
> > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > +build machine.')
> > > > >
> > > > > First comment: I don't like having so long description.
> > > > > Second: I don't understand.
> > > > >
> > > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > > But later it is said -1 is for detection. So ?
> > > > >
> > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > discussed quite a bit in previous versions and this was te best
> > > > compromise we could come up with. Having a default of auto-detect is
> > > > definitely not something I think we should go with - just thinking of
> > > > all the build CI jobs running on
> > > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > >
> > > The problem is that I don't understand what 0 means.
> > >
> >
> > There are three pieces of information which we need to convey:
> > 1. The default value (0) indicates that per-arch or cross-compilation defaults will be used.
> > 2. Positive integer values will be used instead of these defaults.
>
> Where these positive values come from?
>
> > 3. Detected values will be used for native build when the value is -1.
>
> Why not detect for any native build set up with 0 (default)?
>
That was one of the original suggestions, but I strongly disagreed with
that, because many builds are done on hardware very different from the
final deployment. It would mean that any builds done in e.g. jenkins or
travis, with a 2-core vm, would be limited to running with two cores only.
18/11/2020 15:54, Bruce Richardson:
> On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > 18/11/2020 15:19, Juraj Linkeš:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > > > defaults. Set to -1 to detect the number of cores on the build machine.')
> > > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > > +build machine.')
> > > > > >
> > > > > > First comment: I don't like having so long description.
> > > > > > Second: I don't understand.
> > > > > >
> > > > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > > > But later it is said -1 is for detection. So ?
> > > > > >
> > > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > > discussed quite a bit in previous versions and this was te best
> > > > > compromise we could come up with. Having a default of auto-detect is
> > > > > definitely not something I think we should go with - just thinking of
> > > > > all the build CI jobs running on
> > > > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > > >
> > > > The problem is that I don't understand what 0 means.
> > > >
> > >
> > > There are three pieces of information which we need to convey:
> > > 1. The default value (0) indicates that per-arch or cross-compilation defaults will be used.
> > > 2. Positive integer values will be used instead of these defaults.
> >
> > Where these positive values come from?
> >
> > > 3. Detected values will be used for native build when the value is -1.
> >
> > Why not detect for any native build set up with 0 (default)?
> >
> That was one of the original suggestions, but I strongly disagreed with
> that, because many builds are done on hardware very different from the
> final deployment. It would mean that any builds done in e.g. jenkins or
> travis, with a 2-core vm, would be limited to running with two cores only.
Yes that's the difference between native and cross build:
native build is not for running on a different machine.
I feel you have a different understanding of native build?
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, November 18, 2020 3:43 PM
> To: Bruce Richardson <bruce.richardson@intel.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>
> Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> 18/11/2020 15:19, Juraj Linkeš:
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 16/11/2020 10:13, Bruce Richardson:
> > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > + description: 'maximum number of cores/threads supported by
> EAL.
> > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > +cross-compilation
> > > defaults. Set to -1 to detect the number of cores on the build
> > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > + description: 'maximum number of NUMA nodes supported by
> EAL.
> > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > +cross-compilation defaults. Set to -1 to detect the number of
> > > > > > +numa nodes on the build machine.')
> > > > >
> > > > > First comment: I don't like having so long description.
> > > > > Second: I don't understand.
> > > > >
> > > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > > But later it is said -1 is for detection. So ?
> > > > >
> > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > discussed quite a bit in previous versions and this was te best
> > > > compromise we could come up with. Having a default of auto-detect
> > > > is definitely not something I think we should go with - just
> > > > thinking of all the build CI jobs running on
> > > > 2 or 4 core VMs! However, Juraj really felt there was value in
> > > > having auto-detection, so it's set as a -1 value, which I'm ok with.
> > >
> > > The problem is that I don't understand what 0 means.
> > >
> >
> > There are three pieces of information which we need to convey:
> > 1. The default value (0) indicates that per-arch or cross-compilation defaults
> will be used.
> > 2. Positive integer values will be used instead of these defaults.
>
> Where these positive values come from?
>
From the user - they will have the option to set it to whatever the like if they don't want to use defaults.
> > 3. Detected values will be used for native build when the value is -1.
>
> Why not detect for any native build set up with 0 (default)?
>
I'll let Bruce explain this, but I'll just say that we wanted to make the detection the default for native builds, so we're in agreement.
On Wed, Nov 18, 2020 at 04:04:25PM +0100, Thomas Monjalon wrote:
> 18/11/2020 15:54, Bruce Richardson:
> > On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > > 18/11/2020 15:19, Juraj Linkeš:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > + description: 'maximum number of cores/threads supported by EAL.
> > > > > > > > +Set to positive integer to overwrite per-arch or cross-compilation
> > > > > defaults. Set to -1 to detect the number of cores on the build machine.')
> > > > > option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > + description: 'maximum number of NUMA nodes supported by EAL. Set
> > > > > > > > +to positive integer to overwrite per-arch or cross-compilation
> > > > > > > > +defaults. Set to -1 to detect the number of numa nodes on the
> > > > > > > > +build machine.')
> > > > > > >
> > > > > > > First comment: I don't like having so long description.
> > > > > > > Second: I don't understand.
> > > > > > >
> > > > > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > > > > But later it is said -1 is for detection. So ?
> > > > > > >
> > > > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > > > discussed quite a bit in previous versions and this was te best
> > > > > > compromise we could come up with. Having a default of auto-detect is
> > > > > > definitely not something I think we should go with - just thinking of
> > > > > > all the build CI jobs running on
> > > > > > 2 or 4 core VMs! However, Juraj really felt there was value in having
> > > > > > auto-detection, so it's set as a -1 value, which I'm ok with.
> > > > >
> > > > > The problem is that I don't understand what 0 means.
> > > > >
> > > >
> > > > There are three pieces of information which we need to convey:
> > > > 1. The default value (0) indicates that per-arch or cross-compilation defaults will be used.
> > > > 2. Positive integer values will be used instead of these defaults.
> > >
> > > Where these positive values come from?
> > >
> > > > 3. Detected values will be used for native build when the value is -1.
> > >
> > > Why not detect for any native build set up with 0 (default)?
> > >
> > That was one of the original suggestions, but I strongly disagreed with
> > that, because many builds are done on hardware very different from the
> > final deployment. It would mean that any builds done in e.g. jenkins or
> > travis, with a 2-core vm, would be limited to running with two cores only.
>
> Yes that's the difference between native and cross build:
> native build is not for running on a different machine.
> I feel you have a different understanding of native build?
>
A cross-build is for running on a different architecture, not a different
machine. I can build on my laptop to run on a Xeon server and it's still a
native build rather than a cross build. For this discussion, a cross-build
is considered something using a cross-file.
/Bruce
<snip>
>
> On Wed, Nov 18, 2020 at 04:04:25PM +0100, Thomas Monjalon wrote:
> > 18/11/2020 15:54, Bruce Richardson:
> > > On Wed, Nov 18, 2020 at 03:42:36PM +0100, Thomas Monjalon wrote:
> > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon
> wrote:
> > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of cores/threads
> supported by EAL.
> > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > +cross-compilation
> > > > > > defaults. Set to -1 to detect the number of cores on the build
> > > > > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of NUMA nodes
> supported
> > > > > > > > > +by EAL. Set to positive integer to overwrite per-arch
> > > > > > > > > +or cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > >
> > > > > > > > First comment: I don't like having so long description.
> > > > > > > > Second: I don't understand.
> > > > > > > >
> > > > > > > > It is said the default value is 0 so I expect it means automatic
> detection.
> > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > >
> > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > This was discussed quite a bit in previous versions and this
> > > > > > > was te best compromise we could come up with. Having a
> > > > > > > default of auto-detect is definitely not something I think
> > > > > > > we should go with - just thinking of all the build CI jobs
> > > > > > > running on
> > > > > > > 2 or 4 core VMs! However, Juraj really felt there was value
> > > > > > > in having auto-detection, so it's set as a -1 value, which I'm ok
> with.
> > > > > >
> > > > > > The problem is that I don't understand what 0 means.
> > > > > >
> > > > >
> > > > > There are three pieces of information which we need to convey:
> > > > > 1. The default value (0) indicates that per-arch or cross-compilation
> defaults will be used.
> > > > > 2. Positive integer values will be used instead of these defaults.
> > > >
> > > > Where these positive values come from?
> > > >
> > > > > 3. Detected values will be used for native build when the value is -1.
> > > >
> > > > Why not detect for any native build set up with 0 (default)?
> > > >
> > > That was one of the original suggestions, but I strongly disagreed
> > > with that, because many builds are done on hardware very different
> > > from the final deployment. It would mean that any builds done in
> > > e.g. jenkins or travis, with a 2-core vm, would be limited to running with
> two cores only.
> >
> > Yes that's the difference between native and cross build:
> > native build is not for running on a different machine.
> > I feel you have a different understanding of native build?
> >
> A cross-build is for running on a different architecture, not a different
> machine. I can build on my laptop to run on a Xeon server and it's still a native
> build rather than a cross build. For this discussion, a cross-build is considered
> something using a cross-file.
>
This requires that the CPU on the laptop and the Xeon server support the same ISA version (I am not sure what it is called in x86). For ex: if the laptop supports AVX512, the Xeon server also needs to support AVX512. So, the user needs to be aware of all the instructions (or ISA version) on which the binary was built to be able to run it on another x86 machine. I am not sure if this is called out or understood by the users. In this regard, I tend to agree with Thomas that a native build is running on the same machine where it was built.
> /Bruce
On Wed, Nov 18, 2020 at 03:23:13PM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Wednesday, November 18, 2020 3:43 PM
> > To: Bruce Richardson <bruce.richardson@intel.com>; Juraj Linkeš
> > <juraj.linkes@pantheon.tech>
> > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> > Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> > jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> > ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> > detection
> >
> > 18/11/2020 15:19, Juraj Linkeš:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of cores/threads supported by
> > EAL.
> > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > +cross-compilation
> > > > defaults. Set to -1 to detect the number of cores on the build
> > > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > + description: 'maximum number of NUMA nodes supported by
> > EAL.
> > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > +cross-compilation defaults. Set to -1 to detect the number of
> > > > > > > +numa nodes on the build machine.')
> > > > > >
> > > > > > First comment: I don't like having so long description.
> > > > > > Second: I don't understand.
> > > > > >
> > > > > > It is said the default value is 0 so I expect it means automatic detection.
> > > > > > But later it is said -1 is for detection. So ?
> > > > > >
> > > > > Zero is for the "per-arch or cross-compilation default". This was
> > > > > discussed quite a bit in previous versions and this was te best
> > > > > compromise we could come up with. Having a default of auto-detect
> > > > > is definitely not something I think we should go with - just
> > > > > thinking of all the build CI jobs running on
> > > > > 2 or 4 core VMs! However, Juraj really felt there was value in
> > > > > having auto-detection, so it's set as a -1 value, which I'm ok with.
> > > >
> > > > The problem is that I don't understand what 0 means.
> > > >
> > >
> > > There are three pieces of information which we need to convey:
> > > 1. The default value (0) indicates that per-arch or cross-compilation defaults
> > will be used.
> > > 2. Positive integer values will be used instead of these defaults.
> >
> > Where these positive values come from?
> >
>
> From the user - they will have the option to set it to whatever the like if they don't want to use defaults.
>
> > > 3. Detected values will be used for native build when the value is -1.
> >
> > Why not detect for any native build set up with 0 (default)?
> >
>
> I'll let Bruce explain this, but I'll just say that we wanted to make the detection the default for native builds, so we're in agreement.
I think most of us agree that the different understanding of the term
"native build", is the cause of much of the disagreements and points of
dispute on this thread. From my view point, the term "native" can refer to:
1. what meson considers a native build, i.e. one not using a cross-file
2. a build for a different machine architecture to the one on the build
machine (this largely overlaps with #1, except that e.g. 32-bit build on
64-bit may be considered a cross-build in this case).
3. a build tailored exactly for the build machine itself i.e. both ISA, and
things like core counts.
4. a flag passed to the compiler to indicate the uarch level of the
instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
that of the build machine.
Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally #4 since
that was it's use on x86 in the first versions of DPDK. With the move from
make to meson, that aspect was kept, but the meaning of #1 (I think we can
ignore #2) also came into play. Finally, while for x86 architecture, the
idea of #4 still held, for ARM use #3 is of major concern.
Is this a fair summary?
Based on this, my thinking is that the current "machine" value really needs
to be either renamed or split into two. We need to separate out the idea of
the "platform" (apologies if this is not the right term), from the
"instruction set"/"uarch" to make it clear what the value refers to. The
default "platform" value should probably be "generic", and the default
"instruction set" should be "default", which means it's set by the
"platform" value.
This I believe should allow the flexibility we need, i.e. to tune to the
native machine (case #3) above, adjust the platform to "native", while to
get behaviour #4, and only just the ISA level, but keep generic in terms of
other values, adjust the "instruction set" value. In other words, for x86
the "machine" value as used becomes the "instruction set" one, while for
ARM (if I understand the requirements correctly) the "machine" value
becomes the "platform" one.
Thoughts on this?
/Bruce
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 19, 2020 1:20 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Thomas Monjalon <thomas@monjalon.net>; Ruifeng.Wang@arm.com;
> Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> ferruh.yigit@intel.com; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> On Wed, Nov 18, 2020 at 03:23:13PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > Sent: Wednesday, November 18, 2020 3:43 PM
> > > To: Bruce Richardson <bruce.richardson@intel.com>; Juraj Linkeš
> > > <juraj.linkes@pantheon.tech>
> > > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> > > Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> > > jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> > > ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and
> > > cpu counts detection
> > >
> > > 18/11/2020 15:19, Juraj Linkeš:
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > + description: 'maximum number of cores/threads supported
> > > > > > > > +by
> > > EAL.
> > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > +cross-compilation
> > > > > defaults. Set to -1 to detect the number of cores on the build
> > > > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > + description: 'maximum number of NUMA nodes supported by
> > > EAL.
> > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > +cross-compilation defaults. Set to -1 to detect the
> > > > > > > > +number of numa nodes on the build machine.')
> > > > > > >
> > > > > > > First comment: I don't like having so long description.
> > > > > > > Second: I don't understand.
> > > > > > >
> > > > > > > It is said the default value is 0 so I expect it means automatic
> detection.
> > > > > > > But later it is said -1 is for detection. So ?
> > > > > > >
> > > > > > Zero is for the "per-arch or cross-compilation default". This
> > > > > > was discussed quite a bit in previous versions and this was te
> > > > > > best compromise we could come up with. Having a default of
> > > > > > auto-detect is definitely not something I think we should go
> > > > > > with - just thinking of all the build CI jobs running on
> > > > > > 2 or 4 core VMs! However, Juraj really felt there was value in
> > > > > > having auto-detection, so it's set as a -1 value, which I'm ok with.
> > > > >
> > > > > The problem is that I don't understand what 0 means.
> > > > >
> > > >
> > > > There are three pieces of information which we need to convey:
> > > > 1. The default value (0) indicates that per-arch or
> > > > cross-compilation defaults
> > > will be used.
> > > > 2. Positive integer values will be used instead of these defaults.
> > >
> > > Where these positive values come from?
> > >
> >
> > From the user - they will have the option to set it to whatever the like if they
> don't want to use defaults.
> >
> > > > 3. Detected values will be used for native build when the value is -1.
> > >
> > > Why not detect for any native build set up with 0 (default)?
> > >
> >
> > I'll let Bruce explain this, but I'll just say that we wanted to make the detection
> the default for native builds, so we're in agreement.
>
> I think most of us agree that the different understanding of the term "native
> build", is the cause of much of the disagreements and points of dispute on this
> thread. From my view point, the term "native" can refer to:
>
> 1. what meson considers a native build, i.e. one not using a cross-file 2. a build
> for a different machine architecture to the one on the build
> machine (this largely overlaps with #1, except that e.g. 32-bit build on
> 64-bit may be considered a cross-build in this case).
> 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> things like core counts.
> 4. a flag passed to the compiler to indicate the uarch level of the
> instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> that of the build machine.
>
> Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally #4 since that
> was it's use on x86 in the first versions of DPDK. With the move from make to
> meson, that aspect was kept, but the meaning of #1 (I think we can ignore #2)
> also came into play. Finally, while for x86 architecture, the idea of #4 still held,
> for ARM use #3 is of major concern.
>
> Is this a fair summary?
>
> Based on this, my thinking is that the current "machine" value really needs to be
> either renamed or split into two. We need to separate out the idea of the
> "platform" (apologies if this is not the right term), from the "instruction
> set"/"uarch" to make it clear what the value refers to. The default "platform"
> value should probably be "generic", and the default "instruction set" should be
> "default", which means it's set by the "platform" value.
>
> This I believe should allow the flexibility we need, i.e. to tune to the native
> machine (case #3) above, adjust the platform to "native", while to get behaviour
> #4, and only just the ISA level, but keep generic in terms of other values, adjust
> the "instruction set" value. In other words, for x86 the "machine" value as used
> becomes the "instruction set" one, while for ARM (if I understand the
> requirements correctly) the "machine" value becomes the "platform" one.
>
> Thoughts on this?
>
> /Bruce
I like where this is heading.
Using a new option to set the platform/build type will remove all the confusion, I think.
Then the 'machine' option will just set the machine args (and RTE_MACHINE, but that doesn't do anything as far as I can tell) and it'll work just like 'max_lcores' and 'max_numa_nodes' do - set just that one thing. But I don't like using the value 'default' to mean 'set by other option' (it's more 'ignore this' than 'default'). I like 'auto' or something similar more.
On Thu, Nov 19, 2020 at 01:57:59PM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, November 19, 2020 1:20 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: Thomas Monjalon <thomas@monjalon.net>; Ruifeng.Wang@arm.com;
> > Honnappa.Nagarahalli@arm.com; Phil.Yang@arm.com;
> > vcchunga@amazon.com; Dharmik.Thakkar@arm.com; jerinjacobk@gmail.com;
> > hemant.agrawal@nxp.com; ajit.khaparde@broadcom.com;
> > ferruh.yigit@intel.com; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> > detection
> >
> > On Wed, Nov 18, 2020 at 03:23:13PM +0000, Juraj Linkeš wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > Sent: Wednesday, November 18, 2020 3:43 PM
> > > > To: Bruce Richardson <bruce.richardson@intel.com>; Juraj Linkeš
> > > > <juraj.linkes@pantheon.tech>
> > > > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> > > > Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> > > > jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> > > > ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; dev@dpdk.org
> > > > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and
> > > > cpu counts detection
> > > >
> > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon wrote:
> > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of cores/threads supported
> > > > > > > > > +by
> > > > EAL.
> > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > +cross-compilation
> > > > > > defaults. Set to -1 to detect the number of cores on the build
> > > > > > machine.') option('max_numa_nodes', type: 'integer', value: 0,
> > > > > > > > > + description: 'maximum number of NUMA nodes supported by
> > > > EAL.
> > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > +cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > >
> > > > > > > > First comment: I don't like having so long description.
> > > > > > > > Second: I don't understand.
> > > > > > > >
> > > > > > > > It is said the default value is 0 so I expect it means automatic
> > detection.
> > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > >
> > > > > > > Zero is for the "per-arch or cross-compilation default". This
> > > > > > > was discussed quite a bit in previous versions and this was te
> > > > > > > best compromise we could come up with. Having a default of
> > > > > > > auto-detect is definitely not something I think we should go
> > > > > > > with - just thinking of all the build CI jobs running on
> > > > > > > 2 or 4 core VMs! However, Juraj really felt there was value in
> > > > > > > having auto-detection, so it's set as a -1 value, which I'm ok with.
> > > > > >
> > > > > > The problem is that I don't understand what 0 means.
> > > > > >
> > > > >
> > > > > There are three pieces of information which we need to convey:
> > > > > 1. The default value (0) indicates that per-arch or
> > > > > cross-compilation defaults
> > > > will be used.
> > > > > 2. Positive integer values will be used instead of these defaults.
> > > >
> > > > Where these positive values come from?
> > > >
> > >
> > > From the user - they will have the option to set it to whatever the like if they
> > don't want to use defaults.
> > >
> > > > > 3. Detected values will be used for native build when the value is -1.
> > > >
> > > > Why not detect for any native build set up with 0 (default)?
> > > >
> > >
> > > I'll let Bruce explain this, but I'll just say that we wanted to make the detection
> > the default for native builds, so we're in agreement.
> >
> > I think most of us agree that the different understanding of the term "native
> > build", is the cause of much of the disagreements and points of dispute on this
> > thread. From my view point, the term "native" can refer to:
> >
> > 1. what meson considers a native build, i.e. one not using a cross-file 2. a build
> > for a different machine architecture to the one on the build
> > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > 64-bit may be considered a cross-build in this case).
> > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > things like core counts.
> > 4. a flag passed to the compiler to indicate the uarch level of the
> > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > that of the build machine.
> >
> > Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally #4 since that
> > was it's use on x86 in the first versions of DPDK. With the move from make to
> > meson, that aspect was kept, but the meaning of #1 (I think we can ignore #2)
> > also came into play. Finally, while for x86 architecture, the idea of #4 still held,
> > for ARM use #3 is of major concern.
> >
> > Is this a fair summary?
> >
> > Based on this, my thinking is that the current "machine" value really needs to be
> > either renamed or split into two. We need to separate out the idea of the
> > "platform" (apologies if this is not the right term), from the "instruction
> > set"/"uarch" to make it clear what the value refers to. The default "platform"
> > value should probably be "generic", and the default "instruction set" should be
> > "default", which means it's set by the "platform" value.
> >
> > This I believe should allow the flexibility we need, i.e. to tune to the native
> > machine (case #3) above, adjust the platform to "native", while to get behaviour
> > #4, and only just the ISA level, but keep generic in terms of other values, adjust
> > the "instruction set" value. In other words, for x86 the "machine" value as used
> > becomes the "instruction set" one, while for ARM (if I understand the
> > requirements correctly) the "machine" value becomes the "platform" one.
> >
> > Thoughts on this?
> >
> > /Bruce
>
> I like where this is heading.
> Using a new option to set the platform/build type will remove all the confusion, I think.
> Then the 'machine' option will just set the machine args (and RTE_MACHINE, but that doesn't do anything as far as I can tell) and it'll work just like 'max_lcores' and 'max_numa_nodes' do - set just that one thing. But I don't like using the value 'default' to mean 'set by other option' (it's more 'ignore this' than 'default'). I like 'auto' or something similar more.
I'm ok with "auto" instead of "default". If max_lcores and max_numa_nodes
stick around as options, I'd even suggest changing them to string type so
that we can use "auto" as a default value there too.
I actually think too that to avoid any confusion with what went before, we
should remove the "machine" option entirely and replace it with the
"platform" and "ISA" (or suitable name, maybe "march_flag"??) options. I'd
like to remove any expectations of behaviour similar to what went before,
otherwise we'll get stuck in lots of compatibility issues. Better to define
new options with new - clearly defined - behaviours than try clarifying an
existing option after the fact.
<snip>
> > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon
> wrote:
> > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > +supported by
> > > > > EAL.
> > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > +cross-compilation
> > > > > > > defaults. Set to -1 to detect the number of cores on the
> > > > > > > build
> > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > 0,
> > > > > > > > > > + description: 'maximum number of NUMA nodes
> supported
> > > > > > > > > > +by
> > > > > EAL.
> > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > +cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > > >
> > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > Second: I don't understand.
> > > > > > > > >
> > > > > > > > > It is said the default value is 0 so I expect it means
> > > > > > > > > automatic
> > > detection.
> > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > >
> > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > This was discussed quite a bit in previous versions and
> > > > > > > > this was te best compromise we could come up with. Having
> > > > > > > > a default of auto-detect is definitely not something I
> > > > > > > > think we should go with - just thinking of all the build
> > > > > > > > CI jobs running on
> > > > > > > > 2 or 4 core VMs! However, Juraj really felt there was
> > > > > > > > value in having auto-detection, so it's set as a -1 value, which I'm
> ok with.
> > > > > > >
> > > > > > > The problem is that I don't understand what 0 means.
> > > > > > >
> > > > > >
> > > > > > There are three pieces of information which we need to convey:
> > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > cross-compilation defaults
> > > > > will be used.
> > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > >
> > > > > Where these positive values come from?
> > > > >
> > > >
> > > > From the user - they will have the option to set it to whatever
> > > > the like if they
> > > don't want to use defaults.
> > > >
> > > > > > 3. Detected values will be used for native build when the value is -1.
> > > > >
> > > > > Why not detect for any native build set up with 0 (default)?
> > > > >
> > > >
> > > > I'll let Bruce explain this, but I'll just say that we wanted to
> > > > make the detection
> > > the default for native builds, so we're in agreement.
> > >
> > > I think most of us agree that the different understanding of the
> > > term "native build", is the cause of much of the disagreements and
Agree, that's the main reason.
> > > points of dispute on this thread. From my view point, the term "native"
> can refer to:
> > >
> > > 1. what meson considers a native build, i.e. one not using a
> > > cross-file 2. a build for a different machine architecture to the one on the
> build
> > > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > > 64-bit may be considered a cross-build in this case).
Sorry, I did not understand #2 here. Are you saying, native "means" - "a build for a different machine architecture to the one on the build machine"
> > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > things like core counts.
> > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > that of the build machine.
> > >
> > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally
> > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > With the move from make to meson, that aspect was kept, but the
> > > meaning of #1 (I think we can ignore #2) also came into play.
> > > Finally, while for x86 architecture, the idea of #4 still held, for ARM use #3
> is of major concern.
Yes, #3 is the concern.
At the same time, I am also interested in avoiding 'native' (or any other option) having different meaning for different architectures.
Now that we have introduced 'soc' option for Arm platforms, we are able to achieve the builds that would be produced by #3.
'soc' combines both the 'platform' and 'instruction set' (as you have defined them below).
> > >
> > > Is this a fair summary?
> > >
> > > Based on this, my thinking is that the current "machine" value
> > > really needs to be either renamed or split into two. We need to
> > > separate out the idea of the "platform" (apologies if this is not
> > > the right term), from the "instruction set"/"uarch" to make it clear what
> the value refers to. The default "platform"
> > > value should probably be "generic", and the default "instruction
> > > set" should be "default", which means it's set by the "platform" value.
> > >
> > > This I believe should allow the flexibility we need, i.e. to tune to
> > > the native machine (case #3) above, adjust the platform to "native",
> > > while to get behaviour #4, and only just the ISA level, but keep
> > > generic in terms of other values, adjust the "instruction set"
> > > value. In other words, for x86 the "machine" value as used becomes
> > > the "instruction set" one, while for ARM (if I understand the
> requirements correctly) the "machine" value becomes the "platform" one.
> > >
> > > Thoughts on this?
> > >
> > > /Bruce
> >
> > I like where this is heading.
> > Using a new option to set the platform/build type will remove all the
> confusion, I think.
> > Then the 'machine' option will just set the machine args (and
> RTE_MACHINE, but that doesn't do anything as far as I can tell) and it'll work
> just like 'max_lcores' and 'max_numa_nodes' do - set just that one thing. But
> I don't like using the value 'default' to mean 'set by other option' (it's more
> 'ignore this' than 'default'). I like 'auto' or something similar more.
>
> I'm ok with "auto" instead of "default". If max_lcores and max_numa_nodes
> stick around as options, I'd even suggest changing them to string type so that
> we can use "auto" as a default value there too.
>
> I actually think too that to avoid any confusion with what went before, we
> should remove the "machine" option entirely and replace it with the
> "platform" and "ISA" (or suitable name, maybe "march_flag"??) options. I'd
> like to remove any expectations of behaviour similar to what went before,
> otherwise we'll get stuck in lots of compatibility issues. Better to define new
> options with new - clearly defined - behaviours than try clarifying an existing
> option after the fact.
On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> <snip>
>
> > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon
> > wrote:
> > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > +supported by
> > > > > > EAL.
> > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > +cross-compilation
> > > > > > > > defaults. Set to -1 to detect the number of cores on the
> > > > > > > > build
> > > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > > 0,
> > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > supported
> > > > > > > > > > > +by
> > > > > > EAL.
> > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > +cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > > > >
> > > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > > Second: I don't understand.
> > > > > > > > > >
> > > > > > > > > > It is said the default value is 0 so I expect it means
> > > > > > > > > > automatic
> > > > detection.
> > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > >
> > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > This was discussed quite a bit in previous versions and
> > > > > > > > > this was te best compromise we could come up with. Having
> > > > > > > > > a default of auto-detect is definitely not something I
> > > > > > > > > think we should go with - just thinking of all the build
> > > > > > > > > CI jobs running on
> > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there was
> > > > > > > > > value in having auto-detection, so it's set as a -1 value, which I'm
> > ok with.
> > > > > > > >
> > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > >
> > > > > > >
> > > > > > > There are three pieces of information which we need to convey:
> > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > cross-compilation defaults
> > > > > > will be used.
> > > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > > >
> > > > > > Where these positive values come from?
> > > > > >
> > > > >
> > > > > From the user - they will have the option to set it to whatever
> > > > > the like if they
> > > > don't want to use defaults.
> > > > >
> > > > > > > 3. Detected values will be used for native build when the value is -1.
> > > > > >
> > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > >
> > > > >
> > > > > I'll let Bruce explain this, but I'll just say that we wanted to
> > > > > make the detection
> > > > the default for native builds, so we're in agreement.
> > > >
> > > > I think most of us agree that the different understanding of the
> > > > term "native build", is the cause of much of the disagreements and
> Agree, that's the main reason.
>
> > > > points of dispute on this thread. From my view point, the term "native"
> > can refer to:
> > > >
> > > > 1. what meson considers a native build, i.e. one not using a
> > > > cross-file 2. a build for a different machine architecture to the one on the
> > build
> > > > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > > > 64-bit may be considered a cross-build in this case).
> Sorry, I did not understand #2 here. Are you saying, native "means" - "a build for a different machine architecture to the one on the build machine"
>
> > > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > > things like core counts.
> > > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > > that of the build machine.
> > > >
> > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally
> > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > With the move from make to meson, that aspect was kept, but the
> > > > meaning of #1 (I think we can ignore #2) also came into play.
> > > > Finally, while for x86 architecture, the idea of #4 still held, for ARM use #3
> > is of major concern.
> Yes, #3 is the concern.
>
> At the same time, I am also interested in avoiding 'native' (or any other option) having different meaning for different architectures.
> Now that we have introduced 'soc' option for Arm platforms, we are able to achieve the builds that would be produced by #3.
> 'soc' combines both the 'platform' and 'instruction set' (as you have defined them below).
>
My thinking was that platform would be a synonym for "soc" for SOCs - it
would just seem weird to refer to x86 or PPC server systems as soc's, so I
thought "platform" a more neutral term.
Also, as I defined it above, the idea of "platform" would always encompass
the "instruction set" option too, unless the user explicitly overrode it -
hence the "auto" default value.
/Bruce
On Fri, Nov 20, 2020 at 10:15:42AM +0000, Bruce Richardson wrote:
> On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> > <snip>
> >
> > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas Monjalon
> > > wrote:
> > > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > > +supported by
> > > > > > > EAL.
> > > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > > +cross-compilation
> > > > > > > > > defaults. Set to -1 to detect the number of cores on the
> > > > > > > > > build
> > > > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > > > 0,
> > > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > > supported
> > > > > > > > > > > > +by
> > > > > > > EAL.
> > > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > > +cross-compilation defaults. Set to -1 to detect the
> > > > > > > > > > > > +number of numa nodes on the build machine.')
> > > > > > > > > > >
> > > > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > > > Second: I don't understand.
> > > > > > > > > > >
> > > > > > > > > > > It is said the default value is 0 so I expect it means
> > > > > > > > > > > automatic
> > > > > detection.
> > > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > > >
> > > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > > This was discussed quite a bit in previous versions and
> > > > > > > > > > this was te best compromise we could come up with. Having
> > > > > > > > > > a default of auto-detect is definitely not something I
> > > > > > > > > > think we should go with - just thinking of all the build
> > > > > > > > > > CI jobs running on
> > > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there was
> > > > > > > > > > value in having auto-detection, so it's set as a -1 value, which I'm
> > > ok with.
> > > > > > > > >
> > > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > > >
> > > > > > > >
> > > > > > > > There are three pieces of information which we need to convey:
> > > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > > cross-compilation defaults
> > > > > > > will be used.
> > > > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > > > >
> > > > > > > Where these positive values come from?
> > > > > > >
> > > > > >
> > > > > > From the user - they will have the option to set it to whatever
> > > > > > the like if they
> > > > > don't want to use defaults.
> > > > > >
> > > > > > > > 3. Detected values will be used for native build when the value is -1.
> > > > > > >
> > > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > > >
> > > > > >
> > > > > > I'll let Bruce explain this, but I'll just say that we wanted to
> > > > > > make the detection
> > > > > the default for native builds, so we're in agreement.
> > > > >
> > > > > I think most of us agree that the different understanding of the
> > > > > term "native build", is the cause of much of the disagreements and
> > Agree, that's the main reason.
> >
> > > > > points of dispute on this thread. From my view point, the term "native"
> > > can refer to:
> > > > >
> > > > > 1. what meson considers a native build, i.e. one not using a
> > > > > cross-file 2. a build for a different machine architecture to the one on the
> > > build
> > > > > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > > > > 64-bit may be considered a cross-build in this case).
> > Sorry, I did not understand #2 here. Are you saying, native "means" - "a build for a different machine architecture to the one on the build machine"
> >
> > > > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > > > things like core counts.
> > > > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > > > that of the build machine.
> > > > >
> > > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was originally
> > > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > > With the move from make to meson, that aspect was kept, but the
> > > > > meaning of #1 (I think we can ignore #2) also came into play.
> > > > > Finally, while for x86 architecture, the idea of #4 still held, for ARM use #3
> > > is of major concern.
> > Yes, #3 is the concern.
> >
> > At the same time, I am also interested in avoiding 'native' (or any other option) having different meaning for different architectures.
> > Now that we have introduced 'soc' option for Arm platforms, we are able to achieve the builds that would be produced by #3.
> > 'soc' combines both the 'platform' and 'instruction set' (as you have defined them below).
> >
>
> My thinking was that platform would be a synonym for "soc" for SOCs - it
> would just seem weird to refer to x86 or PPC server systems as soc's, so I
> thought "platform" a more neutral term.
> Also, as I defined it above, the idea of "platform" would always encompass
> the "instruction set" option too, unless the user explicitly overrode it -
> hence the "auto" default value.
>
Also, we could document the "instruction set" value as a 'hint', rather
than a mandatory value, which gives the arm meson.build files the option to
ignore it if it doesn't make sense to use for a given value of
"platform/soc".
/Bruce
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, November 20, 2020 11:20 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net; Ruifeng
> Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
>
> On Fri, Nov 20, 2020 at 10:15:42AM +0000, Bruce Richardson wrote:
> > On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas
> > > > > > > > > > > Monjalon
> > > > wrote:
> > > > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > > > +supported by
> > > > > > > > EAL.
> > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > +or cross-compilation
> > > > > > > > > > defaults. Set to -1 to detect the number of cores on
> > > > > > > > > > the build
> > > > > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > > > > 0,
> > > > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > > > supported
> > > > > > > > > > > > > +by
> > > > > > > > EAL.
> > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > +or cross-compilation defaults. Set to -1 to
> > > > > > > > > > > > > +detect the number of numa nodes on the build
> > > > > > > > > > > > > +machine.')
> > > > > > > > > > > >
> > > > > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > > > > Second: I don't understand.
> > > > > > > > > > > >
> > > > > > > > > > > > It is said the default value is 0 so I expect it
> > > > > > > > > > > > means automatic
> > > > > > detection.
> > > > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > > > >
> > > > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > > > This was discussed quite a bit in previous versions
> > > > > > > > > > > and this was te best compromise we could come up
> > > > > > > > > > > with. Having a default of auto-detect is definitely
> > > > > > > > > > > not something I think we should go with - just
> > > > > > > > > > > thinking of all the build CI jobs running on
> > > > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there
> > > > > > > > > > > was value in having auto-detection, so it's set as a
> > > > > > > > > > > -1 value, which I'm
> > > > ok with.
> > > > > > > > > >
> > > > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > There are three pieces of information which we need to convey:
> > > > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > > > cross-compilation defaults
> > > > > > > > will be used.
> > > > > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > > > > >
> > > > > > > > Where these positive values come from?
> > > > > > > >
> > > > > > >
> > > > > > > From the user - they will have the option to set it to
> > > > > > > whatever the like if they
> > > > > > don't want to use defaults.
> > > > > > >
> > > > > > > > > 3. Detected values will be used for native build when the value is -
> 1.
> > > > > > > >
> > > > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > > > >
> > > > > > >
> > > > > > > I'll let Bruce explain this, but I'll just say that we
> > > > > > > wanted to make the detection
> > > > > > the default for native builds, so we're in agreement.
> > > > > >
> > > > > > I think most of us agree that the different understanding of
> > > > > > the term "native build", is the cause of much of the
> > > > > > disagreements and
> > > Agree, that's the main reason.
> > >
> > > > > > points of dispute on this thread. From my view point, the term "native"
> > > > can refer to:
> > > > > >
> > > > > > 1. what meson considers a native build, i.e. one not using a
> > > > > > cross-file 2. a build for a different machine architecture to
> > > > > > the one on the
> > > > build
> > > > > > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > > > > > 64-bit may be considered a cross-build in this case).
> > > Sorry, I did not understand #2 here. Are you saying, native "means" - "a build
> for a different machine architecture to the one on the build machine"
> > >
> > > > > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > > > > things like core counts.
> > > > > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > > > > that of the build machine.
> > > > > >
> > > > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was
> > > > > > originally
> > > > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > > > With the move from make to meson, that aspect was kept, but
> > > > > > the meaning of #1 (I think we can ignore #2) also came into play.
> > > > > > Finally, while for x86 architecture, the idea of #4 still
> > > > > > held, for ARM use #3
> > > > is of major concern.
> > > Yes, #3 is the concern.
> > >
> > > At the same time, I am also interested in avoiding 'native' (or any other
> option) having different meaning for different architectures.
> > > Now that we have introduced 'soc' option for Arm platforms, we are able to
> achieve the builds that would be produced by #3.
> > > 'soc' combines both the 'platform' and 'instruction set' (as you have defined
> them below).
> > >
> >
> > My thinking was that platform would be a synonym for "soc" for SOCs -
> > it would just seem weird to refer to x86 or PPC server systems as
> > soc's, so I thought "platform" a more neutral term.
> > Also, as I defined it above, the idea of "platform" would always
> > encompass the "instruction set" option too, unless the user explicitly
> > overrode it - hence the "auto" default value.
> >
> Also, we could document the "instruction set" value as a 'hint', rather than a
> mandatory value, which gives the arm meson.build files the option to ignore it if
> it doesn't make sense to use for a given value of "platform/soc".
>
> /Bruce
Let's (or let me) put together a new patch that would decouple ISA from platform. How should we do this?
I want to send this commit separately - should I add the ISA/platform to that series or do a completely new series? I like the latter, since we're basically done with reviewing this commit.
On Fri, Nov 20, 2020 at 11:56:44AM +0000, Juraj Linkeš wrote:
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, November 20, 2020 11:20 AM
> > To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net; Ruifeng
> > Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> > vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> > jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> > (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> > ferruh.yigit@intel.com; dev@dpdk.org; nd <nd@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> > detection
> >
> > On Fri, Nov 20, 2020 at 10:15:42AM +0000, Bruce Richardson wrote:
> > > On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> > > > <snip>
> > > >
> > > > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas
> > > > > > > > > > > > Monjalon
> > > > > wrote:
> > > > > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > > > > +supported by
> > > > > > > > > EAL.
> > > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > > +or cross-compilation
> > > > > > > > > > > defaults. Set to -1 to detect the number of cores on
> > > > > > > > > > > the build
> > > > > > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > > > > > 0,
> > > > > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > > > > supported
> > > > > > > > > > > > > > +by
> > > > > > > > > EAL.
> > > > > > > > > > > > > > +Set to positive integer to overwrite per-arch
> > > > > > > > > > > > > > +or cross-compilation defaults. Set to -1 to
> > > > > > > > > > > > > > +detect the number of numa nodes on the build
> > > > > > > > > > > > > > +machine.')
> > > > > > > > > > > > >
> > > > > > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > > > > > Second: I don't understand.
> > > > > > > > > > > > >
> > > > > > > > > > > > > It is said the default value is 0 so I expect it
> > > > > > > > > > > > > means automatic
> > > > > > > detection.
> > > > > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > > > > >
> > > > > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > > > > This was discussed quite a bit in previous versions
> > > > > > > > > > > > and this was te best compromise we could come up
> > > > > > > > > > > > with. Having a default of auto-detect is definitely
> > > > > > > > > > > > not something I think we should go with - just
> > > > > > > > > > > > thinking of all the build CI jobs running on
> > > > > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there
> > > > > > > > > > > > was value in having auto-detection, so it's set as a
> > > > > > > > > > > > -1 value, which I'm
> > > > > ok with.
> > > > > > > > > > >
> > > > > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > There are three pieces of information which we need to convey:
> > > > > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > > > > cross-compilation defaults
> > > > > > > > > will be used.
> > > > > > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > > > > > >
> > > > > > > > > Where these positive values come from?
> > > > > > > > >
> > > > > > > >
> > > > > > > > From the user - they will have the option to set it to
> > > > > > > > whatever the like if they
> > > > > > > don't want to use defaults.
> > > > > > > >
> > > > > > > > > > 3. Detected values will be used for native build when the value is -
> > 1.
> > > > > > > > >
> > > > > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > > > > >
> > > > > > > >
> > > > > > > > I'll let Bruce explain this, but I'll just say that we
> > > > > > > > wanted to make the detection
> > > > > > > the default for native builds, so we're in agreement.
> > > > > > >
> > > > > > > I think most of us agree that the different understanding of
> > > > > > > the term "native build", is the cause of much of the
> > > > > > > disagreements and
> > > > Agree, that's the main reason.
> > > >
> > > > > > > points of dispute on this thread. From my view point, the term "native"
> > > > > can refer to:
> > > > > > >
> > > > > > > 1. what meson considers a native build, i.e. one not using a
> > > > > > > cross-file 2. a build for a different machine architecture to
> > > > > > > the one on the
> > > > > build
> > > > > > > machine (this largely overlaps with #1, except that e.g. 32-bit build on
> > > > > > > 64-bit may be considered a cross-build in this case).
> > > > Sorry, I did not understand #2 here. Are you saying, native "means" - "a build
> > for a different machine architecture to the one on the build machine"
> > > >
> > > > > > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > > > > > things like core counts.
> > > > > > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > > > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > > > > > that of the build machine.
> > > > > > >
> > > > > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was
> > > > > > > originally
> > > > > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > > > > With the move from make to meson, that aspect was kept, but
> > > > > > > the meaning of #1 (I think we can ignore #2) also came into play.
> > > > > > > Finally, while for x86 architecture, the idea of #4 still
> > > > > > > held, for ARM use #3
> > > > > is of major concern.
> > > > Yes, #3 is the concern.
> > > >
> > > > At the same time, I am also interested in avoiding 'native' (or any other
> > option) having different meaning for different architectures.
> > > > Now that we have introduced 'soc' option for Arm platforms, we are able to
> > achieve the builds that would be produced by #3.
> > > > 'soc' combines both the 'platform' and 'instruction set' (as you have defined
> > them below).
> > > >
> > >
> > > My thinking was that platform would be a synonym for "soc" for SOCs -
> > > it would just seem weird to refer to x86 or PPC server systems as
> > > soc's, so I thought "platform" a more neutral term.
> > > Also, as I defined it above, the idea of "platform" would always
> > > encompass the "instruction set" option too, unless the user explicitly
> > > overrode it - hence the "auto" default value.
> > >
> > Also, we could document the "instruction set" value as a 'hint', rather than a
> > mandatory value, which gives the arm meson.build files the option to ignore it if
> > it doesn't make sense to use for a given value of "platform/soc".
> >
> > /Bruce
>
> Let's (or let me) put together a new patch that would decouple ISA from platform. How should we do this?
>
> I want to send this commit separately - should I add the ISA/platform to that series or do a completely new series? I like the latter, since we're basically done with reviewing this commit.
>
Yes, let's start a new series for this. It makes the whole thing easier to
see and review.
/Bruce
<snip>
> >
> > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > 16/11/2020 10:13, Bruce Richardson:
> > > > > > > > > > On Mon, Nov 16, 2020 at 08:24:48AM +0100, Thomas
> > > > > > > > > > Monjalon
> > > wrote:
> > > > > > > > > > > 13/11/2020 15:31, Juraj Linkeš:
> > > > > > > > > > > > +option('max_lcores', type: 'integer', value: 0,
> > > > > > > > > > > > + description: 'maximum number of cores/threads
> > > > > > > > > > > > +supported by
> > > > > > > EAL.
> > > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > > +cross-compilation
> > > > > > > > > defaults. Set to -1 to detect the number of cores on the
> > > > > > > > > build
> > > > > > > > > machine.') option('max_numa_nodes', type: 'integer', value:
> > > > > > > > > 0,
> > > > > > > > > > > > + description: 'maximum number of NUMA nodes
> > > supported
> > > > > > > > > > > > +by
> > > > > > > EAL.
> > > > > > > > > > > > +Set to positive integer to overwrite per-arch or
> > > > > > > > > > > > +cross-compilation defaults. Set to -1 to detect
> > > > > > > > > > > > +the number of numa nodes on the build machine.')
> > > > > > > > > > >
> > > > > > > > > > > First comment: I don't like having so long description.
> > > > > > > > > > > Second: I don't understand.
> > > > > > > > > > >
> > > > > > > > > > > It is said the default value is 0 so I expect it
> > > > > > > > > > > means automatic
> > > > > detection.
> > > > > > > > > > > But later it is said -1 is for detection. So ?
> > > > > > > > > > >
> > > > > > > > > > Zero is for the "per-arch or cross-compilation default".
> > > > > > > > > > This was discussed quite a bit in previous versions
> > > > > > > > > > and this was te best compromise we could come up with.
> > > > > > > > > > Having a default of auto-detect is definitely not
> > > > > > > > > > something I think we should go with - just thinking of
> > > > > > > > > > all the build CI jobs running on
> > > > > > > > > > 2 or 4 core VMs! However, Juraj really felt there was
> > > > > > > > > > value in having auto-detection, so it's set as a -1
> > > > > > > > > > value, which I'm
> > > ok with.
> > > > > > > > >
> > > > > > > > > The problem is that I don't understand what 0 means.
> > > > > > > > >
> > > > > > > >
> > > > > > > > There are three pieces of information which we need to convey:
> > > > > > > > 1. The default value (0) indicates that per-arch or
> > > > > > > > cross-compilation defaults
> > > > > > > will be used.
> > > > > > > > 2. Positive integer values will be used instead of these defaults.
> > > > > > >
> > > > > > > Where these positive values come from?
> > > > > > >
> > > > > >
> > > > > > From the user - they will have the option to set it to
> > > > > > whatever the like if they
> > > > > don't want to use defaults.
> > > > > >
> > > > > > > > 3. Detected values will be used for native build when the value is
> -1.
> > > > > > >
> > > > > > > Why not detect for any native build set up with 0 (default)?
> > > > > > >
> > > > > >
> > > > > > I'll let Bruce explain this, but I'll just say that we wanted
> > > > > > to make the detection
> > > > > the default for native builds, so we're in agreement.
> > > > >
> > > > > I think most of us agree that the different understanding of the
> > > > > term "native build", is the cause of much of the disagreements
> > > > > and
> > Agree, that's the main reason.
> >
> > > > > points of dispute on this thread. From my view point, the term
> "native"
> > > can refer to:
> > > > >
> > > > > 1. what meson considers a native build, i.e. one not using a
> > > > > cross-file 2. a build for a different machine architecture to
> > > > > the one on the
> > > build
> > > > > machine (this largely overlaps with #1, except that e.g. 32-bit build
> on
> > > > > 64-bit may be considered a cross-build in this case).
> > Sorry, I did not understand #2 here. Are you saying, native "means" - "a
> build for a different machine architecture to the one on the build machine"
> >
> > > > > 3. a build tailored exactly for the build machine itself i.e. both ISA, and
> > > > > things like core counts.
> > > > > 4. a flag passed to the compiler to indicate the uarch level of the
> > > > > instruction set to be used, e.g. on x86, AVX2, AVX-512 etc., based on
> > > > > that of the build machine.
> > > > >
> > > > > Historically, IIRC, in DPDK the "RTE_MACHINE" value was
> > > > > originally
> > > > > #4 since that was it's use on x86 in the first versions of DPDK.
> > > > > With the move from make to meson, that aspect was kept, but the
> > > > > meaning of #1 (I think we can ignore #2) also came into play.
> > > > > Finally, while for x86 architecture, the idea of #4 still held,
> > > > > for ARM use #3
> > > is of major concern.
> > Yes, #3 is the concern.
> >
> > At the same time, I am also interested in avoiding 'native' (or any other
> option) having different meaning for different architectures.
> > Now that we have introduced 'soc' option for Arm platforms, we are able
> to achieve the builds that would be produced by #3.
> > 'soc' combines both the 'platform' and 'instruction set' (as you have defined
> them below).
> >
>
> My thinking was that platform would be a synonym for "soc" for SOCs - it
> would just seem weird to refer to x86 or PPC server systems as soc's, so I
> thought "platform" a more neutral term.
I am fine with using 'platform' for 'soc'.
> Also, as I defined it above, the idea of "platform" would always encompass
> the "instruction set" option too, unless the user explicitly overrode it - hence
> the "auto" default value.
Ok.
>
> /Bruce
new file mode 100644
@@ -0,0 +1,7 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2020 PANTHEON.tech s.r.o.
+
+import os
+
+print(os.cpu_count())
new file mode 100644
@@ -0,0 +1,22 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2020 PANTHEON.tech s.r.o.
+
+import ctypes
+import glob
+import os
+import subprocess
+
+if os.name == 'posix':
+ if os.path.isdir('/sys/devices/system/node'):
+ print(len(glob.glob('/sys/devices/system/node/node*')))
+ else:
+ subprocess.run(['sysctl', '-n', 'vm.ndomains'])
+
+elif os.name == 'nt':
+ libkernel32 = ctypes.windll.kernel32
+
+ count = ctypes.c_ulong()
+
+ libkernel32.GetNumaHighestNodeNumber(ctypes.pointer(count))
+ print(count.value + 1)
@@ -17,3 +17,5 @@ else
endif
map_to_win_cmd = py3 + files('map_to_win.py')
sphinx_wrapper = py3 + files('call-sphinx-build.py')
+get_cpu_count_cmd = py3 + files('get_cpu_count.py')
+get_numa_count_cmd = py3 + files('get_numa_count.py')
@@ -233,8 +233,6 @@ foreach arg: warning_flags
endforeach
# set other values pulled from the build options
-dpdk_conf.set('RTE_MAX_LCORE', get_option('max_lcores'))
-dpdk_conf.set('RTE_MAX_NUMA_NODES', get_option('max_numa_nodes'))
dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
@@ -253,6 +251,51 @@ compile_time_cpuflags = []
subdir(arch_subdir)
dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
+max_lcores = get_option('max_lcores')
+if max_lcores > 0
+ # Overwrite the default value from arch_subdir with user input
+ dpdk_conf.set('RTE_MAX_LCORE', max_lcores)
+elif max_lcores == -1
+ # Overwrite the default value with discovered values
+ if meson.is_cross_build()
+ error('Discovery of max_lcores is not supported for cross-compilation.')
+ endif
+ # Discovery makes sense only for non-cross builds
+ max_lcores = run_command(get_cpu_count_cmd).stdout().to_int()
+ min_lcores = 2
+ # DPDK must be build for at least 2 cores
+ if max_lcores < min_lcores
+ message('Found less than @0@ cores, building for @0@ cores'.format(min_lcores))
+ max_lcores = min_lcores
+ else
+ message('Found @0@ cores'.format(max_lcores))
+ endif
+ dpdk_conf.set('RTE_MAX_LCORE', max_lcores)
+endif
+
+max_numa_nodes = get_option('max_numa_nodes')
+if max_numa_nodes > 0
+ # Overwrite the default value from arch_subdir with user input
+ dpdk_conf.set('RTE_MAX_NUMA_NODES', max_numa_nodes)
+elif max_numa_nodes == -1
+ # Overwrite the default value with discovered values
+ if meson.is_cross_build()
+ error('Discovery of max_numa_nodes not supported for cross-compilation.')
+ endif
+ # Discovery makes sense only for non-cross builds
+ max_numa_nodes = run_command(get_numa_count_cmd).stdout().to_int()
+ message('Found @0@ numa nodes'.format(max_numa_nodes))
+ dpdk_conf.set('RTE_MAX_NUMA_NODES', max_numa_nodes)
+endif
+
+# check that cpu and numa count is set and error out if it's not set
+if not dpdk_conf.has('RTE_MAX_LCORE')
+ error('Number of cores not specified.')
+endif
+if not dpdk_conf.has('RTE_MAX_NUMA_NODES')
+ error('Number of numa nodes not specified.')
+endif
+
# set the install path for the drivers
dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
@@ -57,3 +57,5 @@ else
endif
dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
+dpdk_conf.set('RTE_MAX_LCORE', 128)
+dpdk_conf.set('RTE_MAX_NUMA_NODES', 4)
@@ -24,10 +24,10 @@ option('machine', type: 'string', value: 'native',
description: 'set the target machine type. Set to generic for a build usable on all machines of the build machine architecture, set to native to let the compiler pick the architecture of the build machine.')
option('max_ethports', type: 'integer', value: 32,
description: 'maximum number of Ethernet devices')
-option('max_lcores', type: 'integer', value: 128,
- description: 'maximum number of cores/threads supported by EAL')
-option('max_numa_nodes', type: 'integer', value: 4,
- description: 'maximum number of NUMA nodes supported by EAL')
+option('max_lcores', type: 'integer', value: 0,
+ description: 'maximum number of cores/threads supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of cores on the build machine.')
+option('max_numa_nodes', type: 'integer', value: 0,
+ description: 'maximum number of NUMA nodes supported by EAL. Set to positive integer to overwrite per-arch or cross-compilation defaults. Set to -1 to detect the number of numa nodes on the build machine.')
option('enable_trace_fp', type: 'boolean', value: false,
description: 'enable fast path trace points.')
option('tests', type: 'boolean', value: true,