From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 1C27E5946 for ; Fri, 3 Jun 2016 16:27:14 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 03 Jun 2016 07:17:05 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,412,1459839600"; d="scan'208";a="980229861" Received: from dhunt5-mobl.ger.corp.intel.com (HELO [10.237.221.24]) ([10.237.221.24]) by fmsmga001.fm.intel.com with ESMTP; 03 Jun 2016 07:17:04 -0700 To: Olivier MATZ , dev@dpdk.org References: <1464797998-76690-1-git-send-email-david.hunt@intel.com> <1464874043-67467-1-git-send-email-david.hunt@intel.com> <1464874043-67467-3-git-send-email-david.hunt@intel.com> <575177FD.4020908@6wind.com> Cc: viktorin@rehivetech.com, jerin.jacob@caviumnetworks.com From: "Hunt, David" Message-ID: <57519160.2020808@intel.com> Date: Fri, 3 Jun 2016 15:17:04 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <575177FD.4020908@6wind.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v7 2/5] mempool: remove rte_ring from rte_mempool struct X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Jun 2016 14:27:15 -0000 On 6/3/2016 1:28 PM, Olivier MATZ wrote: > > On 06/02/2016 03:27 PM, David Hunt wrote: >> Now that we're moving to an external mempoool handler, which >> uses a void *pool_data as a pointer to the pool data, remove the >> unneeded ring pointer from the mempool struct. >> >> Signed-off-by: David Hunt >> --- >> app/test/test_mempool_perf.c | 1 - >> lib/librte_mempool/rte_mempool.h | 1 - >> 2 files changed, 2 deletions(-) >> >> diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c >> index cdc02a0..091c1df 100644 >> --- a/app/test/test_mempool_perf.c >> +++ b/app/test/test_mempool_perf.c >> @@ -161,7 +161,6 @@ per_lcore_mempool_test(__attribute__((unused)) void *arg) >> n_get_bulk); >> if (unlikely(ret < 0)) { >> rte_mempool_dump(stdout, mp); >> - rte_ring_dump(stdout, mp->ring); >> /* in this case, objects are lost... */ >> return -1; >> } >> diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h >> index a6b28b0..c33eeb8 100644 >> --- a/lib/librte_mempool/rte_mempool.h >> +++ b/lib/librte_mempool/rte_mempool.h >> @@ -204,7 +204,6 @@ struct rte_mempool_memhdr { >> */ >> struct rte_mempool { >> char name[RTE_MEMPOOL_NAMESIZE]; /**< Name of mempool. */ >> - struct rte_ring *ring; /**< Ring to store objects. */ >> union { >> void *pool_data; /**< Ring or pool to store objects */ >> uint64_t pool_id; /**< External mempool identifier */ >> > Sorry if I missed it in previous discussions, but I don't really > see the point of having this in a separate commit, as the goal > of the previous commit is to replace the ring by configurable ops. > > Moreover, after applying only the previous commit, the > call to rte_ring_dump(stdout, mp->ring) would probably crash > sine ring is NULL. > > I think this comment also applies to the next commit. Splitting > between functionalities is good, but in this case I think the 3 > commits are linked together, and it should not break compilation > or tests to facilitate the git bisect. > > > Regards, > Olivier Yes. Originally there was a lot of discussion to split out the bigger patch, which I did, and it was easier to review, but I think that now we're (very) close to final revision, I can merge those three back into one. Thanks, Dave.