env: Use rte_malloc in spdk_mem_register code path when possible

For TCMalloc regions which we register with spdk at runtime in the MMapHook, we
need to ensure that SPDK doesn't do any allocations in that path otherwise we
will hit a livelock situation. MmapHook is invoked when TCMalloc is out of free
memory and needs to get more memory from the system, for the hugepage case it
gets via mmap.

In the current code, we could end up calling malloc in the spdk_mem_register
call via the following call path.

spdk_mem_register -> spdk_mem_map_set_translation -> spdk_mem_map_get_map_1gb

To avoid this livelock situation we call rte_malloc instead which shouldn't
invoke the system allocator. Note that in try_expand_heap_primary() which is
invoked in the rte_malloc code path, we can still call malloc, so we need to
only use this when dynamic memory allocation is disabled via --legacy-mem.
It is possible in the future we could work around even this limitation,
but for now this implementation will be much simpler.

Have verified this change fixes the livelock condition which I was hitting in
my setup without this fix.

Change-Id: I69d0813a70da1f26f8c4d9d8895e406c026be18b
Signed-off-by: Alok N Kataria <alok.kataria@nutanix.com>
Signed-off-by: Jim Harris <james.r.harris@intel.com>
Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/475943
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Community-CI: SPDK CI Jenkins <sys_sgci@intel.com>

lib/env_dpdk/memory.c

@@ -36,6 +36,7 @@
 #include "env_internal.h"
 
 #include <rte_config.h>
+#include <rte_malloc.h>
 #include <rte_memory.h>
 #include <rte_eal_memconfig.h>
 
@@ -342,7 +343,11 @@ spdk_mem_map_free(struct spdk_mem_map **pmap)
 	}
 
 	for (i = 0; i < sizeof(map->map_256tb.map) / sizeof(map->map_256tb.map[0]); i++) {
-		free(map->map_256tb.map[i]);
+		if (g_legacy_mem) {
+			rte_free(map->map_256tb.map[i]);
+		} else {
+			free(map->map_256tb.map[i]);
+		}
 	}
 
 	pthread_mutex_destroy(&map->mutex);
@@ -522,7 +527,23 @@ spdk_mem_map_get_map_1gb(struct spdk_mem_map *map, uint64_t vfn_2mb)
 		/* Recheck to make sure nobody else got the mutex first. */
 		map_1gb = map->map_256tb.map[idx_256tb];
 		if (!map_1gb) {
-			map_1gb = malloc(sizeof(struct map_1gb));
+			/* Some of the existing apps use TCMalloc hugepage
+			 * allocator and register this tcmalloc allocated
+			 * hugepage memory with SPDK in the mmap hook. Since
+			 * this function is called in the spdk_mem_register
+			 * code path we can't do a malloc here otherwise that
+			 * would cause a livelock. So we use the dpdk provided
+			 * allocator instead, which avoids this cyclic
+			 * dependency.  Note this is only guaranteed to work when
+			 * DPDK dynamic memory allocation is disabled (--legacy-mem),
+			 * which then is a requirement for anyone using TCMalloc in
+			 * this way.
+			 */
+			if (g_legacy_mem) {
+				map_1gb = rte_malloc(NULL, sizeof(struct map_1gb), 0);
+			} else {
+				map_1gb = malloc(sizeof(struct map_1gb));
+			}
 			if (map_1gb) {
 				/* initialize all entries to default translation */
 				for (i = 0; i < SPDK_COUNTOF(map_1gb->map); i++) {

test/env/memory/memory_ut.c

@@ -52,6 +52,20 @@ DEFINE_STUB(rte_mem_event_callback_register, int,
 	    (const char *name, rte_mem_event_callback_t clb, void *arg), 0);
 DEFINE_STUB(rte_mem_virt2iova, rte_iova_t, (const void *virtaddr), 0);
 
+void *
+rte_malloc(const char *type, size_t size, unsigned align)
+{
+	CU_ASSERT(type == NULL);
+	CU_ASSERT(align == 0);
+	return malloc(size);
+}
+
+void
+rte_free(void *ptr)
+{
+	free(ptr);
+}
+
 static int
 test_mem_map_notify(void *cb_ctx, struct spdk_mem_map *map,
 		    enum spdk_mem_map_notify_action action,