All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.