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

[Konstantin Ananyev <konstantin.ananyev@huawei.com>]

> 
> > 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.
I suppose for 23.11 we have not much choice but accept that patch as it is.
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.    

> 
> 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. :-)