DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Suanming Mou" <suanmingm@mellanox.com>,
	"Olivier Matz" <olivier.matz@6wind.com>,
	"Xueming(Steven) Li" <xuemingl@mellanox.com>
Cc: "Andrew Rybchenko" <arybchenko@solarflare.com>, <dev@dpdk.org>,
	"Asaf Penso" <asafp@mellanox.com>, "Ori Kam" <orika@mellanox.com>
Subject: Re: [dpdk-dev] [RFC] mempool: introduce indexed memory pool
Date: Fri, 6 Mar 2020 09:57:50 +0100	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35C60E8F@smartserver.smartshare.dk> (raw)
In-Reply-To: <HE1PR05MB34844B6EFC55BD2280E00C4CCCE30@HE1PR05MB3484.eurprd05.prod.outlook.com>

> From: Suanming Mou [mailto:suanmingm@mellanox.com]
> 
> Hi Morten,
> 
> Thanks for the comments.
> 
> > From: Morten Brørup <mb@smartsharesystems.com>
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Suanming Mou
> On
> > > 12/26/2019 7:05 PM, Olivier Matz wrote:
> > > > On Thu, Oct 17, 2019 at 06:55:01AM +0000, Xueming Li wrote:
> > > >> Indexed memory pool manages memory entries by index, allocation
> > > >> from pool returns both memory pointer and index(ID). users save
> ID
> > > >> as u32 or less(u16) instead of traditional 8 bytes pointer.
> Memory
> > > >> could be retrieved from pool or returned to pool later by index.
> > > >>
> > > >> Pool allocates backend memory in chunk on demand, pool size
> grows
> > > >> dynamically. Bitmap is used to track entry usage in chunk, thus
> > > >> management overhead is one bit per entry.
> > > >>
> > > >> Standard rte_malloc demands malloc overhead(64B) and minimal
> data
> > > >> size(64B). This pool aims to such cost saving also pointer size.
> > > >> For scenario like creating millions of rte_flows each consists
> of
> > > >> small pieces of memories, the difference is huge.
> > > >>
> > > >> Like standard memory pool, this lightweight pool only support
> fixed
> > > >> size memory allocation. Pools should be created for each
> different
> > > >> size.
> > > >>
> > > >> To facilitate memory allocated by index, a set of ILIST_XXX
> macro
> > > >> defined to operate entries as regular LIST.
> > > >>
> > > >> By setting entry size to zero, pool can be used as ID generator.
> > > >>
> > > >> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> >
> > So, you have a use case where 64 bit pointers use too much memory,
> and you
> > want to optimize for memory at the cost of performance by using 16,
> 24 or 32
> > bit references instead. A lot of compilers have an option to do this,
> so this is
> > generally a valid optimization from a high level point of view.
> >
> > I like the general concept, so I have a few high level comments to
> the RFC:
> >
> > Your API should separate pool creation from element allocation, i.e.
> define one
> > function to create a pool and set the element size of that pool, and
> define other
> > functions to allocate (get) and free (put) elements in a pool.
> >
> > Furthermore, your implementation takes a lock when referencing an
> element.
> > Dereferencing an element by its index should be optimized for speed,
> and should
> > be lockless. Remember: DPDK is a data plane development kit, not a
> control
> > plane development kit.
> 
> Agree with that, however, this is used for control plane. And there is
> already a lock need option for user to configure.

The library must be designed for the data plane, at least on API level. If done right, this library could be become a core library - a genuine "memory optimized" alternative to the Mempool library.

With that said, the implementation can be done step by step, so the first version uses locking and serves your control plane use case well. But there should be a roadmap towards a later version for use in the data plane, preferably lockless.

I am not saying that you are obligated to implement the lockless data plane version. I am only asking you to design the API with the data plane in mind.

> It seems the v1 RFC misses some code which will free the pool trunk
> memory once the trunk is totally not used anymore.
> In this case, lock is required with multiple threads as trunk memory
> maybe freed.
> 
> >
> > Also consider providing functions to allocate/free consecutive arrays
> of
> > elements, so they can be dereferenced even faster because only the
> address of
> > the first element in the array needs to be looked up through your
> library.
> > Alternatively, provide a function for bulk dereferencing. I don't
> know if there is a
> > use case for this... just mentioning it. And if the library's
> dereferencing function
> > is fast enough, this becomes less relevant.
> 
> Currently, it is not needed. But once implemented as Mempool device
> driver, it will be supported.
> 
> >
> > This library will be used for well defined structures, so the library
> should
> > resemble the Mempool library (for fixed size element allocations)
> more than the
> > Malloc library (for variable size allocations).
> >
> > You can consider copying the Mempool API, but returning indexes
> instead of
> > pointers.
> >
> > You should also consider building your implementation on top of the
> Mempool
> > library, like the Mbuf library does. This will give you per-lcore
> caching and other
> > benefits already provided by the Mempool library.
> 
> Step by step, the on top of Mempool implementation will depend on the
> requirement.
> 
> >
> >
> > Finally, a small detail: The macros for using your indexed mempool
> elements in a
> > linked list should not be part of the library itself. They should be
> placed in a
> > separate library, so similar macros/functions for using indexed
> mempool
> > elements in other structures (hashes, queues, etc.) can also be added
> as
> > separate libraries at a later time.
> 
> Good suggestion.
> 
> >
> >
> > Med venlig hilsen / kind regards
> > - Morten Brørup

      reply	other threads:[~2020-03-06  8:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-17  6:55 Xueming Li
2019-10-17  7:13 ` Jerin Jacob
2019-10-17 13:13   ` Xueming(Steven) Li
2019-10-17 16:40     ` Jerin Jacob
2019-10-18 10:10       ` Xueming(Steven) Li
2019-10-19 12:28         ` Jerin Jacob
2019-10-25 15:29           ` Xueming(Steven) Li
2019-10-25 16:28             ` Jerin Jacob
2019-12-26 11:05 ` Olivier Matz
2020-03-05  7:43   ` Suanming Mou
2020-03-05  9:52     ` Morten Brørup
2020-03-06  7:27       ` Suanming Mou
2020-03-06  8:57         ` Morten Brørup [this message]

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=98CBD80474FA8B44BF855DF32C47DC35C60E8F@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=arybchenko@solarflare.com \
    --cc=asafp@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=orika@mellanox.com \
    --cc=suanmingm@mellanox.com \
    --cc=xuemingl@mellanox.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).