From: Lazaros Koromilas <l@nofutznetworks.com>
To: "Wiles, Keith" <keith.wiles@intel.com>
Cc: Olivier Matz <olivier.matz@6wind.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] mempool: allow for user-owned mempool caches
Date: Thu, 24 Mar 2016 16:35:47 +0200 [thread overview]
Message-ID: <CAHPNE8iwh--NCDMo9BZqq==GBchwrPC3cWWK5egnvfm1FXMFCg@mail.gmail.com> (raw)
In-Reply-To: <1FFBBED4-00F5-4703-BDEC-961EB800C21B@intel.com>
On Mon, Mar 21, 2016 at 3:49 PM, Wiles, Keith <keith.wiles@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@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...
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
>
>
>
>
next prev parent reply other threads:[~2016-03-24 14:36 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-10 14:44 Lazaros Koromilas
2016-03-21 12:22 ` Olivier Matz
2016-03-21 13:15 ` Ananyev, Konstantin
2016-03-24 13:51 ` Lazaros Koromilas
2016-03-21 13:49 ` Wiles, Keith
2016-03-24 14:35 ` Lazaros Koromilas [this message]
2016-03-24 14:58 ` Venkatesan, Venky
2016-03-25 10:55 ` Olivier Matz
2016-03-25 20:42 ` Mauricio Vásquez
2016-03-25 20:46 ` Venkatesan, Venky
2016-03-24 15:03 ` Wiles, Keith
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHPNE8iwh--NCDMo9BZqq==GBchwrPC3cWWK5egnvfm1FXMFCg@mail.gmail.com' \
--to=l@nofutznetworks.com \
--cc=dev@dpdk.org \
--cc=keith.wiles@intel.com \
--cc=olivier.matz@6wind.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).