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 065C1A045E for ; Thu, 30 May 2019 12:16:02 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A20461B94D; Thu, 30 May 2019 12:16:01 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 5CFB34CA6 for ; Thu, 30 May 2019 12:16:00 +0200 (CEST) X-Amp-Result: UNSCANNABLE 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 03:15:59 -0700 X-ExtLoop1: 1 Received: from bricha3-mobl.ger.corp.intel.com ([10.237.221.64]) by fmsmga006.fm.intel.com with SMTP; 30 May 2019 03:15:56 -0700 Received: by (sSMTP sendmail emulation); Thu, 30 May 2019 11:15:56 +0100 Date: Thu, 30 May 2019 11:15:56 +0100 From: Bruce Richardson To: "Burakov, Anatoly" Cc: David Marchand , dev , Stephen Hemminger , Thomas Monjalon Message-ID: <20190530101555.GB1107@bricha3-MOBL.ger.corp.intel.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.11.4 (2019-03-13) 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 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 > > 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.