Re: [dpdk-dev] [PATCH 00/25] Make shared memory config non-public

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, May 30, 2019 at 09:07:44AM +0100, Burakov, Anatoly wrote:
> On 29-May-19 9:11 PM, David Marchand wrote:
> > On Wed, May 29, 2019 at 6:31 PM Anatoly Burakov <anatoly.burakov@intel.com>
> > wrote:
> > 
> > > This patchset removes the shared memory config from public
> > > API, and replaces all usages of said config with new API
> > > calls.
> > > 
> > > The patchset is mostly a search-and-replace job and should
> > > be pretty easy to review. However, the changes to ENA
> > > 
> > 
> > I went and did the same job with some scripts.
> > 
> > Not sure you really need to split in all those patches.
> > We are not going to backport this.
> 
> The "separate commits" thing is made for the benefit of reviewers, not
> backporters. In my experience it's much easier to get a maintainer to review
> a smaller patch than it is to look through a wall of irrelevant changes.
> 
> That said, for trivial changes such as these, maybe this is indeed
> unnecessary.
> 
> > Some changes are mixed, the kni changes are in the hash: patch.
> 
> Oops, will fix, thanks for pointing it out!
> 
> > 
> > 
> > I spotted a missed qlock in :
> > lib/librte_eal/common/eal_common_tailqs.c:
> >   rte_rwlock_read_lock(&mcfg->qlock);
> > lib/librte_eal/common/eal_common_tailqs.c:
> >   rte_rwlock_read_unlock(&mcfg->qlock);
> > 
> > 
> > On the names of the functions, could we have something shorter ?
> > The prefix rte_eal_mcfg_ is not necessary from my pov.
> 
> I can drop the mcfg, but IMO all of these locking functions should be kept
> under one namespace, and rte_eal_ is too broad.
> 

I think most/all developers are aware that memory is part of eal, so
rte_mcfg_ prefix (or rte_memcfg) might work.