> -----Original Message-----
> From: Bruce Richardson <[email protected]>
> Sent: Tuesday, October 13, 2020 5:48 PM
> To: Juraj Linkeš <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [RFC PATCH v2 3/6] build: automatic NUMA and cpu counts
> detection
> 
> On Tue, Oct 13, 2020 at 04:54:39PM +0200, Juraj Linkeš wrote:
> > The build machine's number of cpus and numa nodes vary, resulting in
> > mismatched counts of RTE_MAX_LCORE and RTE_MAX_NUMA_NODES for
> many
> > builds. Automatically discover the host's numa and cpu counts to
> > remove this mismatch for native builds. Use current defaults for cross 
> > builds.
> > Leave users the option to override both if the specify a non-zero
> > amount on the command line.
> >
> > Signed-off-by: Juraj Linkeš <[email protected]>
> > ---
> >  buildtools/get_cpu_count.py  |  7 +++++++
> > buildtools/get_numa_count.py | 17 +++++++++++++++++
> >  buildtools/meson.build       |  2 ++
> >  config/meson.build           | 20 ++++++++++++++++++--
> >  meson_options.txt            |  8 ++++----
> >  5 files changed, 48 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..386f85f8b
> > --- /dev/null
> > +++ b/buildtools/get_cpu_count.py
> > @@ -0,0 +1,7 @@
> > +#!/usr/bin/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..f2ad35532
> > --- /dev/null
> > +++ b/buildtools/get_numa_count.py
> > @@ -0,0 +1,17 @@
> > +#!/usr/bin/python3
> > +# SPDX-License-Identifier: BSD-3-Clause # Copyright (c) 2020
> > +PANTHEON.tech s.r.o.
> > +
> > +import ctypes
> > +import glob
> > +import os
> > +
> > +if os.name == 'posix':
> > +    print(len(glob.glob('/sys/devices/system/node/node*')))
> 
> FreeBSD also reports it's os.name as posix, but doesn't have a /sys/devices 
> path
> to query.
> 

Do you know how do I get numa count on FreeBDS? I don't want to use the numa 
library if we can to avoid unneccesary dependencies. Do we need to cover some 
other cases other than FreeBDS, other Linux and Windows?

> > +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
> > 4bd65d98e..84c31c8e7 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -226,8 +226,24 @@ 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'))
> > +max_lcores = get_option('max_lcores') if max_lcores == 0
> > +   if meson.is_cross_build()
> > +           max_lcores = 4
> 
> This seems a rather low number to default to. I would suspect in this case you
> might be better just to error out, i.e. require that the max_lcores value is 
> given
> when cross-compiling.
> 
> > +   else
> > +           max_lcores = run_command(get_cpu_count_cmd).stdout()
> > +   endif
> > +endif
> > +dpdk_conf.set('RTE_MAX_LCORE', max_lcores) max_numa_nodes =
> > +get_option('max_numa_nodes') if max_numa_nodes == 0
> > +   if meson.is_cross_build()
> > +           max_numa_nodes = 128
> 
> Same here, I think it's better to force the user to specify a value.
> BTW: did you unintentionally switch the max cores and max numa nodes values
> in this patch?
> 

Yes, the values are switched. Forcing the user to specify these might be tricky 
- we want to specify cpu and numa counts in cross files and this check happens 
before the values from cross files are used (in another patch I'm asking about 
having defaults in config/arm/meson.build and if we don't have defaults we 
won't want them there as well. The remaning question would be what get priority 
- command line options or cross file options. I'm leaning towards command line 
options). I'll think about switching the order.

> > +   else
> > +           max_numa_nodes =
> run_command(get_numa_count_cmd).stdout()
> > +   endif
> > +endif
> > +dpdk_conf.set('RTE_MAX_NUMA_NODES', 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'))
> > diff --git a/meson_options.txt b/meson_options.txt index
> > 9bf18ab6b..60a949fca 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')  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.
> > +Value 0 means the number of cpus on the host will be used')
> option('max_numa_nodes', type: 'integer', value: 0,
> > +   description: 'maximum number of NUMA nodes supported by EAL. Value
> 0
> > +means the number of numa nodes on the host will be used')
> >  option('enable_trace_fp', type: 'boolean', value: false,
> >     description: 'enable fast path trace points.')  option('tests',
> > type: 'boolean', value: true,
> > --
> > 2.20.1
> >

Reply via email to