From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <olivier.matz@6wind.com>
Received: from mail.droids-corp.org (zoll.droids-corp.org [94.23.50.67])
 by dpdk.org (Postfix) with ESMTP id A2956199B0
 for <dev@dpdk.org>; Thu,  7 Sep 2017 11:09:14 +0200 (CEST)
Received: from lfbn-1-18623-73.w90-103.abo.wanadoo.fr ([90.103.154.73]
 helo=droids-corp.org)
 by mail.droids-corp.org with esmtpsa (TLS1.0:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.84_2) (envelope-from <olivier.matz@6wind.com>)
 id 1dpstc-0004Kh-Fb; Thu, 07 Sep 2017 11:14:49 +0200
Received: by droids-corp.org (sSMTP sendmail emulation);
 Thu, 07 Sep 2017 11:09:06 +0200
Date: Thu, 7 Sep 2017 11:09:06 +0200
From: Olivier MATZ <olivier.matz@6wind.com>
To: santosh <santosh.shukla@caviumnetworks.com>
Cc: dev@dpdk.org, thomas@monjalon.net, jerin.jacob@caviumnetworks.com,
 hemant.agrawal@nxp.com
Message-ID: <20170907090906.z4q5vyubtboiabd2@neon>
References: <20170815060743.21076-1-santosh.shukla@caviumnetworks.com>
 <20170906112834.32378-1-santosh.shukla@caviumnetworks.com>
 <20170906112834.32378-9-santosh.shukla@caviumnetworks.com>
 <20170907082959.lfrkdp4mcg3qpb4r@neon>
 <d29e0093-d42f-7788-f5c9-10b0fa9a963b@caviumnetworks.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d29e0093-d42f-7788-f5c9-10b0fa9a963b@caviumnetworks.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Subject: Re: [dpdk-dev] [PATCH v5 8/8] mempool: update range info to pool
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Thu, 07 Sep 2017 09:09:14 -0000

On Thu, Sep 07, 2017 at 02:26:49PM +0530, santosh wrote:
> 
> On Thursday 07 September 2017 02:00 PM, Olivier MATZ wrote:
> > On Wed, Sep 06, 2017 at 04:58:34PM +0530, Santosh Shukla wrote:
> >> HW pool manager e.g. Octeontx SoC demands s/w to program start and end
> >> address of pool. Currently, there is no such handle in external mempool.
> >> Introducing rte_mempool_update_range handle which will let HW(pool
> >> manager) to know when common layer selects hugepage:
> >> For each hugepage - update its start/end address to HW pool manager.
> >>
> >> Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
> >> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> ---
> >>  lib/librte_mempool/rte_mempool.c           |  3 +++
> >>  lib/librte_mempool/rte_mempool.h           | 22 ++++++++++++++++++++++
> >>  lib/librte_mempool/rte_mempool_ops.c       | 13 +++++++++++++
> >>  lib/librte_mempool/rte_mempool_version.map |  1 +
> >>  4 files changed, 39 insertions(+)
> >>
> >> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
> >> index 38dab1067..65f17a7a7 100644
> >> --- a/lib/librte_mempool/rte_mempool.c
> >> +++ b/lib/librte_mempool/rte_mempool.c
> >> @@ -363,6 +363,9 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char *vaddr,
> >>  	struct rte_mempool_memhdr *memhdr;
> >>  	int ret;
> >>  
> >> +	/* update range info to mempool */
> >> +	rte_mempool_ops_update_range(mp, vaddr, paddr, len);
> >> +
> >
> > My understanding is that the 2 capability flags imply that the mempool
> > is composed of only one memory area (rte_mempool_memhdr). Do you confirm?
> 
> yes.
> 
> > So in your case, you will be notified only once with the full range of
> > the mempool. But if there are several memory areas, the function will
> > be called each time.
> >
> > So I suggest to rename rte_mempool_ops_update_range() in
> > rte_mempool_ops_register_memory_area(), which goal is to notify the mempool
> > handler each time a new memory area is added.
> >
> > This should be properly explained in the API comments.
> >
> > I think this handler can return an error code (0 on success, negative on
> > error). On error, rte_mempool_populate_phys() should fail.
> >
> Will rename to rte_mempool_ops_register_memory_area() and
> change return type from void to int.
> 
> 
> return description:
> 0 : for success
> <0 : failure, such that
>  - if handler returns -ENOTSUP then valid error case--> no error handling at mempool layer
>  - Otherwise  rte_mempool_populate_phys () fails.
> 
> Are you okay with error return? pl. confirm. 

yes.
Just take care of doubled spaces and avoid "-->".