From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH 2/4] mempool: detect physical contiguous object in pool Date: Mon, 3 Jul 2017 18:37:15 +0200 Message-ID: <20170703183715.3b1c8ff6@platinum> References: <20170621173248.1313-1-santosh.shukla@caviumnetworks.com> <20170621173248.1313-3-santosh.shukla@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, thomas@monjalon.net, hemant.agrawal@nxp.com, jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com To: Santosh Shukla Return-path: Received: from mail-wr0-f177.google.com (mail-wr0-f177.google.com [209.85.128.177]) by dpdk.org (Postfix) with ESMTP id 08038374F for ; Mon, 3 Jul 2017 18:37:17 +0200 (CEST) Received: by mail-wr0-f177.google.com with SMTP id c11so237258696wrc.3 for ; Mon, 03 Jul 2017 09:37:17 -0700 (PDT) In-Reply-To: <20170621173248.1313-3-santosh.shukla@caviumnetworks.com> 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 Wed, 21 Jun 2017 17:32:46 +0000, Santosh Shukla wrote: > HW mempool blocks may need physical contiguous obj in a pool. This should be clarified: the memory area containing all the objects must be physically contiguous, right? > Introducing MEMPOOL_F_POOL_CONTIG flag for such use-case. The flag > useful to detect whether all buffer fits within a hugepage or not. If > not then return -ENOSPC. This way, we make sure that all object within a > pool is contiguous. > > Signed-off-by: Santosh Shukla > Signed-off-by: Jerin Jacob > --- > lib/librte_mempool/rte_mempool.c | 8 ++++++++ > lib/librte_mempool/rte_mempool.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index 045baef45..7dec2f51d 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -368,6 +368,14 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr, > > total_elt_sz = mp->header_size + mp->elt_size + mp->trailer_size; > > + /* Detect nb_mbuf fit in hugepage */ > + if (mp->flags & MEMPOOL_F_POOL_CONTIG) { > + if (len < total_elt_sz * mp->size) { > + RTE_LOG(ERR, MEMPOOL, "nb_mbufs not fitting in one hugepage,..exit\n"); > + return -ENOSPC; > + } > + } > + We should not reference mbuf, we are in mempool code, dealing with any kind of object. Also, len is not necessarily the size of a hugepage, but the size of the physical area passed to te_mempool_populate_phys(). > memhdr = rte_zmalloc("MEMPOOL_MEMHDR", sizeof(*memhdr), 0); > if (memhdr == NULL) > return -ENOMEM; > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h > index c3cdc77e4..fd8722e69 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -266,6 +266,7 @@ struct rte_mempool { > #define MEMPOOL_F_SC_GET 0x0008 /**< Default get is "single-consumer".*/ > #define MEMPOOL_F_POOL_CREATED 0x0010 /**< Internal: pool is created. */ > #define MEMPOOL_F_NO_PHYS_CONTIG 0x0020 /**< Don't need physically contiguous objs. */ > +#define MEMPOOL_F_POOL_CONTIG 0x0040 /**< Detect physcially contiguous objs */ We must highlight here that it's a capability flag. Following my other comments on the first patch, this define should be renamed in something else. I suggest: #define RTE_MEMPOOL_CAPA_PHYS_CONTIG 0x0001 The description should be longer and more accurate. I'm also a bit puzzled because this is more a limitation than a capability. > > /** > * @internal When debug is enabled, store some statistics.