From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 336DB378B for ; Wed, 18 Mar 2015 21:59:02 +0100 (CET) Received: from [2001:470:8:a08:215:ff:fecc:4872] (helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1YYL3K-0000bW-Dd; Wed, 18 Mar 2015 16:59:00 -0400 Date: Wed, 18 Mar 2015 16:58:56 -0400 From: Neil Horman To: "vadim.suraev@gmail.com" Message-ID: <20150318205856.GA5151@neilslaptop.think-freely.org> References: <1426710078-11230-1-git-send-email-vadim.suraev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1426710078-11230-1-git-send-email-vadim.suraev@gmail.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v2] rte_mbuf: mbuf bulk alloc/free functions added + unittest 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: Wed, 18 Mar 2015 20:59:02 -0000 On Wed, Mar 18, 2015 at 10:21:18PM +0200, vadim.suraev@gmail.com wrote: > From: "vadim.suraev@gmail.com" > > This patch adds mbuf bulk allocation/freeing functions and unittest > > Signed-off-by: Vadim Suraev > > --- > New in v2: > - function rte_pktmbuf_alloc_bulk added > - function rte_pktmbuf_bulk_free added > - function rte_pktmbuf_free_chain added > - applied reviewers' comments > > app/test/test_mbuf.c | 94 +++++++++++++++++++++++++++++++++++++++++++- > lib/librte_mbuf/rte_mbuf.h | 91 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 184 insertions(+), 1 deletion(-) > > diff --git a/app/test/test_mbuf.c b/app/test/test_mbuf.c > index 1ff66cb..b20c6a4 100644 > --- a/app/test/test_mbuf.c > +++ b/app/test/test_mbuf.c > @@ -77,6 +77,7 @@ > #define REFCNT_RING_SIZE (REFCNT_MBUF_NUM * REFCNT_MAX_REF) > > #define MAKE_STRING(x) # x > +#define MBUF_POOL_LOCAL_CACHE_SIZE 32 > > static struct rte_mempool *pktmbuf_pool = NULL; > > @@ -405,6 +406,84 @@ test_pktmbuf_pool(void) > return ret; > } > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > index 17ba791..fabeae2 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -825,6 +825,97 @@ static inline void rte_pktmbuf_free(struct rte_mbuf *m) > } > > /** > + * Allocate a bulk of mbufs, initiate refcnt and resets > + * > + * @param pool > + * memory pool to allocate from > + * @param mbufs > + * Array of pointers to mbuf > + * @param count > + * Array size > + */ > +static inline int rte_pktmbuf_alloc_bulk(struct rte_mempool *pool, > + struct rte_mbuf **mbufs, > + unsigned count) > +{ > + unsigned idx; > + int rc = 0; > + > + rc = rte_mempool_get_bulk(pool, (void **)mbufs, count); > + if (unlikely(rc)) > + return rc; > + > + for (idx = 0; idx < count; idx++) { > + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 0); > + rte_mbuf_refcnt_set(mbufs[idx], 1); > + rte_pktmbuf_reset(mbufs[idx]); > + } > + return rc; > +} > + > +/** > + * Free a bulk of mbufs into its original mempool. > + * This function assumes: > + * - refcnt equals 1 > + * - mbufs are direct > + * - all mbufs must belong to the same mempool > + * > + * @param mbufs > + * Array of pointers to mbuf > + * @param count > + * Array size > + */ > +static inline void rte_pktmbuf_bulk_free(struct rte_mbuf **mbufs, > + unsigned count) > +{ > + unsigned idx; > + > + RTE_MBUF_ASSERT(count > 0); > + > + for (idx = 0; idx < count; idx++) { > + RTE_MBUF_ASSERT(mbufs[idx]->pool == mbufs[0]->pool); > + RTE_MBUF_ASSERT(rte_mbuf_refcnt_read(mbufs[idx]) == 1); > + rte_mbuf_refcnt_set(mbufs[idx], 0); This is really a misuse of the API. The entire point of reference counting is to know when an mbuf has no more references and can be freed. By forcing all the reference counts to zero here, you allow the refcnt infrastructure to be circumvented, causing memory leaks. I think what you need to do here is enhance the underlying pktmbuf interface such that an rte_mbuf structure has a destructor method association with it which is called when its refcnt reaches zero. That way the rte_pktmbuf_bulk_free function can just decrement the refcnt on each mbuf_structure, and the pool as a whole can be returned when the destructor function discovers that all mbufs in that bulk pool are freed. Neil