Hi Shreyansh, On 16/6/2016 5:35 AM, Shreyansh Jain wrote: > Though I am late to the discussion... > >> -----Original Message----- >> From: Olivier MATZ [mailto:olivier.matz at 6wind.com] >> Sent: Wednesday, June 15, 2016 10:10 PM >> To: Hunt, David <david.hunt at intel.com>; Jan Viktorin >> <viktorin at rehivetech.com> >> Cc: dev at dpdk.org; jerin.jacob at caviumnetworks.com; Shreyansh Jain >> <shreyansh.jain at nxp.com> >> Subject: Re: [PATCH v12 0/3] mempool: add external mempool manager >> >> >> >> 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. > +1 > And, would 'pool_config' (picked from a previous email from David) a better > name? > > From a user perspective, the application is passing a configuration item to > the pool to work one. Only the application and mempool allocator understand > it (opaque). > As for 'ops_arg', it would be to control 'assignment-of-operations' to the > framework. > > Maybe just my point of view.
I agree. I was originally happy with pool_config, sits well with pool_data. And it's data for configuring the pool during allocation. I'll go with that, then. >>> 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. > Me too. Though I would like to clarify something for my understanding: > > Mempool->pool_data => Used by allocator to store private data > Mempool->pool_config => (or ops_arg) used by allocator to access user/app > provided value. > > Is that correct? Yes, that's correct. Regard, David.
