[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
On Tue, 31 May 2016 22:40:59 +0200
Olivier MATZ wrote:
> Hi,
>
> On 05/31/2016 03:47 PM, Hunt, David wrote:
> > On 5/31/2016 1:06 PM, Jan Viktorin wrote:
> >> On Tue, 31 May 2016 10:09:42 +0100
> >> "Hunt, David" wrote:
> >>
> >>> The *p pointer is the opaque data for a given mempool handler (ring,
> >>> array, linked list, etc)
> >> Again, doc comments...
> >>
> >> I don't like the obj_table representation to be an array of void *. I
> >> could see
> >> it already in DPDK for defining Ethernet driver queues, so, it's
> >> probably not
> >> an issue. I just say, I would prefer some basic type safety like
> >>
> >> struct rte_mempool_obj {
> >> void *p;
> >> };
> >>
> >> Is there somebody with different opinions?
> >>
> >> [...]
> >
> > Comments added. I've left as a void* for the moment.
>
> Jan, could you please detail why you think having a
> rte_mempool_obj structure brings more safety?
First, void * does not say anything about the purpose of the argument.
So, anybody, who is not familiar with the mempool internals would be
lost for a while (as I was when studying the Ethernet queue API of DPDK).
The type safety (in C, LoL) here is in the means of messing up order of
arguments.
When there are more void * args, you can accidently pass something wrong.
DPDK has quite strict compiler settings, however, I don't consider it to be
a good practice in general.
When you have a named struct or a typedef, its definition usually contains doc
comments describing its purposes. Nobody usually writes good doc comments for
void * arguments of functions.
>
> For now, I'm in favor of keeping the array of void *, because
> that's what we use in other mempool or ring functions.
It was just a suggestion... I don't consider this to be an issue (as stated
earlier).
Jan
>
>
> > +/** Structure defining a mempool handler. */
> Later in the text, I suggested to rename rte_mempool_handler to
> rte_mempool_ops.
> I believe that it explains the purpose of this struct better. It
> would improve
> consistency in function names (the *_ext_* mark is very strange and
> inconsistent).
> >>> I agree. I've gone through all the code and renamed to
> >>> rte_mempool_handler_ops.
> >> Ok. I meant rte_mempool_ops because I find the word "handler" to be
> >> redundant.
> >
> > I prefer the use of the word handler, unless others also have opinions
> > either way?
>
> Well, I think rte_mempool_ops is clear enough, and shorter,
> so I'd vote for it.
>
>
> Regards,
> Olivier
--
Jan Viktorin E-mail: Viktorin at RehiveTech.com
System Architect Web:www.RehiveTech.com
RehiveTech
Brno, Czech Republic
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
On 5/31/2016 9:40 PM, Olivier MATZ wrote: [...] >> +/** Structure defining a mempool handler. */ > Later in the text, I suggested to rename rte_mempool_handler to > rte_mempool_ops. > I believe that it explains the purpose of this struct better. It > would improve > consistency in function names (the *_ext_* mark is very strange and > inconsistent). I agree. I've gone through all the code and renamed to rte_mempool_handler_ops. >>> Ok. I meant rte_mempool_ops because I find the word "handler" to be >>> redundant. >> I prefer the use of the word handler, unless others also have opinions >> either way? > Well, I think rte_mempool_ops is clear enough, and shorter, > so I'd vote for it. > OK, I've just changed it. It will be in next revision. :) Regards, Dave.
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
Hi,
On 05/31/2016 03:47 PM, Hunt, David wrote:
> On 5/31/2016 1:06 PM, Jan Viktorin wrote:
>> On Tue, 31 May 2016 10:09:42 +0100
>> "Hunt, David" wrote:
>>
>>> The *p pointer is the opaque data for a given mempool handler (ring,
>>> array, linked list, etc)
>> Again, doc comments...
>>
>> I don't like the obj_table representation to be an array of void *. I
>> could see
>> it already in DPDK for defining Ethernet driver queues, so, it's
>> probably not
>> an issue. I just say, I would prefer some basic type safety like
>>
>> struct rte_mempool_obj {
>> void *p;
>> };
>>
>> Is there somebody with different opinions?
>>
>> [...]
>
> Comments added. I've left as a void* for the moment.
Jan, could you please detail why you think having a
rte_mempool_obj structure brings more safety?
For now, I'm in favor of keeping the array of void *, because
that's what we use in other mempool or ring functions.
> +/** Structure defining a mempool handler. */
Later in the text, I suggested to rename rte_mempool_handler to
rte_mempool_ops.
I believe that it explains the purpose of this struct better. It
would improve
consistency in function names (the *_ext_* mark is very strange and
inconsistent).
>>> I agree. I've gone through all the code and renamed to
>>> rte_mempool_handler_ops.
>> Ok. I meant rte_mempool_ops because I find the word "handler" to be
>> redundant.
>
> I prefer the use of the word handler, unless others also have opinions
> either way?
Well, I think rte_mempool_ops is clear enough, and shorter,
so I'd vote for it.
Regards,
Olivier
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
On 5/31/2016 1:06 PM, Jan Viktorin wrote:
> On Tue, 31 May 2016 10:09:42 +0100
> "Hunt, David" wrote:
>
>> Hi Jan,
>>
> [...]
>
+typedef void *(*rte_mempool_alloc_t)(struct rte_mempool *mp);
+
+/** Free the external pool. */
>>> What is the purpose of this callback?
>>> What exactly does it allocate?
>>> Some rte_mempool internals?
>>> Or the memory?
>>> What does it return?
>> This is the main allocate function of the handler. It is up to the
>> mempool handlers control.
>> The handler's alloc function does whatever it needs to do to grab memory
>> for this handler, and places
>> a pointer to it in the *pool opaque pointer in the rte_mempool struct.
>> In the default handler, *pool
>> points to a ring, in other handlers, it will mostlikely point to a
>> different type of data structure. It will
>> be transparent to the application programmer.
> Thanks for explanation. Please, add doc comments there.
>
> Should the *mp be const here? It is not clear to me whether the callback is
> expected to modify the mempool or not (eg. set the pool pointer).
Comment added. Not const, as the *pool is set.
+typedef void (*rte_mempool_free_t)(void *p);
+
+/** Put an object in the external pool. */
>>> Why this *_free callback does not accept the rte_mempool param?
>>>
>> We're freeing the pool opaque data here.
> Add doc comments...
Done.
>>
+typedef int (*rte_mempool_put_t)(void *p, void * const *obj_table,
unsigned n);
>>> What is the *p pointer?
>>> What is the obj_table?
>>> Why is it void *?
>>> Why is it const?
>>>
>> The *p pointer is the opaque data for a given mempool handler (ring,
>> array, linked list, etc)
> Again, doc comments...
>
> I don't like the obj_table representation to be an array of void *. I could
> see
> it already in DPDK for defining Ethernet driver queues, so, it's probably not
> an issue. I just say, I would prefer some basic type safety like
>
> struct rte_mempool_obj {
> void *p;
> };
>
> Is there somebody with different opinions?
>
> [...]
Comments added. I've left as a void* for the moment.
+
+/** Structure defining a mempool handler. */
>>> Later in the text, I suggested to rename rte_mempool_handler to
>>> rte_mempool_ops.
>>> I believe that it explains the purpose of this struct better. It would
>>> improve
>>> consistency in function names (the *_ext_* mark is very strange and
>>> inconsistent).
>> I agree. I've gone through all the code and renamed to
>> rte_mempool_handler_ops.
> Ok. I meant rte_mempool_ops because I find the word "handler" to be redundant.
I prefer the use of the word handler, unless others also have opinions
either way?
>
+/**
+ * Set the handler 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 handler.
+ * @return
+ * - 0: Sucess; the new handler is configured.
+ * - <0: Error (errno)
>>> Should this doc be more specific about the possible failures?
>>>
>>> The body of rte_mempool_set_handler does not set errno at all.
>>> It returns e.g. -EEXIST.
>> This is up to the handler. We cannot know what codes will be returned at
>> this stage.
>> errno was a cut-and paste error, this is now fixed.
> I don't think so. The rte_mempool_set_handler is not handler-specific:
[...]
> So, it is possible to define those in the doc comment.
Ah, I see now. My mistake, I assumed a different function. Added now.
>>
+ */
+int
+rte_mempool_set_handler(struct rte_mempool *mp, const char *name);
+
+/**
+ * Register an external pool handler.
+ *
+ * @param h
+ * Pointer to the external pool handler
+ * @return
+ * - >=0: Sucess; return the index of the handler in the table.
+ * - <0: Error (errno)
>>> Should this doc be more specific about the possible failures?
>> This is up to the handler. We cannot know what codes will be returned at
>> this stage.
>> errno was a cut-and paste error, this is now fixed.
> Same, here... -ENOSPC, -EINVAL are returned in certain cases. And again,
> this call is not handler-specific.
Yes, Added now to comments.
>
> [...]
>
+
+static struct rte_mempool_handler handler_mp_mc = {
+ .name = "ring_mp_mc",
+ .alloc = common_ring_alloc,
+ .free = common_ring_free,
+ .put = common_ring_mp_put,
+ .get = common_ring_mc_get,
+ .get_count = common_ring_get_count,
+};
+
+static struct rte_mempool_handler handler_sp_sc = {
+ .name = "ring_sp_sc",
+ .alloc = common_ring_alloc,
+ .free = common_ring_free,
+ .put = common_ring_sp_put,
+ .get = common_ring_sc_get,
+ .get_count = common_ring_get_count,
+};
+
+static struct rte_mempool_handler handler_mp_sc = {
>>
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
On Tue, 31 May 2016 10:09:42 +0100
"Hunt, David" wrote:
> Hi Jan,
>
[...]
>
> >> +typedef void *(*rte_mempool_alloc_t)(struct rte_mempool *mp);
> >> +
> >> +/** Free the external pool. */
> > What is the purpose of this callback?
> > What exactly does it allocate?
> > Some rte_mempool internals?
> > Or the memory?
> > What does it return?
>
> This is the main allocate function of the handler. It is up to the
> mempool handlers control.
> The handler's alloc function does whatever it needs to do to grab memory
> for this handler, and places
> a pointer to it in the *pool opaque pointer in the rte_mempool struct.
> In the default handler, *pool
> points to a ring, in other handlers, it will mostlikely point to a
> different type of data structure. It will
> be transparent to the application programmer.
Thanks for explanation. Please, add doc comments there.
Should the *mp be const here? It is not clear to me whether the callback is
expected to modify the mempool or not (eg. set the pool pointer).
>
> >> +typedef void (*rte_mempool_free_t)(void *p);
> >> +
> >> +/** Put an object in the external pool. */
> > Why this *_free callback does not accept the rte_mempool param?
> >
>
> We're freeing the pool opaque data here.
Add doc comments...
>
>
> >> +typedef int (*rte_mempool_put_t)(void *p, void * const *obj_table,
> >> unsigned n);
> > What is the *p pointer?
> > What is the obj_table?
> > Why is it void *?
> > Why is it const?
> >
>
> The *p pointer is the opaque data for a given mempool handler (ring,
> array, linked list, etc)
Again, doc comments...
I don't like the obj_table representation to be an array of void *. I could see
it already in DPDK for defining Ethernet driver queues, so, it's probably not
an issue. I just say, I would prefer some basic type safety like
struct rte_mempool_obj {
void *p;
};
Is there somebody with different opinions?
[...]
> >> +
> >> +/** Structure defining a mempool handler. */
> > Later in the text, I suggested to rename rte_mempool_handler to
> > rte_mempool_ops.
> > I believe that it explains the purpose of this struct better. It would
> > improve
> > consistency in function names (the *_ext_* mark is very strange and
> > inconsistent).
>
> I agree. I've gone through all the code and renamed to
> rte_mempool_handler_ops.
Ok. I meant rte_mempool_ops because I find the word "handler" to be redundant.
>
[...]
> >> +/**
> >> + * Set the handler 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 handler.
> >> + * @return
> >> + * - 0: Sucess; the new handler is configured.
> >> + * - <0: Error (errno)
> > Should this doc be more specific about the possible failures?
> >
> > The body of rte_mempool_set_handler does not set errno at all.
> > It returns e.g. -EEXIST.
>
> This is up to the handler. We cannot know what codes will be returned at
> this stage.
> errno was a cut-and paste error, this is now fixed.
I don't think so. The rte_mempool_set_handler is not handler-specific:
116 /* set the handler of a mempool */
117 int
118 rte_mempool_set_handler(struct rte_mempool *mp, const char *name)
119 {
120 struct rte_mempool_handler *handler = NULL;
121 unsigned i;
122
123 /* too late, the mempool is already populated */
124 if (mp->flags & MEMPOOL_F_RING_CREATED)
125 return -EEXIST;
Here, it returns -EEXIST.
126
127 for (i = 0; i < rte_mempool_handler_table.num_handlers; i++) {
128 if (!strcmp(name,
rte_mempool_handler_table.handler[i].name)) {
129 handler = &rte_mempool_handler_table.handler[i];
130 break;
131 }
132 }
133
134 if (handler == NULL)
135 return -EINVAL;
And here, it returns -EINVAL.
136
137 mp->handler_idx = i;
138 return 0;
139 }
So, it is possible to define those in the doc comment.
>
>
> >> + */
> >> +int
> >> +rte_mempool_set_handler(struct rte_mempool *mp, const char *name);
> >> +
> >> +/**
> >> + * Register an external pool handler.
> >> + *
> >> + * @param h
> >> + * Pointer to the external pool handler
> >> + * @return
> >> + * - >=0: Sucess; return the index of the handler in the table.
> >> + * - <0: Error (errno)
> > Should this doc be more specific about the possible failures?
>
> This is up to the handler. We cannot know what codes will be returned at
> this stage.
> errno was a cut-and paste error, this is now fixed.
Same, here... -ENOSPC, -EINVAL are returned in certain cases. And again,
this call is not handler-specific.
>
[...]
> >>
> >> +
> >> +static struct rte_mempool_handler handler_mp_mc = {
> >> + .name = "ring_mp_mc",
> >> + .alloc = common_ring_
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
Hi Jan,
On 5/23/2016 1:35 PM, Jan Viktorin wrote:
>> Until now, the objects stored in mempool mempool were internally stored a
> s/mempool mempool/mempool/
>
> stored _in_ a ring?
Fixed.
>
> @@ -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;
> }
> I think, this should be in a separate patch explaining the reason to remove
> it.
Done. Moved.
>> diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
>> index 43423e0..f19366e 100644
>> --- a/lib/librte_mempool/Makefile
>> +++ b/lib/librte_mempool/Makefile
>> @@ -42,6 +42,8 @@ LIBABIVER := 2
>>
>> # all source are stored in SRCS-y
>> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool.c
>> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_handler.c
>> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_default.c
>> # install includes
>> SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
>>
>> diff --git a/lib/librte_mempool/rte_mempool.c
>> b/lib/librte_mempool/rte_mempool.c
>> index 1ab6701..6ec2b3f 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -148,7 +148,7 @@ mempool_add_elem(struct rte_mempool *mp, void *obj,
>> phys_addr_t physaddr)
>> #endif
>>
>> /* enqueue in ring */
>> -rte_ring_sp_enqueue(mp->ring, obj);
>> +rte_mempool_ext_put_bulk(mp, &obj, 1);
> I suppose this is OK, however, replacing "enqueue" by "put" (semantically)
> sounds to me
> like a bug. Enqueue is put into a queue. Put is to drop a reference.
Yes, Makes sense. Changed 'put' and 'get' to 'enqueue' and 'dequeue'
>
>>
>> -/* create the internal ring */
>> -static int
>> -rte_mempool_ring_create(struct rte_mempool *mp)
>> -{
>> -int rg_flags = 0, ret;
>> -char rg_name[RTE_RING_NAMESIZE];
>> -struct rte_ring *r;
>> -
>> -ret = snprintf(rg_name, sizeof(rg_name),
>> -RTE_MEMPOOL_MZ_FORMAT, mp->name);
>> -if (ret < 0 || ret >= (int)sizeof(rg_name))
>> -return -ENAMETOOLONG;
>> -
>> -/* ring flags */
>> -if (mp->flags & MEMPOOL_F_SP_PUT)
>> -rg_flags |= RING_F_SP_ENQ;
>> -if (mp->flags & MEMPOOL_F_SC_GET)
>> -rg_flags |= RING_F_SC_DEQ;
>> -
>> -/* Allocate the ring that will be used to store objects.
>> - * Ring functions will return appropriate errors if we are
>> - * running as a secondary process etc., so no checks made
>> - * in this function for that condition.
>> - */
>> -r = rte_ring_create(rg_name, rte_align32pow2(mp->size + 1),
>> -mp->socket_id, rg_flags);
>> -if (r == NULL)
>> -return -rte_errno;
>> -
>> -mp->ring = r;
>> -mp->flags |= MEMPOOL_F_RING_CREATED;
>> -return 0;
>> -}
> This is a big change. I suggest (if possible) to make a separate patch with
> something like "replace rte_mempool_ring_create by ...". Where is this code
> placed now?
The code is not gone away, it's now part of the default handler, which
uses a ring. It's
in rte_mempool_default.c
>> -
>> /* free a memchunk allocated with rte_memzone_reserve() */
>> static void
>> rte_mempool_memchunk_mz_free(__rte_unused struct rte_mempool_memhdr
>> *memhdr,
>> @@ -351,7 +317,7 @@ rte_mempool_free_memchunks(struct rte_mempool *mp)
>> void *elt;
>>
>> while (!STAILQ_EMPTY(&mp->elt_list)) {
>> -rte_ring_sc_dequeue(mp->ring, &elt);
>> +rte_mempool_ext_get_bulk(mp, &elt, 1);
> Similar as for put_bulk... Replacing "dequeue" by "get" (semantically) sounds
> to me
> like a bug. Dequeue is drop from a queue. Get is to obtain a reference.
Done
>> (void)elt;
>> STAILQ_REMOVE_HEAD(&mp->elt_list, next);
>> mp->populated_size--;
>> @@ -380,15 +346,18 @@ rte_mempool_populate_phys(struct rte_mempool *mp, char
>> *vaddr,
>> unsigned i = 0;
>> size_t off;
>> struct rte_mempool_memhdr *memhdr;
>> -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)
>> -return ret;
>> +rte_errno = 0;
>> +mp->pool = rte_mempool_ext_alloc(mp);
>> +if (mp->pool == NULL) {
>> +if (rte_errno == 0)
>> +return -EINVAL;
>> +else
>> +return -rte_errno;
>> +}
>> }
>> -
> Is this a whitespace change?
Accidental. Reverted
>> +
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
On 5/23/2016 1:35 PM, Jan Viktorin wrote: > Hello David, > > please, see my comments inline. > > I didn't see the previous versions of the mempool (well, only very roughly) > so I am > probably missing some points... My point of view is as a user of the handler > API. > I need to understand the API to implement a custom handler for my purposes. Thanks for the review, Jan. I'm working on the changes now, will post soon. I'll reply to each of you're emails when I'm ready with the patch. Regards, David.
[dpdk-dev] [dpdk-dev,v5,1/3] mempool: support external handler
Hello David,
please, see my comments inline.
I didn't see the previous versions of the mempool (well, only very roughly) so
I am
probably missing some points... My point of view is as a user of the handler
API.
I need to understand the API to implement a custom handler for my purposes.
On Thu, 19 May 2016 14:44:59 +0100
David Hunt wrote:
> Until now, the objects stored in mempool mempool were internally stored a
s/mempool mempool/mempool/
stored _in_ a ring?
> ring. This patch introduce the possibility to register external handlers
> replacing the ring.
>
> The default behavior remains unchanged, but calling the new function
> rte_mempool_set_handler() right after rte_mempool_create_empty() allows to
> change the handler that will be used when populating the mempool.
>
> v5 changes: rebasing on top of 35 patch set mempool work.
>
> Signed-off-by: David Hunt
> Signed-off-by: Olivier Matz
>
> ---
> app/test/test_mempool_perf.c | 1 -
> lib/librte_mempool/Makefile| 2 +
> lib/librte_mempool/rte_mempool.c | 73 --
> lib/librte_mempool/rte_mempool.h | 212
> +
> lib/librte_mempool/rte_mempool_default.c | 147
> lib/librte_mempool/rte_mempool_handler.c | 139 +++
> lib/librte_mempool/rte_mempool_version.map | 4 +
> 7 files changed, 506 insertions(+), 72 deletions(-)
> create mode 100644 lib/librte_mempool/rte_mempool_default.c
> create mode 100644 lib/librte_mempool/rte_mempool_handler.c
>
> diff --git a/app/test/test_mempool_perf.c b/app/test/test_mempool_perf.c
> index cdc02a0..091c1df 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;
> }
I think, this should be in a separate patch explaining the reason to remove it.
> diff --git a/lib/librte_mempool/Makefile b/lib/librte_mempool/Makefile
> index 43423e0..f19366e 100644
> --- a/lib/librte_mempool/Makefile
> +++ b/lib/librte_mempool/Makefile
> @@ -42,6 +42,8 @@ LIBABIVER := 2
>
> # all source are stored in SRCS-y
> SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_handler.c
> +SRCS-$(CONFIG_RTE_LIBRTE_MEMPOOL) += rte_mempool_default.c
> # install includes
> SYMLINK-$(CONFIG_RTE_LIBRTE_MEMPOOL)-include := rte_mempool.h
>
> diff --git a/lib/librte_mempool/rte_mempool.c
> b/lib/librte_mempool/rte_mempool.c
> index 1ab6701..6ec2b3f 100644
> --- a/lib/librte_mempool/rte_mempool.c
> +++ b/lib/librte_mempool/rte_mempool.c
> @@ -148,7 +148,7 @@ mempool_add_elem(struct rte_mempool *mp, void *obj,
> phys_addr_t physaddr)
> #endif
>
> /* enqueue in ring */
> - rte_ring_sp_enqueue(mp->ring, obj);
> + rte_mempool_ext_put_bulk(mp, &obj, 1);
I suppose this is OK, however, replacing "enqueue" by "put" (semantically)
sounds to me
like a bug. Enqueue is put into a queue. Put is to drop a reference.
> }
>
> /* call obj_cb() for each mempool element */
> @@ -300,40 +300,6 @@ rte_mempool_xmem_usage(__rte_unused void *vaddr,
> uint32_t elt_num,
> return (size_t)paddr_idx << pg_shift;
> }
>
> -/* create the internal ring */
> -static int
> -rte_mempool_ring_create(struct rte_mempool *mp)
> -{
> - int rg_flags = 0, ret;
> - char rg_name[RTE_RING_NAMESIZE];
> - struct rte_ring *r;
> -
> - ret = snprintf(rg_name, sizeof(rg_name),
> - RTE_MEMPOOL_MZ_FORMAT, mp->name);
> - if (ret < 0 || ret >= (int)sizeof(rg_name))
> - return -ENAMETOOLONG;
> -
> - /* ring flags */
> - if (mp->flags & MEMPOOL_F_SP_PUT)
> - rg_flags |= RING_F_SP_ENQ;
> - if (mp->flags & MEMPOOL_F_SC_GET)
> - rg_flags |= RING_F_SC_DEQ;
> -
> - /* Allocate the ring that will be used to store objects.
> - * Ring functions will return appropriate errors if we are
> - * running as a secondary process etc., so no checks made
> - * in this function for that condition.
> - */
> - r = rte_ring_create(rg_name, rte_align32pow2(mp->size + 1),
> - mp->socket_id, rg_flags);
> - if (r == NULL)
> - return -rte_errno;
> -
> - mp->ring = r;
> - mp->flags |= MEMPOOL_F_RING_CREATED;
> - return 0;
> -}
This is a big change. I suggest (if possible) to make a separate patch with
something like "replace rte_mempool_ring_create by ...". Where is this code
placed now?
> -
> /* free a memchunk allocated with rte_me
