From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas.monjalon@6wind.com>
Subject: Re: [PATCH v10 1/3] mempool: support external mempool
	operations
Date: Tue, 14 Jun 2016 15:29:54 +0200
Message-ID: <4802005.Wuc0l2sk9u@xps13>
References: <1465571806-22008-1-git-send-email-david.hunt@intel.com>
 <1713942.6DB6T3RQsY@xps13> <576004B4.7070708@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, olivier.matz@6wind.com, viktorin@rehivetech.com,
 jerin.jacob@caviumnetworks.com, shreyansh.jain@nxp.com, "Mcnamara,
 John" <john.mcnamara@intel.com>
To: "Hunt, David" <david.hunt@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f54.google.com (mail-wm0-f54.google.com [74.125.82.54])
 by dpdk.org (Postfix) with ESMTP id B3DC595EC
 for <dev@dpdk.org>; Tue, 14 Jun 2016 15:29:57 +0200 (CEST)
Received: by mail-wm0-f54.google.com with SMTP id k204so123223805wmk.0
 for <dev@dpdk.org>; Tue, 14 Jun 2016 06:29:57 -0700 (PDT)
In-Reply-To: <576004B4.7070708@intel.com>
List-Id: patches and discussions about DPDK <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>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

2016-06-14 14:20, Hunt, David:
> 
> Hi Thomas,
> 
> On 14/6/2016 1:55 PM, Thomas Monjalon wrote:
> > Hi David,
> >
> > 2016-06-14 10:46, David Hunt:
> >> Until now, the objects stored in a mempool were internally stored in a
> >> ring. This patch introduces the possibility to register external handlers
> >> replacing the ring.
> >>
> >> The default behavior remains unchanged, but calling the new function
> >> rte_mempool_set_handler() right after rte_mempool_create_empty() allows
> >> the user to change the handler that will be used when populating
> >> the mempool.
> >>
> >> This patch also adds a set of default ops (function callbacks) based
> >> on rte_ring.
> >>
> >> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> >> Signed-off-by: David Hunt <david.hunt@intel.com>
> > Glad to see we are close to have this feature integrated.
> >
> > I've just looked into few details before pushing.
> > One of them are the comments. In mempool they were all ended by a dot.
> > Please check the new comments.
> 
> Do you mean the rte_mempool struct definition, or all comments? Shall I 
> leave the
> old comments the way they were before the change, or will I clean up?
> If I clean up, I'd suggest I add a separate patch for that.

Just check and clean the comments added in this patch.

> > The doc/guides/rel_notes/deprecation.rst must be updated to remove
> > the deprecation notice in this patch.
> 
> Will do. As a separate patch in the set?

In this patch.

> > Isn't there some explanations to add in
> > doc/guides/prog_guide/mempool_lib.rst?
> 
> Yes, I'll adapt some of the cover letter, and add as a separate patch.

It is OK (and better) to add it in this patch.
Maybe you can request John's help for doc review.

> > Isn't there a better name than "default" for the default implementation?
> > I don't think the filename rte_mempool_default.c is meaningful.
> 
> I could call it rte_mempool_ring.c? Since the default handler is ring based?

It is an idea.

Thanks