DPDK patches and discussions
 help / color / mirror / Atom feed
From: Dmitry Kozlyuk <dkozlyuk@nvidia.com>
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
Date: Mon, 8 Aug 2022 12:42:35 +0300	[thread overview]
Message-ID: <20220808094236.3395516-2-dkozlyuk@nvidia.com> (raw)
In-Reply-To: <20220808094236.3395516-1-dkozlyuk@nvidia.com>

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(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index de59009baf..1e039f6247 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -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;
 }
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index 3ada37cb86..bfc1f3c823 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -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.
-- 
2.25.1


  reply	other threads:[~2022-08-08  9:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-08  9:42 [PATCH 0/2] common/mlx5: fix multi-process mempool registration Dmitry Kozlyuk
2022-08-08  9:42 ` Dmitry Kozlyuk [this message]
2022-08-28 18:33   ` [PATCH 1/2] mempool: make event callbacks process-private Slava Ovsiienko
2022-10-10  8:02     ` Andrew Rybchenko
2022-09-22  7:31   ` Dmitry Kozlyuk
2022-08-08  9:42 ` [PATCH 2/2] common/mlx5: fix multi-process mempool registration Dmitry Kozlyuk
2022-08-28 18:34   ` Slava Ovsiienko
2022-10-10 13:20 ` [PATCH 0/2] " Thomas Monjalon

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=20220808094236.3395516-2-dkozlyuk@nvidia.com \
    --to=dkozlyuk@nvidia.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=olivier.matz@6wind.com \
    --cc=stable@dpdk.org \
    /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).