Re: [PATCH 1/6] mempool: add external mempool manager support

["Hunt, David" <david.hunt@intel.com>]

On 2/19/2016 1:30 PM, Olivier MATZ wrote:
> Hi David,
>
> On 02/16/2016 03:48 PM, David Hunt wrote:
>> Adds the new rte_mempool_create_ext api and callback mechanism for
>> external mempool handlers
>>
>> Modifies the existing rte_mempool_create to set up the handler_idx to
>> the relevant mempool handler based on the handler name:
>>      ring_sp_sc
>>      ring_mp_mc
>>      ring_sp_mc
>>      ring_mp_sc
>>
>> v2: merges the duplicated code in rte_mempool_xmem_create and
>> rte_mempool_create_ext into one common function. The old functions
>> now call the new common function with the relevant parameters.
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
> I think the refactoring of rte_mempool_create() (adding of
> mempool_create()) should go in another commit. It will make the
> patches much easier to read.
>
> Also, I'm sorry but it seems that several comments or question I've made
> in http://dpdk.org/ml/archives/dev/2016-February/032706.html are
> not addressed.
>
> Examples:
> - putting some part of the patch in separate commits
> - meaning of "rt_pool"
> - put_pool_bulk unclear comment
> - should we also have get_pool_bulk stats?
> - missing _MEMPOOL_STAT_ADD() in mempool_bulk()
> - why internal in rte_mempool_internal.h?
> - why default in rte_mempool_default.c?
> - remaining references to stack handler (in a comment)
> - ...?
>
> As you know, doing a proper code review takes a lot of time. If I
> have to re-check all of my previous comments, it will take even
> more. I'm not saying all my comments require a code change, but in case
> you don't agree, please at least explain your opinion so we can debate
> on the list.
>
Hi Olivier,
    Sincerest apologies. I had intended in coming back around to your 
original comments after refactoring the code. I will do that now. I did 
take them into consideration, but I see now that I need to do further 
work, such as a clearer name for rt_pool, etc.  I will respond to your 
original email.
Thanks
David.