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

[Burakov, Anatoly]

On 29-May-19 9:11 PM, David Marchand wrote:
> On Wed, May 29, 2019 at 6:31 PM Anatoly Burakov <anatoly.burakov at 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.

> 
> 
> driver are of particular interest, because they're using
>> the shared memory config in a way that i find confusing.
>>
> 
> I thought the same when I looked at it before.
> Hopefully the ena maintainers will enlight us :-).
> 
> 
> I tried to implement the equivalent changes as well as
>> i could, but since the code doesn't make any sense to me,
>> i would really like to request help from ENA maintainers.
>>
>> Everything else should be pretty straightforward.
>>
> 
> We are missing the deprecation notice removal at the end of the series and
> a note in the release notes.

Will add. Making into V1 deadline was higher priority :D

> 
> Thanks Anatoly!
> 
> 


-- 
Thanks,
Anatoly