From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BCD8CA04DD; Wed, 28 Oct 2020 16:04:40 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 152DBCB7A; Wed, 28 Oct 2020 16:04:38 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 3B3BBCB53 for ; Wed, 28 Oct 2020 16:04:35 +0100 (CET) IronPort-SDR: KYCyn5Q7bTOLMViI8JCMxRUiC5M+7acy3mF51dwtFMs+2ASv9QdP0nPCsptDkqFqgPI2elgcEw ZE68YSG4uF7Q== X-IronPort-AV: E=McAfee;i="6000,8403,9788"; a="147557441" X-IronPort-AV: E=Sophos;i="5.77,426,1596524400"; d="scan'208";a="147557441" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Oct 2020 08:04:31 -0700 IronPort-SDR: uGaKAsRbuW+/VHb70v4oOXhqb2GLiJBEXqih3M7CkuOlmj1mDiunMWhoZNQqZJr5Qz7MFFC18G DSf6qTsvqJzQ== X-IronPort-AV: E=Sophos;i="5.77,426,1596524400"; d="scan'208";a="536259446" Received: from bricha3-mobl.ger.corp.intel.com ([10.255.205.158]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 28 Oct 2020 08:04:25 -0700 Date: Wed, 28 Oct 2020 15:04:20 +0000 From: Bruce Richardson To: Juraj =?utf-8?Q?Linke=C5=A1?= 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, dev@dpdk.org Message-ID: <20201028150420.GB1634@bricha3-MOBL.ger.corp.intel.com> References: <1603464488-25493-1-git-send-email-juraj.linkes@pantheon.tech> <1603893845-5736-1-git-send-email-juraj.linkes@pantheon.tech> <1603893845-5736-9-git-send-email-juraj.linkes@pantheon.tech> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1603893845-5736-9-git-send-email-juraj.linkes@pantheon.tech> Subject: Re: [dpdk-dev] [PATCH v5 08/11] build: optional NUMA and cpu counts detection X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Oct 28, 2020 at 03:04:02PM +0100, Juraj Linkeš wrote: > 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š > --- > buildtools/get_cpu_count.py | 7 ++++++ > buildtools/get_numa_count.py | 22 +++++++++++++++++ > buildtools/meson.build | 2 ++ > config/meson.build | 48 ++++++++++++++++++++++++++++++++++-- > config/x86/meson.build | 2 ++ > meson_options.txt | 8 +++--- > 6 files changed, 83 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 I think for DPDK python scripts we standardized on "/usr/bin/env python3" [Someone please correct me if I'm wrong here!] > +# 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..e7f3de6b0 > --- /dev/null > +++ b/buildtools/get_numa_count.py > @@ -0,0 +1,22 @@ > +#!/usr/bin/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', 'vm.ndomains']) > + I just realised that by default sysctl outputs the name of the element as well as the value, which is not what we want. Using "-n" flag suppresses the name, so please add that to the command. [Sorry for not spotting this in previous versions]. > +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..a51000ab2 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,52 @@ compile_time_cpuflags = [] > subdir(arch_subdir) > dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags)) > > +max_lcores = get_option('max_lcores') > +max_numa_nodes = get_option('max_numa_nodes') > +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 not meson.is_cross_build() I'd suggest changing this to the inverse and erroring out, since I assume that having -1 for cross-compilation should be an error? if meson.is_cross_build() error('....') endif Thereafter you save a level of indentation for the rest of the block. > + # 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 Even 2 seems rather low. I'd personally look to set the minimum at 4. > + 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 > +endif > + > +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 not meson.is_cross_build() > + # 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 > +endif > + > +# check that cpu and numa count is set in cross builds > +# and error out if it's not set > +if meson.is_cross_build() > + if not dpdk_conf.has('RTE_MAX_LCORE') > + error('Number of cores for cross build not specified in @0@ subdir (e.g. in a cross-file) nor on the cmdline'.format(arch_subdir)) > + endif > + if not dpdk_conf.has('RTE_MAX_NUMA_NODES') > + error('Number of numa nodes for cross build not specified in @0@ subdir (e.g. in a cross-file) nor on the cmdline'.format(arch_subdir)) > + endif > +endif > + Not sure you need the is_cross_build bit, since if we don't have RTE_MAX_LCORES set at all, then it's an error too. It just so happens that the code paths to this point prevent that from ever happening. Is the intention here that the subdir meson.build files will just literally take the value from the cross-file, or are you doing other processing in the ARM case, e.g. to compute it based on other info. If it's just the former where it's always read from the cross-files, I'd have the reading done in this file (above the other assignment work), since it would be architecture agnostic. > # set the install path for the drivers > dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path) >