Re: [dpdk-dev] [PATCH] mempool: allow for user-owned mempool caches

["Wiles, Keith" <keith.wiles@intel.com>]

>Hi Lazaros,
>
>Thanks for this patch. To me, this is a valuable enhancement.
>Please find some comments inline.
>
>On 03/10/2016 03:44 PM, Lazaros Koromilas wrote:
>> The mempool cache is only available to EAL threads as a per-lcore
>> resource. Change this so that the user can create and provide their own
>> cache on mempool get and put operations. This works with non-EAL threads
>> too. This commit introduces new API calls with the 'with_cache' suffix,
>> while the current ones default to the per-lcore local cache.
>> 
>> Signed-off-by: Lazaros Koromilas <l@nofutznetworks.com>
>> ---
>>  lib/librte_mempool/rte_mempool.c |  65 +++++-
>>  lib/librte_mempool/rte_mempool.h | 442 ++++++++++++++++++++++++++++++++++++---
>>  2 files changed, 467 insertions(+), 40 deletions(-)
>> 
>> diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
>> index f8781e1..cebc2b7 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -375,6 +375,43 @@ rte_mempool_xmem_usage(void *vaddr, uint32_t elt_num, size_t elt_sz,
>>  	return usz;
>>  }
>>  
>> +#if RTE_MEMPOOL_CACHE_MAX_SIZE > 0
>
>I wonder if this wouldn't cause a conflict with Keith's patch
>that removes some #ifdefs RTE_MEMPOOL_CACHE_MAX_SIZE.
>See: http://www.dpdk.org/dev/patchwork/patch/10492/

Hi Lazaros,

The patch I submitted keeps the mempool cache structure (pointers and variables) and only allocates the cache if specified by the caller to use a cache. This means to me the caller could fill in the cache pointer and values into the mempool structure to get a cache without a lot of extra code. If we added a set of APIs to fill in these structure variables would that not give you the external cache support. I have not really looked at the patch to verify this will work, but it sure seems like it.

So my suggestion the caller can just create a mempool without a cache and then call a set of APIs to fill in his cache values, does that not work?

If we can do this it reduces the API and possible the ABI changes to mempool as the new cache create routines and APIs could be in a new file I think, which just updates the mempool structure correctly.

>
>As this patch is already acked for 16.07, I think that your v2
>could be rebased on top of it to avoid conflicts when Thomas will apply
>it.
>
>By the way, I also encourage you to have a look at other works in
>progress in mempool:
>http://www.dpdk.org/ml/archives/dev/2016-March/035107.html
>http://www.dpdk.org/ml/archives/dev/2016-March/035201.html
>
>

Regards,
Keith