From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 1429EA045E for ; Thu, 30 May 2019 10:08:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 5A9021B944; Thu, 30 May 2019 10:07:49 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id D67DA5680 for ; Thu, 30 May 2019 10:07:47 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 30 May 2019 01:07:46 -0700 X-ExtLoop1: 1 Received: from itopinsk-mobl.ccr.corp.intel.com (HELO [10.252.14.83]) ([10.252.14.83]) by fmsmga006.fm.intel.com with ESMTP; 30 May 2019 01:07:45 -0700 To: David Marchand Cc: dev , Stephen Hemminger , Thomas Monjalon References: From: "Burakov, Anatoly" Message-ID: Date: Thu, 30 May 2019 09:07:44 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 00/25] Make shared memory config non-public X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 29-May-19 9:11 PM, David Marchand wrote: > On Wed, May 29, 2019 at 6:31 PM Anatoly Burakov > 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