Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Thomas Monjalon <thomas@monjalon.net>]

Please, what is the conclusion here?


18/09/2020 07:47, Dharmik Thakkar:
> 
> > On Sep 17, 2020, at 4:56 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> > 
> > 
> > 
> >> -----Original Message-----
> >> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> >> Sent: Friday, September 4, 2020 7:44 AM
> >> To: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> >> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev <dev@dpdk.org>;
> >> nd <nd@arm.com>
> >> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> >> 
> >> 
> >> 
> >>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
> >> <stephen@networkplumber.org> wrote:
> >>> 
> >>> On Thu, 3 Sep 2020 06:20:17 +0000
> >>> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >>> 
> >>>>> -----Original Message-----
> >>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>>>> Sent: Wednesday, August 26, 2020 6:56 AM
> >>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
> >>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
> >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >>>>> programmatically
> >>>>> 
> >>>>> 
> >>>>> 
> >>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com>
> >> wrote:
> >>>>>> 
> >>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> >>>>> <dharmik.thakkar@arm.com> wrote:
> >>>>>>> 
> >>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
> >>>>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and
> >>>>>>> part number but different number of CPUs.
> >>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set
> >>>>>>> in
> >>>>>>> meson_options.txt)
> >>>>>>> 
> >>>>>>> Use python script to find max lcore when using native build to
> >>>>>>> correctly set RTE_MAX_LCORE.
> >>>>>> 
> >>>>>> We may need to build on the native arm64 machine and use it on
> >>>>>> another
> >>>>>> arm64 machine(Just like x86).
> >>>>>> So I think, at least for default config(which will be used by
> >>>>>> distribution) to support max
> >>>>>> lcores as fixed. I am not sure this patch changes those aspects or
> >>>>>> not? Please check.
> >>>>> 
> >>>>> This patch does *not* affect ‘default’ build type and cross-compilation.
> >>>>> 
> >>>>>> 
> >>>>>>> 
> >>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >>>>>>> ---
> >>>>>>> config/get_max_lcores.py | 13 +++++++++++++
> >>>>>>> config/meson.build       | 13 ++++++++++++-
> >>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
> >>>>>>> 100755 config/get_max_lcores.py
> >>>>>>> 
> >>>>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
> >>>>>>> new file mode 100755 index 000000000000..ebf1c7efdadd
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/config/get_max_lcores.py
> >>>>>>> @@ -0,0 +1,13 @@
> >>>>>>> +#!/usr/bin/python3
> >>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
> >>>>>>> +Limited
> >>>>>>> +
> >>>>>>> +import os
> >>>>>>> +
> >>>>>>> +max_lcores = []
> >>>>>>> +
> >>>>>>> +nCPU = os.cpu_count()
> >>>>>>> +
> >>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> >>>>>>> +
> >>>>>>> +print(' '.join(max_lcores))
> >>>>>>> diff --git a/config/meson.build b/config/meson.build index
> >>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
> >>>>>>> --- a/config/meson.build
> >>>>>>> +++ b/config/meson.build
> >>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved
> >>>>>>> memory
> >>>>> areas
> >>>>>>>      dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> >>>>>>> 
> >>>>>>> -
> >>>>>>> compile_time_cpuflags = []
> >>>>>>> subdir(arch_subdir)
> >>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> >>>>>>> ','.join(compile_time_cpuflags))
> >>>>>>> 
> >>>>>>> +# set max lcores
> >>>>>>> +if machine != 'default' and not meson.is_cross_build()
> >>>>>>> +       # The script returns max lcores
> >>>>>>> +       params = files('get_max_lcores.py')
> >>>>>>> +       cmd_out = run_command(params)
> >>>> 
> >>>> Have you considered running just a shell command, such as "nproc --all"?
> >>> 
> >>> Is this really a good idea?
> >>> For real distributions and NFV products, the build and runtime
> >>> environment will usually be different even if on same CPU architecture.
> >>> 
> >>> In many cases there maybe a huge build machine (128 CPU) or in a
> >>> container (reported as single cpu) even if not doing cross build.
> >> 
> >> That’s a great point, Stephen. IMO, this patch is useful when building and
> >> running natively.
> >> For all other purposes (like the ones you mentioned), do you think it is a good
> >> idea to set RTE_MAX_LCORE using -Dmax_lcores?
> > 
> > We should only use this native builds, as that would be consistent with the current meson build philosophy of "meson figuring as much as possible on its own". Any build other than native implies the user wants to deviate from the build machine.
> > 
> 
> The MIDR value-based probing doesn’t quite work well for Arm IP (currently being discussed on this patch: https://patches.dpdk.org/patch/76981/).
> 
> > One way to do this automatic core count is when max_lcores=0 (0 would have the special meaning of "figure core count automatically"). We can set that as default in meson_option.txt and then users will have the ability to set it to whatever they want, even for native builds. What do you think?
> > 
> 
> Yes, agreed.
> 
> > Currently the -Dmax_lcores option doesn't work for arm builds (the value of RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch tries to address this, but still, we need to be mindful of that.
> > 
> > Juraj