[dpdk-dev] [PATCH v8 10/14] build: optional NUMA and cpu counts detection

[Juraj Linkeš]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> Sent: Monday, November 9, 2020 11:53 PM
> To: Juraj Linkeš <juraj.linkes at pantheon.tech>; bruce.richardson at intel.com;
> Ruifeng Wang <Ruifeng.Wang at arm.com>; Phil Yang <Phil.Yang at arm.com>;
> vcchunga at amazon.com; Dharmik Thakkar <Dharmik.Thakkar at arm.com>;
> jerinjacobk at gmail.com; hemant.agrawal at nxp.com; Ajit Khaparde
> (ajit.khaparde at broadcom.com) <ajit.khaparde at broadcom.com>;
> ferruh.yigit at intel.com; aconole at redhat.com
> Cc: dev at dpdk.org; nd <nd at arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli at arm.com>; nd <nd at arm.com>
> Subject: RE: [PATCH v8 10/14] build: optional NUMA and cpu counts detection
> 
> <snip>
> 
> >
> > 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 at pantheon.tech>
> Few nits, otherwise looks fine
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at 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
> >
> > diff --git a/buildtools/get_cpu_count.py b/buildtools/get_cpu_count.py
> > new file mode 100644 index 000000000..b269d557b
> > --- /dev/null
> > +++ b/buildtools/get_cpu_count.py
> > @@ -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())
> > diff --git a/buildtools/get_numa_count.py
> > b/buildtools/get_numa_count.py new file mode 100644 index
> > 000000000..be73c5c3f
> > --- /dev/null
> > +++ b/buildtools/get_numa_count.py
> > @@ -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)
> > diff --git a/buildtools/meson.build b/buildtools/meson.build index
> > 04808dabc..925e733b1 100644
> > --- a/buildtools/meson.build
> > +++ b/buildtools/meson.build
> > @@ -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')
> > diff --git a/config/meson.build b/config/meson.build index
> > c7f7aa6e2..2974f7f6f 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -231,8 +231,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')) @@
> > -251,6 +249,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('Discovered values (user setting -1) are not supported
> > be used when cross-compiling.')
> Suggest something like:
> 'Discovery of max_lcore value 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('Discovered values (user setting -1) are not supported
> > be used when cross-compiling.')
> Suggest something like:
> 'Discovery of max_numa_nodes value not supported for cross-compilation'
> 

This is a bit better, thanks.

> > +	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)
> >
> > diff --git a/config/x86/meson.build b/config/x86/meson.build index
> > 31bfa63b1..4989d47f3 100644
> > --- a/config/x86/meson.build
> > +++ b/config/x86/meson.build
> > @@ -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)
> > diff --git a/meson_options.txt b/meson_options.txt index
> > ce23289e3..e1059fb16 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -26,10 +26,10 @@ option('machine', type: 'string', value: 'native',
> >  	description: 'set the target machine type. Set to generic for a
> > build usable on most machines of the build machine architecture, set
> > to native to let the compiler choose the best fit for 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 use number of cores on the build machine.')
> > option('max_numa_nodes',
>         ^^^ detect? will be aligned with the comment above
> 

Ok, that's also better.

> > 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 use number of numa nodes on the build machine.')
>                           ^^^ detect? will be aligned with the comment above
> 
> >  option('enable_trace_fp', type: 'boolean', value: false,
> >  	description: 'enable fast path trace points.')  option('tests', type:
> > 'boolean', value: true,
> > --
> > 2.20.1