From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lazaros Koromilas Subject: Re: [PATCH] mempool: allow for user-owned mempool caches Date: Thu, 24 Mar 2016 16:35:47 +0200 Message-ID: References: <1457621082-22151-1-git-send-email-l@nofutznetworks.com> <56EFE781.4090809@6wind.com> <1FFBBED4-00F5-4703-BDEC-961EB800C21B@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: Olivier Matz , "dev@dpdk.org" To: "Wiles, Keith" Return-path: Received: from mail-oi0-f47.google.com (mail-oi0-f47.google.com [209.85.218.47]) by dpdk.org (Postfix) with ESMTP id BD8312BD6 for ; Thu, 24 Mar 2016 15:36:23 +0100 (CET) Received: by mail-oi0-f47.google.com with SMTP id d205so62799294oia.0 for ; Thu, 24 Mar 2016 07:36:23 -0700 (PDT) In-Reply-To: <1FFBBED4-00F5-4703-BDEC-961EB800C21B@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 Mon, Mar 21, 2016 at 3:49 PM, Wiles, Keith 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 thread= s >>> 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 >>> --- >>> 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_n= um, 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 var= iables) and only allocates the cache if specified by the caller to use a ca= che. 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 memp= ool as the new cache create routines and APIs could be in a new file I thin= k, 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... 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 everyo= ne! 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 > > > >