[dpdk-dev] [PATCH v2 2/4] malloc: separate destroying memseg list and heap data

[Anatoly Burakov <anatoly.burakov@intel.com>]

Currently, destroying external heap chunk and its memseg list is
part of one process. When we will gain the ability to unregister
external memory from DPDK that doesn't have any heap structures
associated with it, we need to be able to find and destroy
memseg lists as well as heap data separately.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Yongseok Koh <yskoh@mellanox.com>
---
 lib/librte_eal/common/malloc_heap.c |  70 +++++++++++++++----
 lib/librte_eal/common/malloc_heap.h |   6 ++
 lib/librte_eal/common/rte_malloc.c  | 104 ++++++++++------------------
 3 files changed, 102 insertions(+), 78 deletions(-)

diff --git a/lib/librte_eal/common/malloc_heap.c b/lib/librte_eal/common/malloc_heap.c
index 25693481f..fa0cb0799 100644
--- a/lib/librte_eal/common/malloc_heap.c
+++ b/lib/librte_eal/common/malloc_heap.c
@@ -1067,12 +1067,9 @@ malloc_heap_dump(struct malloc_heap *heap, FILE *f)
 }
 
 static int
-destroy_seg(struct malloc_elem *elem, size_t len)
+destroy_elem(struct malloc_elem *elem, size_t len)
 {
 	struct malloc_heap *heap = elem->heap;
-	struct rte_memseg_list *msl;
-
-	msl = elem->msl;
 
 	/* notify all subscribers that a memory area is going to be removed */
 	eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE, elem, len);
@@ -1085,13 +1082,6 @@ destroy_seg(struct malloc_elem *elem, size_t len)
 
 	memset(elem, 0, sizeof(*elem));
 
-	/* destroy the fbarray backing this memory */
-	if (rte_fbarray_destroy(&msl->memseg_arr) < 0)
-		return -1;
-
-	/* reset the memseg list */
-	memset(msl, 0, sizeof(*msl));
-
 	return 0;
 }
 
@@ -1158,6 +1148,62 @@ malloc_heap_create_external_seg(void *va_addr, rte_iova_t iova_addrs[],
 	return msl;
 }
 
+struct extseg_walk_arg {
+	void *va_addr;
+	size_t len;
+	struct rte_memseg_list *msl;
+};
+
+static int
+extseg_walk(const struct rte_memseg_list *msl, void *arg)
+{
+	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
+	struct extseg_walk_arg *wa = arg;
+
+	if (msl->base_va == wa->va_addr && msl->len == wa->len) {
+		unsigned int found_idx;
+
+		/* msl is const */
+		found_idx = msl - mcfg->memsegs;
+		wa->msl = &mcfg->memsegs[found_idx];
+		return 1;
+	}
+	return 0;
+}
+
+struct rte_memseg_list *
+malloc_heap_find_external_seg(void *va_addr, size_t len)
+{
+	struct extseg_walk_arg wa;
+	int res;
+
+	wa.va_addr = va_addr;
+	wa.len = len;
+
+	res = rte_memseg_list_walk_thread_unsafe(extseg_walk, &wa);
+
+	if (res != 1) {
+		/* 0 means nothing was found, -1 shouldn't happen */
+		if (res == 0)
+			rte_errno = ENOENT;
+		return NULL;
+	}
+	return wa.msl;
+}
+
+int
+malloc_heap_destroy_external_seg(struct rte_memseg_list *msl)
+{
+	/* destroy the fbarray backing this memory */
+	if (rte_fbarray_destroy(&msl->memseg_arr) < 0)
+		return -1;
+
+	/* reset the memseg list */
+	memset(msl, 0, sizeof(*msl));
+
+	return 0;
+}
+
 int
 malloc_heap_add_external_memory(struct malloc_heap *heap,
 		struct rte_memseg_list *msl)
@@ -1206,7 +1252,7 @@ malloc_heap_remove_external_memory(struct malloc_heap *heap, void *va_addr,
 		rte_errno = EBUSY;
 		return -1;
 	}
-	return destroy_seg(elem, len);
+	return destroy_elem(elem, len);
 }
 
 int
diff --git a/lib/librte_eal/common/malloc_heap.h b/lib/librte_eal/common/malloc_heap.h
index 255a315b8..ca9ff666f 100644
--- a/lib/librte_eal/common/malloc_heap.h
+++ b/lib/librte_eal/common/malloc_heap.h
@@ -44,6 +44,12 @@ malloc_heap_create_external_seg(void *va_addr, rte_iova_t iova_addrs[],
 		unsigned int n_pages, size_t page_sz, const char *seg_name,
 		unsigned int socket_id);
 
+struct rte_memseg_list *
+malloc_heap_find_external_seg(void *va_addr, size_t len);
+
+int
+malloc_heap_destroy_external_seg(struct rte_memseg_list *msl);
+
 int
 malloc_heap_add_external_memory(struct malloc_heap *heap,
 		struct rte_memseg_list *msl);
