[PATCH v1 2/3] mempool: skip unpopulated pools in walk and dump

[Robert Sanford]

From: Robert Sanford <rsanford at akamai.com>

rte_mempool_create_empty() adds a mempool to the mempool list before
the mempool has a store. The store is allocated, and
RTE_MEMPOOL_F_POOL_CREATED is set, by the first populate call. In
between, rte_mempool_walk() passes the mempool to its callback, and
rte_mempool_list_dump() calls rte_mempool_dump() on it, which reads
the number of objects in the store. With the ring driver this
dereferences a NULL ring, and the process crashes.

Skip mempools that do not have RTE_MEMPOOL_F_POOL_CREATED set in
rte_mempool_walk() and rte_mempool_list_dump(), and document it. The
populating thread sets the flag without holding a lock, so add a
release fence before setting it and an acquire fence after checking
it. A walker that sees the flag then also sees the store.

rte_mempool_lookup() is unchanged. It does not access the store, and
callers may look up a mempool that is not populated yet.

Fixes: 449c49b93a6b ("mempool: support handler operations")
Cc: stable at dpdk.org

Signed-off-by: Robert Sanford <rsanford at akamai.com>
---
 lib/mempool/rte_mempool.c | 20 +++++++++++++++++++-
 lib/mempool/rte_mempool.h |  3 +++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
index d9f339a3c2..7c4d4697c7 100644
--- a/lib/mempool/rte_mempool.c
+++ b/lib/mempool/rte_mempool.c
@@ -304,6 +304,12 @@ mempool_ops_alloc_once(struct rte_mempool *mp)
 		ret = rte_mempool_ops_alloc(mp);
 		if (ret != 0)
 			return ret;
+		/*
+		 * rte_mempool_walk() and rte_mempool_list_dump() access the
+		 * store of pools with this flag set. Make the store visible
+		 * before the flag.
+		 */
+		rte_atomic_thread_fence(rte_memory_order_release);
 		mp->flags |= RTE_MEMPOOL_F_POOL_CREATED;
 	}
 	return 0;
@@ -1412,6 +1418,11 @@ rte_mempool_list_dump(FILE *f)
 
 	TAILQ_FOREACH(te, mempool_list, next) {
 		mp = (struct rte_mempool *) te->data;
+		/* skip mempools that are not populated yet */
+		if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
+			continue;
+		/* pairs with the release fence in mempool_ops_alloc_once() */
+		rte_atomic_thread_fence(rte_memory_order_acquire);
 		rte_mempool_dump(f, mp);
 	}
 
@@ -1460,7 +1471,14 @@ void rte_mempool_walk(void (*func)(struct rte_mempool *, void *),
 	rte_mcfg_mempool_read_lock();
 
 	RTE_TAILQ_FOREACH_SAFE(te, mempool_list, next, tmp_te) {
-		(*func)((struct rte_mempool *) te->data, arg);
+		struct rte_mempool *mp = (struct rte_mempool *) te->data;
+
+		/* skip mempools that are not populated yet */
+		if ((mp->flags & RTE_MEMPOOL_F_POOL_CREATED) == 0)
+			continue;
+		/* pairs with the release fence in mempool_ops_alloc_once() */
+		rte_atomic_thread_fence(rte_memory_order_acquire);
+		(*func)(mp, arg);
 	}
 
 	rte_mcfg_mempool_read_unlock();
diff --git a/lib/mempool/rte_mempool.h b/lib/mempool/rte_mempool.h
index bd58d8a9d5..7782768a2d 100644
--- a/lib/mempool/rte_mempool.h
+++ b/lib/mempool/rte_mempool.h
@@ -1907,6 +1907,8 @@ static inline void *rte_mempool_get_priv(struct rte_mempool *mp)
 /**
  * Dump the status of all mempools on the console
  *
+ * Mempools that are not populated yet are skipped.
+ *
  * @param f
  *   A pointer to a file for output
  */
@@ -1949,6 +1951,7 @@ uint32_t rte_mempool_calc_obj_size(uint32_t elt_size, uint32_t flags,
  *
  * The mempool list is locked while the iterator function runs, so the
  * iterator function must not create or free a mempool.
+ * Mempools that are not populated yet are skipped.
  *
  * @param func
  *   Iterator function
-- 
2.34.1