[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
Hi David, If you plan to do a v14 for this API comment, I'm wondering if the documentation could be slightly modified too. I think "external mempool manager" was the legacy name for the feature, but maybe it could be changed in "alternative mempool handlers" or "changing the mempool handler". I mean the word "external" is probably not appropriate now, especially if we add other handlers in the mempool lib. My 2 cents, Olivier >>> I had not planned on doing another revision. And I think the term >>> "External >>> Mempool Manager" accurately describes the functionality, so I'd really >>> prefer to leave it as it is. >> I think there is no manager, just a default handler which can be changed. >> I agree the documentation must be fixed. > > OK, I have two suggestions to add into the mix. > 1. mempool handler framework > or simply > 2. mempool handlers. (the alternative is implied). "The mempool handler > feature", etc. Option 2 is fine for me. Thanks!
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
Hi David,
While testing Lazaros' patch, I found an issue in this series.
I the test application is started with --no-huge, it does not work,
the mempool_autotest does not work. Please find the exaplanation
below:
On 06/16/2016 02:30 PM, David Hunt wrote:
> @@ -386,9 +352,9 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char
> *vaddr,
> int ret;
>
> /* create the internal ring if not already done */
> - if ((mp->flags & MEMPOOL_F_RING_CREATED) == 0) {
> - ret = rte_mempool_ring_create(mp);
> - if (ret < 0)
> + if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
> + ret = rte_mempool_ops_alloc(mp);
> + if (ret != 0)
> return ret;
> }
>
Previously, the function rte_mempool_ring_create(mp) was setting the
MEMPOOL_F_RING_CREATED flag. Now it is not set. I think we could
set it just after the "return ret".
When started with --no-huge, the mempool memory is allocated in
several chunks (one per page), so it tries to allocate the ring for
each chunk.
Regards,
Olivier
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
On 17/6/2016 11:18 AM, Olivier Matz wrote:
> Hi David,
>
> While testing Lazaros' patch, I found an issue in this series.
> I the test application is started with --no-huge, it does not work,
> the mempool_autotest does not work. Please find the exaplanation
> below:
>
> On 06/16/2016 02:30 PM, David Hunt wrote:
>> @@ -386,9 +352,9 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char
>> *vaddr,
>> int ret;
>>
>> /* create the internal ring if not already done */
>> -if ((mp->flags & MEMPOOL_F_RING_CREATED) == 0) {
>> -ret = rte_mempool_ring_create(mp);
>> -if (ret < 0)
>> +if ((mp->flags & MEMPOOL_F_POOL_CREATED) == 0) {
>> +ret = rte_mempool_ops_alloc(mp);
>> +if (ret != 0)
>> return ret;
>> }
>>
> Previously, the function rte_mempool_ring_create(mp) was setting the
> MEMPOOL_F_RING_CREATED flag. Now it is not set. I think we could
> set it just after the "return ret".
>
> When started with --no-huge, the mempool memory is allocated in
> several chunks (one per page), so it tries to allocate the ring for
> each chunk.
>
> Regards,
> Olivier
OK, Will do.
ret = rte_mempool_ops_alloc(mp);
if (ret != 0)
return ret;
+ mp->flags |= MEMPOOL_F_POOL_CREATED;
Rgds,
Dave.
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
2016-06-17 09:42, Hunt, David: > > On 17/6/2016 9:08 AM, Olivier Matz wrote: > > Hi David, > > > > On 06/17/2016 08:58 AM, Hunt, David wrote: > >> A comment below: > >> > >> On 16/6/2016 1:30 PM, David Hunt wrote: > >>> +/** > >>> + * Set the ops of a mempool. > >>> + * > >>> + * This can only be done on a mempool that is not populated, i.e. > >>> just after > >>> + * a call to rte_mempool_create_empty(). > >>> + * > >>> + * @param mp > >>> + * Pointer to the memory pool. > >>> + * @param name > >>> + * Name of the ops structure to use for this mempool. > >> + * @param pool_config > >> + * Opaque data that can be used by the ops functions. > >>> + * @return > >>> + * - 0: Success; the mempool is now using the requested ops functions. > >>> + * - -EINVAL - Invalid ops struct name provided. > >>> + * - -EEXIST - mempool already has an ops struct assigned. > >>> + */ > >>> +int > >>> +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, > >>> +void *pool_config); > >>> + > >> > > The changes related to the pool_config look good to me. > > > > If you plan to do a v14 for this API comment, I'm wondering if the > > documentation could be slightly modified too. I think "external mempool > > manager" was the legacy name for the feature, but maybe it could be > > changed in "alternative mempool handlers" or "changing the mempool > > handler". I mean the word "external" is probably not appropriate now, > > especially if we add other handlers in the mempool lib. > > > > My 2 cents, > > Olivier > > I had not planned on doing another revision. And I think the term "External > Mempool Manager" accurately describes the functionality, so I'd really > prefer to leave it as it is. I think there is no manager, just a default handler which can be changed. I agree the documentation must be fixed.
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
On 17/6/2016 10:09 AM, Thomas Monjalon wrote: > 2016-06-17 09:42, Hunt, David: >> On 17/6/2016 9:08 AM, Olivier Matz wrote: >>> Hi David, >>> >>> On 06/17/2016 08:58 AM, Hunt, David wrote: A comment below: On 16/6/2016 1:30 PM, David Hunt wrote: > +/** > + * Set the ops of a mempool. > + * > + * This can only be done on a mempool that is not populated, i.e. > just after > + * a call to rte_mempool_create_empty(). > + * > + * @param mp > + * Pointer to the memory pool. > + * @param name > + * Name of the ops structure to use for this mempool. + * @param pool_config + * Opaque data that can be used by the ops functions. > + * @return > + * - 0: Success; the mempool is now using the requested ops functions. > + * - -EINVAL - Invalid ops struct name provided. > + * - -EEXIST - mempool already has an ops struct assigned. > + */ > +int > +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, > +void *pool_config); > + >>> The changes related to the pool_config look good to me. >>> >>> If you plan to do a v14 for this API comment, I'm wondering if the >>> documentation could be slightly modified too. I think "external mempool >>> manager" was the legacy name for the feature, but maybe it could be >>> changed in "alternative mempool handlers" or "changing the mempool >>> handler". I mean the word "external" is probably not appropriate now, >>> especially if we add other handlers in the mempool lib. >>> >>> My 2 cents, >>> Olivier >> I had not planned on doing another revision. And I think the term "External >> Mempool Manager" accurately describes the functionality, so I'd really >> prefer to leave it as it is. > I think there is no manager, just a default handler which can be changed. > I agree the documentation must be fixed. OK, I have two suggestions to add into the mix. 1. mempool handler framework or simply 2. mempool handlers. (the alternative is implied). "The mempool handler feature", etc. Thoughts? David.
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
Hi David, On 06/17/2016 08:58 AM, Hunt, David wrote: > A comment below: > > On 16/6/2016 1:30 PM, David Hunt wrote: >> +/** >> + * Set the ops of a mempool. >> + * >> + * This can only be done on a mempool that is not populated, i.e. >> just after >> + * a call to rte_mempool_create_empty(). >> + * >> + * @param mp >> + * Pointer to the memory pool. >> + * @param name >> + * Name of the ops structure to use for this mempool. > + * @param pool_config > + * Opaque data that can be used by the ops functions. >> + * @return >> + * - 0: Success; the mempool is now using the requested ops functions. >> + * - -EINVAL - Invalid ops struct name provided. >> + * - -EEXIST - mempool already has an ops struct assigned. >> + */ >> +int >> +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, >> +void *pool_config); >> + > > The changes related to the pool_config look good to me. If you plan to do a v14 for this API comment, I'm wondering if the documentation could be slightly modified too. I think "external mempool manager" was the legacy name for the feature, but maybe it could be changed in "alternative mempool handlers" or "changing the mempool handler". I mean the word "external" is probably not appropriate now, especially if we add other handlers in the mempool lib. My 2 cents, Olivier
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
On 17/6/2016 9:08 AM, Olivier Matz wrote: > Hi David, > > On 06/17/2016 08:58 AM, Hunt, David wrote: >> A comment below: >> >> On 16/6/2016 1:30 PM, David Hunt wrote: >>> +/** >>> + * Set the ops of a mempool. >>> + * >>> + * This can only be done on a mempool that is not populated, i.e. >>> just after >>> + * a call to rte_mempool_create_empty(). >>> + * >>> + * @param mp >>> + * Pointer to the memory pool. >>> + * @param name >>> + * Name of the ops structure to use for this mempool. >> + * @param pool_config >> + * Opaque data that can be used by the ops functions. >>> + * @return >>> + * - 0: Success; the mempool is now using the requested ops functions. >>> + * - -EINVAL - Invalid ops struct name provided. >>> + * - -EEXIST - mempool already has an ops struct assigned. >>> + */ >>> +int >>> +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, >>> +void *pool_config); >>> + >> > The changes related to the pool_config look good to me. > > If you plan to do a v14 for this API comment, I'm wondering if the > documentation could be slightly modified too. I think "external mempool > manager" was the legacy name for the feature, but maybe it could be > changed in "alternative mempool handlers" or "changing the mempool > handler". I mean the word "external" is probably not appropriate now, > especially if we add other handlers in the mempool lib. > > My 2 cents, > Olivier I had not planned on doing another revision. And I think the term "External Mempool Manager" accurately describes the functionality, so I'd really prefer to leave it as it is. Regards, David.
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
A comment below: On 16/6/2016 1:30 PM, David Hunt wrote: > +/** > + * Set the ops of a mempool. > + * > + * This can only be done on a mempool that is not populated, i.e. just after > + * a call to rte_mempool_create_empty(). > + * > + * @param mp > + * Pointer to the memory pool. > + * @param name > + * Name of the ops structure to use for this mempool. + * @param pool_config + * Opaque data that can be used by the ops functions. > + * @return > + * - 0: Success; the mempool is now using the requested ops functions. > + * - -EINVAL - Invalid ops struct name provided. > + * - -EEXIST - mempool already has an ops struct assigned. > + */ > +int > +rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, > + void *pool_config); > +
[dpdk-dev] [PATCH v13 1/3] mempool: support external mempool operations
Until now, the objects stored in a mempool were internally stored in a
ring. This patch introduces the possibility to register external handlers
replacing the ring.
The default behavior remains unchanged, but calling the new function
rte_mempool_set_ops_byname() right after rte_mempool_create_empty() allows
the user to change the handler that will be used when populating
the mempool.
This patch also adds a set of default ops (function callbacks) based
on rte_ring.
Signed-off-by: Olivier Matz
Signed-off-by: David Hunt
Acked-by: Shreyansh Jain
Acked-by: Olivier Matz
---
app/test/test_mempool_perf.c | 1 -
doc/guides/prog_guide/mempool_lib.rst | 31 +++-
doc/guides/rel_notes/deprecation.rst | 9 -
lib/librte_mempool/Makefile| 2 +
lib/librte_mempool/rte_mempool.c | 66 +++-
lib/librte_mempool/rte_mempool.h | 253 ++---
lib/librte_mempool/rte_mempool_ops.c | 150 +
lib/librte_mempool/rte_mempool_ring.c | 161 ++
lib/librte_mempool/rte_mempool_version.map | 13 +-
9 files changed, 605 insertions(+), 81 deletions(-)
create mode 100644 lib/librte_mempool/rte_mempool_ops.c
create mode 100644 lib/librte_mempool/rte_mempool_ring.c
diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c
index c5e3576..c5f8455 100644
--- a/app/test/test_mempool_perf.c
+++ b/app/test/test_mempool_perf.c
@@ -161,7 +161,6 @@ per_lcore_mempool_test(__attribute__((unused)) void *arg)
n_get_bulk);
if (unlikely(ret < 0)) {
rte_mempool_dump(stdout, mp);
- rte_ring_dump(stdout, mp->ring);
/* in this case, objects are lost... */
return -1;
}
diff --git a/doc/guides/prog_guide/mempool_lib.rst
b/doc/guides/prog_guide/mempool_lib.rst
index c3afc2e..2e3116e 100644
--- a/doc/guides/prog_guide/mempool_lib.rst
+++ b/doc/guides/prog_guide/mempool_lib.rst
@@ -34,7 +34,7 @@ Mempool Library
===
A memory pool is an allocator of a fixed-sized object.
-In the DPDK, it is identified by name and uses a ring to store free objects.
+In the DPDK, it is identified by name and uses a ring or an external mempool
manager to store free objects.
It provides some other optional services such as a per-core object cache and
an alignment helper to ensure that objects are padded to spread them equally
on all DRAM or DDR3 channels.
@@ -127,6 +127,35 @@ The maximum size of the cache is static and is defined at
compilation time (CONF
A mempool in Memory with its Associated Ring
+External Mempool Manager
+
+
+This allows external memory subsystems, such as external hardware memory
+management systems and software based memory allocators, to be used with DPDK.
+
+There are two aspects to external mempool manager.
+
+* Adding the code for your new mempool operations (ops). This is achieved by
+ adding a new mempool ops code, and using the ``REGISTER_MEMPOOL_OPS`` macro.
+
+* Using the new API to call ``rte_mempool_create_empty()`` and
+ ``rte_mempool_set_ops_byname()`` to create a new mempool and specifying which
+ ops to use.
+
+Several external mempool managers may be used in the same application. A new
+mempool can be created by using the ``rte_mempool_create_empty()`` function,
+then using ``rte_mempool_set_ops_byname()`` to point the mempool to the
+relevant mempool manager callback (ops) structure.
+
+Legacy applications may continue to use the old ``rte_mempool_create()`` API
+call, which uses a ring based mempool manager by default. These applications
+will need to be modified to use a new external mempool manager.
+
+For applications that use ``rte_pktmbuf_create()``, there is a config setting
+(``RTE_MBUF_DEFAULT_MEMPOOL_OPS``) that allows the application to make use of
+an external mempool manager.
+
+
Use Cases
-
diff --git a/doc/guides/rel_notes/deprecation.rst
b/doc/guides/rel_notes/deprecation.rst
index 7d947ae..c415095 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -39,15 +39,6 @@ Deprecation Notices
compact API. The ones that remain are backwards compatible and use the
per-lcore default cache if available. This change targets release 16.07.
-* The rte_mempool struct will be changed in 16.07 to facilitate the new
- external mempool manager functionality.
- The ring element will be replaced with a more generic 'pool' opaque pointer
- to allow new mempool handlers to use their own user-defined mempool
- layout. Also newly added to rte_mempool is a handler index.
- The existing API will be backward compatible, but there will be new API
- functions added to facilitate the creation of mempools using