diff --git a/lib/librte_eal/common/rte_malloc.c b/lib/librte_eal/common/rte_malloc.c
index 66bfe63c3..9a82e3386 100644
--- a/lib/librte_eal/common/rte_malloc.c
+++ b/lib/librte_eal/common/rte_malloc.c
@@ -396,6 +396,7 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct malloc_heap *heap = NULL;
+	struct rte_memseg_list *msl;
 	int ret;
 
 	if (heap_name == NULL || va_addr == NULL || len == 0 ||
@@ -420,9 +421,19 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
 		goto unlock;
 	}
 
+	msl = malloc_heap_find_external_seg(va_addr, len);
+	if (msl == NULL) {
+		ret = -1;
+		goto unlock;
+	}
+
 	rte_spinlock_lock(&heap->lock);
 	ret = malloc_heap_remove_external_memory(heap, va_addr, len);
 	rte_spinlock_unlock(&heap->lock);
+	if (ret != 0)
+		goto unlock;
+
+	ret = malloc_heap_destroy_external_seg(msl);
 
 unlock:
 	rte_rwlock_write_unlock(&mcfg->memory_hotplug_lock);
@@ -430,63 +441,12 @@ rte_malloc_heap_memory_remove(const char *heap_name, void *va_addr, size_t len)
 	return ret;
 }
 
-struct sync_mem_walk_arg {
-	void *va_addr;
-	size_t len;
-	int result;
-	bool attach;
-};
-
-static int
-sync_mem_walk(const struct rte_memseg_list *msl, void *arg)
-{
-	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
-	struct sync_mem_walk_arg *wa = arg;
-	size_t len = msl->page_sz * msl->memseg_arr.len;
-
-	if (msl->base_va == wa->va_addr &&
-			len == wa->len) {
-		struct rte_memseg_list *found_msl;
-		int msl_idx, ret;
-
-		/* msl is const */
-		msl_idx = msl - mcfg->memsegs;
-		found_msl = &mcfg->memsegs[msl_idx];
-
-		if (wa->attach) {
-			ret = rte_fbarray_attach(&found_msl->memseg_arr);
-		} else {
-			/* notify all subscribers that a memory area is about to
-			 * be removed
-			 */
-			eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,
-					msl->base_va, msl->len);
-			ret = rte_fbarray_detach(&found_msl->memseg_arr);
-		}
-
-		if (ret < 0) {
-			wa->result = -rte_errno;
-		} else {
-			/* notify all subscribers that a new memory area was
-			 * added
-			 */
-			if (wa->attach)
-				eal_memalloc_mem_event_notify(
-						RTE_MEM_EVENT_ALLOC,
-						msl->base_va, msl->len);
-			wa->result = 0;
-		}
-		return 1;
-	}
-	return 0;
-}
-
 static int
 sync_memory(const char *heap_name, void *va_addr, size_t len, bool attach)
 {
 	struct rte_mem_config *mcfg = rte_eal_get_configuration()->mem_config;
 	struct malloc_heap *heap = NULL;
-	struct sync_mem_walk_arg wa;
+	struct rte_memseg_list *msl;
 	int ret;
 
 	if (heap_name == NULL || va_addr == NULL || len == 0 ||
@@ -513,23 +473,35 @@ sync_memory(const char *heap_name, void *va_addr, size_t len, bool attach)
 	}
 
 	/* find corresponding memseg list to sync to */
-	wa.va_addr = va_addr;
-	wa.len = len;
-	wa.result = -ENOENT; /* fail unless explicitly told to succeed */
-	wa.attach = attach;
-
-	/* we're already holding a read lock */
-	rte_memseg_list_walk_thread_unsafe(sync_mem_walk, &wa);
-
-	if (wa.result < 0) {
-		rte_errno = -wa.result;
+	msl = malloc_heap_find_external_seg(va_addr, len);
+	if (msl == NULL) {
 		ret = -1;
-	} else {
-		/* notify all subscribers that a new memory area was added */
-		if (attach)
+		goto unlock;
+	}
+
+	if (attach) {
+		ret = rte_fbarray_attach(&msl->memseg_arr);
+		if (ret == 0) {
+			/* notify all subscribers that a new memory area was
+			 * added.
+			 */
 			eal_memalloc_mem_event_notify(RTE_MEM_EVENT_ALLOC,
 					va_addr, len);
-		ret = 0;
+		} else {
+			ret = -1;
+			goto unlock;
+		}
+	} else {
+		/* notify all subscribers that a memory area is about to
+		 * be removed.
+		 */
+		eal_memalloc_mem_event_notify(RTE_MEM_EVENT_FREE,
+				msl->base_va, msl->len);
+		ret = rte_fbarray_detach(&msl->memseg_arr);
+		if (ret < 0) {
+			ret = -1;
+			goto unlock;
+		}
 	}
 unlock:
 	rte_rwlock_read_unlock(&mcfg->memory_hotplug_lock);
-- 
2.17.1