[1/2] mempool: make event callbacks process-private
Checks
Commit Message
Callbacks for mempool events were registered in a process-shared tailq.
This was inherently incorrect because the same function
may be loaded to a different address in each process.
Make the tailq process-private.
Use the EAL tailq lock to reduce the number of different locks
this module operates.
Fixes: da2b9cb25e5f ("mempool: add event callbacks")
Cc: stable@dpdk.org
Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
---
lib/mempool/rte_mempool.c | 54 ++++++++++-----------------------------
lib/mempool/rte_mempool.h | 2 ++
2 files changed, 16 insertions(+), 40 deletions(-)
Comments
> -----Original Message-----
> From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Sent: Monday, August 8, 2022 12:43
> To: dev@dpdk.org
> Cc: Olivier Matz <olivier.matz@6wind.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; stable@dpdk.org
> Subject: [PATCH 1/2] mempool: make event callbacks process-private
>
> Callbacks for mempool events were registered in a process-shared tailq.
> This was inherently incorrect because the same function may be loaded to a
> different address in each process.
> Make the tailq process-private.
> Use the EAL tailq lock to reduce the number of different locks this module
> operates.
>
> Fixes: da2b9cb25e5f ("mempool: add event callbacks")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Sent: Monday, August 8, 2022 12:43 PM
> To: dev@dpdk.org
> Cc: Olivier Matz <olivier.matz@6wind.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; stable@dpdk.org
> Subject: [PATCH 1/2] mempool: make event callbacks process-private
>
> Callbacks for mempool events were registered in a process-shared tailq.
> This was inherently incorrect because the same function
> may be loaded to a different address in each process.
> Make the tailq process-private.
> Use the EAL tailq lock to reduce the number of different locks
> this module operates.
>
> Fixes: da2b9cb25e5f ("mempool: add event callbacks")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
Kind reminder to the mempool maintainers.
On 8/28/22 21:33, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
>> Sent: Monday, August 8, 2022 12:43
>> To: dev@dpdk.org
>> Cc: Olivier Matz <olivier.matz@6wind.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; stable@dpdk.org
>> Subject: [PATCH 1/2] mempool: make event callbacks process-private
>>
>> Callbacks for mempool events were registered in a process-shared tailq.
>> This was inherently incorrect because the same function may be loaded to a
>> different address in each process.
>> Make the tailq process-private.
>> Use the EAL tailq lock to reduce the number of different locks this module
>> operates.
>>
>> Fixes: da2b9cb25e5f ("mempool: add event callbacks")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
@@ -36,12 +36,10 @@ static struct rte_tailq_elem rte_mempool_tailq = {
};
EAL_REGISTER_TAILQ(rte_mempool_tailq)
-TAILQ_HEAD(mempool_callback_list, rte_tailq_entry);
+TAILQ_HEAD(mempool_callback_tailq, mempool_callback_data);
-static struct rte_tailq_elem callback_tailq = {
- .name = "RTE_MEMPOOL_CALLBACK",
-};
-EAL_REGISTER_TAILQ(callback_tailq)
+static struct mempool_callback_tailq callback_tailq =
+ TAILQ_HEAD_INITIALIZER(callback_tailq);
/* Invoke all registered mempool event callbacks. */
static void
@@ -1372,6 +1370,7 @@ void rte_mempool_walk(void (*func)(struct rte_mempool *, void *),
}
struct mempool_callback_data {
+ TAILQ_ENTRY(mempool_callback_data) callbacks;
rte_mempool_event_callback *func;
void *user_data;
};
@@ -1380,14 +1379,11 @@ static void
mempool_event_callback_invoke(enum rte_mempool_event event,
struct rte_mempool *mp)
{
- struct mempool_callback_list *list;
- struct rte_tailq_entry *te;
+ struct mempool_callback_data *cb;
void *tmp_te;
rte_mcfg_tailq_read_lock();
- list = RTE_TAILQ_CAST(callback_tailq.head, mempool_callback_list);
- RTE_TAILQ_FOREACH_SAFE(te, list, next, tmp_te) {
- struct mempool_callback_data *cb = te->data;
+ RTE_TAILQ_FOREACH_SAFE(cb, &callback_tailq, callbacks, tmp_te) {
rte_mcfg_tailq_read_unlock();
cb->func(event, mp, cb->user_data);
rte_mcfg_tailq_read_lock();
@@ -1399,10 +1395,7 @@ int
rte_mempool_event_callback_register(rte_mempool_event_callback *func,
void *user_data)
{
- struct mempool_callback_list *list;
- struct rte_tailq_entry *te = NULL;
struct mempool_callback_data *cb;
- void *tmp_te;
int ret;
if (func == NULL) {
@@ -1411,36 +1404,23 @@ rte_mempool_event_callback_register(rte_mempool_event_callback *func,
}
rte_mcfg_tailq_write_lock();
- list = RTE_TAILQ_CAST(callback_tailq.head, mempool_callback_list);
- RTE_TAILQ_FOREACH_SAFE(te, list, next, tmp_te) {
- cb = te->data;
+ TAILQ_FOREACH(cb, &callback_tailq, callbacks) {
if (cb->func == func && cb->user_data == user_data) {
ret = -EEXIST;
goto exit;
}
}
- te = rte_zmalloc("mempool_cb_tail_entry", sizeof(*te), 0);
- if (te == NULL) {
- RTE_LOG(ERR, MEMPOOL,
- "Cannot allocate event callback tailq entry!\n");
- ret = -ENOMEM;
- goto exit;
- }
-
- cb = rte_malloc("mempool_cb_data", sizeof(*cb), 0);
+ cb = calloc(1, sizeof(*cb));
if (cb == NULL) {
- RTE_LOG(ERR, MEMPOOL,
- "Cannot allocate event callback!\n");
- rte_free(te);
+ RTE_LOG(ERR, MEMPOOL, "Cannot allocate event callback!\n");
ret = -ENOMEM;
goto exit;
}
cb->func = func;
cb->user_data = user_data;
- te->data = cb;
- TAILQ_INSERT_TAIL(list, te, next);
+ TAILQ_INSERT_TAIL(&callback_tailq, cb, callbacks);
ret = 0;
exit:
@@ -1453,27 +1433,21 @@ int
rte_mempool_event_callback_unregister(rte_mempool_event_callback *func,
void *user_data)
{
- struct mempool_callback_list *list;
- struct rte_tailq_entry *te = NULL;
struct mempool_callback_data *cb;
int ret = -ENOENT;
rte_mcfg_tailq_write_lock();
- list = RTE_TAILQ_CAST(callback_tailq.head, mempool_callback_list);
- TAILQ_FOREACH(te, list, next) {
- cb = te->data;
+ TAILQ_FOREACH(cb, &callback_tailq, callbacks) {
if (cb->func == func && cb->user_data == user_data) {
- TAILQ_REMOVE(list, te, next);
+ TAILQ_REMOVE(&callback_tailq, cb, callbacks);
ret = 0;
break;
}
}
rte_mcfg_tailq_write_unlock();
- if (ret == 0) {
- rte_free(te);
- rte_free(cb);
- }
+ if (ret == 0)
+ free(cb);
rte_errno = -ret;
return ret;
}
@@ -1847,6 +1847,8 @@ typedef void (rte_mempool_event_callback)(
* Register a callback function invoked on mempool life cycle event.
* The function will be invoked in the process
* that performs an action which triggers the callback.
+ * Registration is process-private,
+ * i.e. each process must manage callbacks on its own if needed.
*
* @param func
* Callback function.