From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier MATZ Subject: Re: [PATCH v12 0/3] mempool: add external mempool manager Date: Wed, 15 Jun 2016 18:40:23 +0200 Message-ID: <576184F7.3040206@6wind.com> References: <1465919341-3209-1-git-send-email-david.hunt@intel.com> <1465976824-83823-1-git-send-email-david.hunt@intel.com> <20160615121358.5ef9f142@pcviktorin.fit.vutbr.cz> <57614043.9090603@intel.com> <57614402.6020708@6wind.com> <576183AD.8070200@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, jerin.jacob@caviumnetworks.com, shreyansh.jain@nxp.com To: "Hunt, David" , Jan Viktorin Return-path: Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 060AFC620 for ; Wed, 15 Jun 2016 18:40:23 +0200 (CEST) In-Reply-To: <576183AD.8070200@intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 06/15/2016 06:34 PM, Hunt, David wrote: > > > On 15/6/2016 1:03 PM, Olivier MATZ wrote: >> [...] >> >> The opaque pointer would be saved in mempool structure, and used >> when the mempool is populated (calling mempool_ops_alloc). >> The type of the structure pointed by the opaque has to be defined >> (and documented) into each mempool_ops manager. >> >> >> Olivier > > > OK, just to be sure before I post another patchset..... > > For the rte_mempool_struct: > struct rte_mempool_memhdr_list mem_list; /**< List of memory > chunks */ > + void *ops_args; /**< optional args for ops > alloc. */ > > (at the end of the struct, as it's just on the control path, not to > affect fast path) Hmm, I would put it just after pool_data. > > Then change function params: > int > -rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name); > +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, > + void *ops_args); > > And (almost) finally in the rte_mempool_set_ops_byname function: > mp->ops_index = i; > + mp->ops_args = ops_args; > return 0; > > Then (actually) finally, add a null to all the calls to > rte_mempool_set_ops_byname. > > OK? :) > Else, looks good to me! Thanks David.