From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67]) by dpdk.org (Postfix) with ESMTP id 10A77559C for ; Mon, 8 Feb 2016 12:02:37 +0100 (CET) Received: from was59-1-82-226-113-214.fbx.proxad.net ([82.226.113.214] helo=[192.168.0.10]) by mail.droids-corp.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84) (envelope-from ) id 1aSjbK-00029f-2u; Mon, 08 Feb 2016 12:03:26 +0100 To: Keith Wiles , dev@dpdk.org References: <1454454177-26743-1-git-send-email-keith.wiles@intel.com> From: Olivier MATZ X-Enigmail-Draft-Status: N1110 Message-ID: <56B875A9.8050402@6wind.com> Date: Mon, 8 Feb 2016 12:02:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.3.0 MIME-Version: 1.0 In-Reply-To: <1454454177-26743-1-git-send-email-keith.wiles@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] mempool: Reduce rte_mempool structure size 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: Mon, 08 Feb 2016 11:02:37 -0000 Hi Keith, Looks good, thanks. Please find some comments below. > [PATCH] mempool: Reduce rte_mempool structure size nit: we usually avoid uppercase letters in title On 02/03/2016 12:02 AM, Keith Wiles wrote: > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index aff5f6d..bdf8e2e 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -450,15 +450,11 @@ rte_mempool_xmem_create(const char *name, unsigned n, unsigned elt_size, > int page_size = getpagesize(); > > /* compilation-time checks */ > +#ifdef RTE_LIBRTE_MEMPOOL_DEBUG > RTE_BUILD_BUG_ON((sizeof(struct rte_mempool) & > RTE_CACHE_LINE_MASK) != 0); > -#if RTE_MEMPOOL_CACHE_MAX_SIZE > 0 > RTE_BUILD_BUG_ON((sizeof(struct rte_mempool_cache) & > RTE_CACHE_LINE_MASK) != 0); > - RTE_BUILD_BUG_ON((offsetof(struct rte_mempool, local_cache) & > - RTE_CACHE_LINE_MASK) != 0); > -#endif > -#ifdef RTE_LIBRTE_MEMPOOL_DEBUG I don't think the #ifdef RTE_LIBRTE_MEMPOOL_DEBUG should be moved. It should only protects the checks on stats which are enabled in debug mode. > @@ -194,10 +192,7 @@ struct rte_mempool { > > unsigned private_data_size; /**< Size of private data. */ > > -#if RTE_MEMPOOL_CACHE_MAX_SIZE > 0 > - /** Per-lcore local cache. */ > - struct rte_mempool_cache local_cache[RTE_MAX_LCORE]; > -#endif > + struct rte_mempool_cache *local_cache; /**< Per-lcore local cache */ > > #ifdef RTE_LIBRTE_MEMPOOL_DEBUG > /** Per-lcore statistics. */ As you noticed it in your initial mail, this changes the ABI. I think your patch justifies the ABI change, so I think it should follow the ABI change process described in dpdk/doc/guides/contributing/versioning.rst. >>From what I understand of versioning.rst, these kind of changes requires a deprecation notice first, and will be integrated in next version. I don't think it's easy to keep a backward compat in this case, especially because the rte_mempool structure is used by several inlined functions. Regards, Olivier