> -----Original Message----- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Lazaros Koromilas > Sent: Thursday, March 24, 2016 7:36 AM > To: Wiles, Keith <keith.wiles at intel.com> > Cc: Olivier Matz <olivier.matz at 6wind.com>; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH] mempool: allow for user-owned mempool > caches > > On Mon, Mar 21, 2016 at 3:49 PM, Wiles, Keith <keith.wiles at intel.com> > wrote: > >>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 at 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. > > Hi Keith, > > The main benefit of having an external cache is to allow mempool users > (threads) to maintain a local cache even though they don't have a valid > lcore_id (non-EAL threads). The fact that cache access is done by indexing > with the lcore_id is what makes it difficult...
Hi Lazaros, Alternative suggestion: This could actually be very simply done via creating an EAL API to register and return an lcore_id for a thread wanting to use DPDK services. That way, you could simply create your pthread, call the eal_register_thread() function that assigns an lcore_id to the caller (and internally sets up the per_lcore variable. The advantage of doing it this way is that you could extend it to other things other than the mempool that may need an lcore_id setup. Regards, -Venky > > What could happen is only have external caches somehow, but that hurts the > common case where you want an automatic cache. > Or a cache registration mechanism (overkill?). > > So, I'm going to work on the comments and send out a v2 asap. Thanks > everyone! > > Lazaros. > > > > >> > >>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 > > > > > > > >
