Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts detection

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, November 20, 2020 11:20 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net; Ruifeng
> Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v12 09/14] build: optional NUMA and cpu counts
> detection
> 
> On Fri, Nov 20, 2020 at 10:15:42AM +0000, Bruce Richardson wrote:
> > On Fri, Nov 20, 2020 at 04:33:12AM +0000, Honnappa Nagarahalli wrote:
> > > <snip>
> > >
> > > > > > > > 18/11/2020 15:19, Juraj Linkeš:
> > > > > > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > > > > > 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
> > > Agree, that's the main reason.
> > >
> > > > > > 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).
> > > Sorry, I did not understand #2 here. Are you saying, native "means" - "a build
> for a different machine architecture to the one on the build machine"
> > >
> > > > > > 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.
> > > Yes, #3 is the concern.
> > >
> > > At the same time, I am also interested in avoiding 'native' (or any other
> option) having different meaning for different architectures.
> > > Now that we have introduced 'soc' option for Arm platforms, we are able to
> achieve the builds that would be produced by #3.
> > > 'soc' combines both the 'platform' and 'instruction set' (as you have defined
> them below).
> > >
> >
> > My thinking was that platform would be a synonym for "soc" for SOCs -
> > it would just seem weird to refer to x86 or PPC server systems as
> > soc's, so I thought "platform" a more neutral term.
> > Also, as I defined it above, the idea of "platform" would always
> > encompass the "instruction set" option too, unless the user explicitly
> > overrode it - hence the "auto" default value.
> >
> Also, we could document the "instruction set" value as a 'hint', rather than a
> mandatory value, which gives the arm meson.build files the option to ignore it if
> it doesn't make sense to use for a given value of "platform/soc".
> 
> /Bruce

Let's (or let me) put together a new patch that would decouple ISA from platform. How should we do this?

I want to send this commit separately - should I add the ISA/platform to that series or do a completely new series? I like the latter, since we're basically done with reviewing this commit.