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 ABBE3A04DD; Thu, 19 Nov 2020 15:51:14 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4D4ED3B5; Thu, 19 Nov 2020 15:51:12 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 5840A98 for ; Thu, 19 Nov 2020 15:51:10 +0100 (CET) IronPort-SDR: KwNTB0boVzjdo3nd8VNUJTckbFaxGggXxgJ/VIKo6FPUBR440sfOD+As1n1cCNkzUca63Ew4E1 yuoGfTtfDtcA== X-IronPort-AV: E=McAfee;i="6000,8403,9809"; a="150565958" X-IronPort-AV: E=Sophos;i="5.77,490,1596524400"; d="scan'208";a="150565958" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Nov 2020 06:51:07 -0800 IronPort-SDR: Bfft/Ow7kOwTTxRfSkLpNeeFK+nK3gdM95e2cU6Ou1tw080+vqDuXQrKAZv3BgDf9SzQrHoJyD QdVbBATO/rCg== X-IronPort-AV: E=Sophos;i="5.77,490,1596524400"; d="scan'208";a="476843264" Received: from bricha3-mobl.ger.corp.intel.com ([10.251.82.136]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 19 Nov 2020 06:51:04 -0800 Date: Thu, 19 Nov 2020 14:51:01 +0000 From: Bruce Richardson To: Juraj =?utf-8?Q?Linke=C5=A1?= Cc: Thomas Monjalon , "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" Message-ID: <20201119145101.GA1835@bricha3-MOBL.ger.corp.intel.com> References: <1605267483-13167-1-git-send-email-juraj.linkes@pantheon.tech> <11763925.GOiJPSdkav@thomas> <2337679.hKZaPKL2be@thomas> <20201119121947.GC1829@bricha3-MOBL.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v12 09/14] 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 Thu, Nov 19, 2020 at 01:57:59PM +0000, Juraj Linkeš wrote: > > > > -----Original Message----- > > From: Bruce Richardson > > Sent: Thursday, November 19, 2020 1:20 PM > > To: Juraj Linkeš > > Cc: Thomas Monjalon ; 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 > > > > Sent: Wednesday, November 18, 2020 3:43 PM > > > > To: Bruce Richardson ; Juraj Linkeš > > > > > > > > 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 > > > > > > 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.