RE: [PATCH] config/x86: config support for AMD EPYC processors

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Konstantin Ananyev [mailto:konstantin.ananyev@huawei.com]
> Sent: Tuesday, 7 November 2023 14.14
> 
> >
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Monday, 6 November 2023 22.05
> > >
> > > 17/10/2023 12:27, Morten Brørup:
> > > > > >> From: Tummala, Sivaprasad <Sivaprasad.Tummala@amd.com>
> > > > > >>> From: David Marchand <david.marchand@redhat.com>
> > > > > >>> On Mon, Sep 25, 2023 at 5:11 PM Sivaprasad Tummala
> > > > > >>>> From: Sivaprasad Tummala <Sivaprasad.Tummala@amd.com>
> > > > > >>>>
> > > > > >>>> By default, max lcores are limited to 128 for x86
> platforms.
> > > > > >>>> On AMD EPYC processors, this limit needs to be increased
> to
> > > > > leverage
> > > > > >>>> all the cores.
> > > > > >>>>
> > > > > >>>> The patch adjusts the limit specifically for native
> > > compilation on
> > > > > >>>> AMD EPYC CPUs.
> > > > > >>>>
> > > > > >>>> Signed-off-by: Sivaprasad Tummala
> <Sivaprasad.Tummala@amd.com>
> > > > > >>>
> > > > > >>> This patch is a revamp of
> > > > > >>>
> > > > > >>
> > > > >
> > >
> http://inbox.dpdk.org/dev/BY5PR12MB3681C3FC6676BC03F0B42CCC96789@BY5PR
> > > > > >>> 12MB3681.namprd12.prod.outlook.com/
> > > > > >>> for which a discussion at techboard is supposed to have
> taken
> > > place.
> > > > > >>> But I didn't find a trace of it.
> > > > > >>>
> > > > > >>> One option that had been discussed in the previous thread
> was
> > > to
> > > > > >>> increase the max number of cores for x86.
> > > > > >>> I am unclear if this option has been properly
> > > evaluated/debatted.
> > > >
> > > > Here are the minutes from the previous techboard discussions:
> > > > [1]: http://inbox.dpdk.org/dev/YZ43U36bFWHYClAi@platinum/
> > > > [2]:
> http://inbox.dpdk.org/dev/20211202112506.68acaa1a@hermes.local/
> > > >
> > > > AFAIK, there has been no progress with dynamic max_lcores, so I
> guess
> > > the techboard's conclusion still stands:
> > > >
> > > > There is no identified use-case where a single application
> requires
> > > more than 128 lcores. If a case a use-case exists for a single
> > > application that uses more than 128 lcores, the TB is ok to update
> the
> > > default config value.
> > > >
> > > > > >>>
> > > > > >>> Can the topic be brought again at techboard?
> > > > > >>
> > > > > >> Hi David,
> > > > > >>
> > > > > >> The patch is intended to detect AMD platforms and enable all
> CPU
> > > > > cores by default
> > > > > >> on native builds.
> > > >
> > > > This is done on native ARM builds, so why not on native X86
> builds
> > > too?
> > > >
> > > > > >>
> > > > > >> As an optimization for memory footprint, users can override
> this
> > > by
> > > > > specifying "-
> > > > > >> Dmax_lcores" option based on DPDK lcores required for their
> > > usecases.
> > > > > >>
> > > > > >> Sure, will request to add this topic for discussion at
> > > techboard.
> > >
> > > This is the summary of the techboard meeting:
> > > (see https://mails.dpdk.org/archives/dev/2023-October/279672.html)
> > >
> > > - There is some asks for more than 128 worker cores
> > > - Discussion about generally increasing the default max core count
> and
> > > trade-offs with memory consumption but this is longer term issue
> >
> > The distros are currently satisfied with the 128 cores default, so
> the decision here was: Leave the 128 cores default as is, for now.
> >
> > Any long term improvements regarding memory consumption of many-core
> systems are not relevant for this patch.
> >
> > > - Acceptance for the direction of this patch in the short term
> >
> > With the twist that it must work for cross compile. It is the
> properties of the target CPU that matter, not the properties of the
> host
> > CPU. (Although the build may be "native", i.e. the target CPU is the
> same as the host CPU, it is still the target CPU that matters.)
> >
> > > - Details of whether it should be for EPYC only or x86 to be
> figured
> > > out
> > > on mailing list
> >
> > I think this is obvious...
> >
> > ARM already provides ARM CPU specific optimizations.
> > AMD should be allowed to provide AMD CPU specific optimizations too.
> > Intel can also provide Intel CPU specific optimizations.
> 
> I suppose no-one stopping AMD/Intel/ARM to provide their CPU specific
> optimizations.
> Though as end-user, my preference would be to have one generic build
> (machine=default) that would work ok
> on all cpus for given architecture (let say x86) instead of
> maintaining/testing dozens of different flavors.

Agree. Machine specific builds should be explicitly specified. I consider "native" a variant of explicitly specifying the target machine.

> I suppose for 23.11 we have not much choice but accept that patch as it
> is.

No. They agreed in the techboard meeting to rework it for cross compile.

> Though I think in future (24.11?) it would be ideal to make
> RTE_MAX_LCORE a runtime parameter and
> remove it from public API structs.

It is probably more difficult than it sounds to fully remove RTE_MAX_LCORE.

However, if we could make it mostly a runtime parameter, and only keep RTE_MAX_LCORE in arrays of small structures, we could increase it to some very large (but still sane) value, like 4096 or whatever number of CPU cores can be found in the largest system out there.

> 
> >
> > And if some of these optimizations are rooted in the same criteria,
> they should be shared across the relevant CPU architectures. We
> > follow this principle in the source code files, and the principle
> also applies to the build files.
> >
> > >
> > > So now let's figure out the details please.
> > > Suggestions?
> >
> > Suggestions provided inline above. :-)