Re: [dpdk-dev] [PATCH v4 8/8] eal/windows: implement basic memory management

[Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>]

On Wed, 13 May 2020 12:25:10 +0000
Fady Bader <fady@mellanox.com> wrote:
[snip]
> > 
> > I meant the code of the application that calls
> > rte_mempool_create(). Or is it one of the DPDK test applications?  
> 
> I got it from dpdk\app\test\test_mempool
> 
> Line 496:
> /* create a mempool (without cache) */
> mp_nocache = rte_mempool_create("test_nocache", MEMPOOL_SIZE,
> 	MEMPOOL_ELT_SIZE, 0, 0,
> 	NULL, NULL,
> 	my_obj_init, NULL,
> 	SOCKET_ID_ANY, 0);
>

For building this code you must have enabled librte_ring,
librte_mempool, and drivers/mempool, and I assume you build test code
without librte_cmdline somehow. This are nontrivial changes, so I can't
be sure to reproduce them exactly. Can you please share a complete
patch?

Meanwhile, I observe a similar issue where rte_mempool_create() fails
to allocate memory and hangs when compiled with Clang, succeeds with
native MinGW, but still hangs with cross MinGW. I'm investigating it.

Testing patch follows, the snippet added is in
examples/helloworld/main.

---

diff --git a/config/meson.build b/config/meson.build
index b6d84687f..018726f75 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -117,7 +117,7 @@ if not is_windows
 endif
 
 # use pthreads if available for the platform
-if not is_ms_linker
+if cc.find_library('pthread', required: false).found()
 	add_project_link_arguments('-pthread', language: 'c')
 	dpdk_extra_ldflags += '-pthread'
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index dc293b270..ee565bc19 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -2,8 +2,8 @@
 # Copyright(c) 2017-2019 Intel Corporation
 
 if is_windows
-	subdir_done()
-endif
+	dpdk_driver_classes = ['mempool']
+else
 
 # Defines the order in which the drivers are buit.
 dpdk_driver_classes = ['common',
@@ -16,7 +16,7 @@ dpdk_driver_classes = ['common',
 	       'vdpa',    # depends on common, bus and mempool.
 	       'event',   # depends on common, bus, mempool and net.
 	       'baseband'] # depends on common and bus.
-
+endif
 disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
 		).stdout().split()
 
@@ -78,13 +78,13 @@ foreach class:dpdk_driver_classes
 			shared_deps = ext_deps
 			static_deps = ext_deps
 			foreach d:deps
-				if not is_variable('shared_rte_' + d)
+				if not is_variable('static_rte_' + d)
 					build = false
 					reason = 'Missing internal dependency, "@0@"'.format(d)
 					message('Disabling @1@ [@2@]: missing internal dependency "@0@"'
 							.format(d, name, 'drivers/' + drv_path))
 				else
-					shared_deps += [get_variable('shared_rte_' + d)]
+					# shared_deps += [get_variable('shared_rte_' + d)]
 					static_deps += [get_variable('static_rte_' + d)]
 				endif
 			endforeach
@@ -110,6 +110,7 @@ foreach class:dpdk_driver_classes
 
 			dpdk_extra_ldflags += pkgconfig_extra_libs
 
+			if host_machine.system() != 'windows'
 			# generate pmdinfo sources by building a temporary
 			# lib and then running pmdinfogen on the contents of
 			# that lib. The final lib reuses the object files and
@@ -126,7 +127,7 @@ foreach class:dpdk_driver_classes
 						'@OUTPUT@', pmdinfogen],
 					output: out_filename,
 					depends: [pmdinfogen, tmp_lib])
-
+			endif
 			version_map = '@0@/@1@/@2@_version.map'.format(
 					meson.current_source_dir(),
 					drv_path, lib_name)
@@ -178,31 +179,31 @@ foreach class:dpdk_driver_classes
 					output: lib_name + '.sym_chk')
 			endif
 
-			shared_lib = shared_library(lib_name,
-				sources,
-				objects: objs,
-				include_directories: includes,
-				dependencies: shared_deps,
-				c_args: cflags,
-				link_args: lk_args,
-				link_depends: lk_deps,
-				version: lib_version,
-				soversion: so_version,
-				install: true,
-				install_dir: driver_install_path)
-
-			# create a dependency object and add it to the global dictionary so
-			# testpmd or other built-in apps can find it if necessary
-			shared_dep = declare_dependency(link_with: shared_lib,
-					include_directories: includes,
-					dependencies: shared_deps)
+			# shared_lib = shared_library(lib_name,
+			# 	sources,
+			# 	objects: objs,
+			# 	include_directories: includes,
+			# 	dependencies: shared_deps,
+			# 	c_args: cflags,
+			# 	link_args: lk_args,
+			# 	link_depends: lk_deps,
+			# 	version: lib_version,
+			# 	soversion: so_version,
+			# 	install: true,
+			# 	install_dir: driver_install_path)
+
+			# # create a dependency object and add it to the global dictionary so
+			# # testpmd or other built-in apps can find it if necessary
+			# shared_dep = declare_dependency(link_with: shared_lib,
+			# 		include_directories: includes,
+			# 		dependencies: shared_deps)
 			static_dep = declare_dependency(link_with: static_lib,
 					include_directories: includes,
 					dependencies: static_deps)
 
 			dpdk_drivers += static_lib
 
-			set_variable('shared_@0@'.format(lib_name), shared_dep)
+			# set_variable('shared_@0@'.format(lib_name), shared_dep)
 			set_variable('static_@0@'.format(lib_name), static_dep)
 			dependency_name = ''.join(lib_name.split('rte_'))
 			message('drivers/@0@: Defining dependency "@1@"'.format(
diff --git a/examples/helloworld/main.c b/examples/helloworld/main.c
index 968045f1b..cf895c840 100644
--- a/examples/helloworld/main.c
+++ b/examples/helloworld/main.c
@@ -14,6 +14,8 @@
 #include <rte_per_lcore.h>
 #include <rte_lcore.h>
 #include <rte_debug.h>
+#include <rte_mempool.h>
+#include <rte_errno.h>
 
 static int
 lcore_hello(__rte_unused void *arg)
@@ -29,11 +31,30 @@ main(int argc, char **argv)
 {
 	int ret;
 	unsigned lcore_id;
+	struct rte_mempool *pool;
+
+	rte_log_set_level(RTE_LOGTYPE_EAL, RTE_LOG_DEBUG);
 
 	ret = rte_eal_init(argc, argv);
 	if (ret < 0)
 		rte_panic("Cannot init EAL\n");
 
+	pool = rte_mempool_create(
+		"test_mempool",
+		(1 << 18) - 1,
+		(1 << 12),
+		1 << 9,
+		0,
+		NULL, NULL, NULL, NULL,
+		SOCKET_ID_ANY,
+		0);
+	if (!pool) {
+		RTE_LOG(ERR, USER1, "cannot create mempool: %d\n", rte_errno);
+		return EXIT_FAILURE;
+	}
+
+	rte_mempool_free(pool);
+
 	/* call lcore_hello() on every slave lcore */
 	RTE_LCORE_FOREACH_SLAVE(lcore_id) {
 		rte_eal_remote_launch(lcore_hello, NULL, lcore_id);
diff --git a/examples/meson.build b/examples/meson.build
index 3b540012f..407322dec 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -83,7 +83,7 @@ foreach example: examples
 	includes = [include_directories(example)]
 	deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline']
 	if is_windows
-		deps = ['eal'] # only supported lib on Windows currently
+		deps = ['eal', 'mempool', 'ring'] # only supported lib on Windows currently
 	endif
 	subdir(example)
 
diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c
index 0bde995b5..017eebba5 100644
--- a/lib/librte_mempool/rte_mempool.c
+++ b/lib/librte_mempool/rte_mempool.c
@@ -12,7 +12,6 @@
 #include <inttypes.h>
 #include <errno.h>
 #include <sys/queue.h>
-#include <sys/mman.h>
 
 #include <rte_common.h>
 #include <rte_log.h>
@@ -148,7 +147,7 @@ get_min_page_size(int socket_id)
 
 	rte_memseg_list_walk(find_min_pagesz, &wa);
 
-	return wa.min == SIZE_MAX ? (size_t) getpagesize() : wa.min;
+	return wa.min == SIZE_MAX ? (size_t) rte_get_page_size() : wa.min;
 }
 
 
@@ -526,7 +525,7 @@ rte_mempool_get_page_size(struct rte_mempool *mp, size_t *pg_sz)
 	else if (rte_eal_has_hugepages() || alloc_in_ext_mem)
 		*pg_sz = get_min_page_size(mp->socket_id);
 	else
-		*pg_sz = getpagesize();
+		*pg_sz = rte_get_page_size();
 
 	rte_mempool_trace_get_page_size(mp, *pg_sz);
 	return 0;
@@ -686,7 +685,7 @@ get_anon_size(const struct rte_mempool *mp)
 	size_t min_chunk_size;
 	size_t align;
 
-	pg_sz = getpagesize();
+	pg_sz = rte_get_page_size();
 	pg_shift = rte_bsf32(pg_sz);
 	size = rte_mempool_ops_calc_mem_size(mp, mp->size, pg_shift,
 					     &min_chunk_size, &align);
@@ -710,7 +709,7 @@ rte_mempool_memchunk_anon_free(struct rte_mempool_memhdr *memhdr,
 	if (size < 0)
 		return;
 
-	munmap(opaque, size);
+	rte_mem_unmap(opaque, size);
 }
 
 /* populate the mempool with an anonymous mapping */
@@ -740,20 +739,20 @@ rte_mempool_populate_anon(struct rte_mempool *mp)
 	}
 
 	/* get chunk of virtually continuous memory */
-	addr = mmap(NULL, size, PROT_READ | PROT_WRITE,
-		MAP_SHARED | MAP_ANONYMOUS, -1, 0);
-	if (addr == MAP_FAILED) {
-		rte_errno = errno;
+	addr = rte_mem_map(NULL, size, RTE_PROT_READ | RTE_PROT_WRITE,
+		RTE_MAP_SHARED | RTE_MAP_ANONYMOUS, -1, 0);
+	if (addr == NULL) {
 		return 0;
 	}
 	/* can't use MMAP_LOCKED, it does not exist on BSD */
-	if (mlock(addr, size) < 0) {
-		rte_errno = errno;
-		munmap(addr, size);
+	if (rte_mem_lock(addr, size) < 0) {
+		ret = rte_errno;
+		rte_mem_unmap(addr, size);
+		rte_errno = ret;
 		return 0;
 	}
 
-	ret = rte_mempool_populate_virt(mp, addr, size, getpagesize(),
+	ret = rte_mempool_populate_virt(mp, addr, size, rte_get_page_size(),
 		rte_mempool_memchunk_anon_free, addr);
 	if (ret == 0) /* should not happen */
 		ret = -ENOBUFS;
diff --git a/lib/meson.build b/lib/meson.build
index d190d84ef..77da5216f 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -36,7 +36,12 @@ libraries = [
 	'flow_classify', 'bpf', 'graph', 'node']
 
 if is_windows
-	libraries = ['kvargs','eal'] # only supported libraries for windows
+	libraries = [
+		'kvargs',
+		'eal',
+		'ring',
+		'mempool',
+		]
 endif
 
 default_cflags = machine_args
@@ -56,6 +61,7 @@ foreach l:libraries
 	use_function_versioning = false
 	sources = []
 	headers = []
+	headers_compat = {}
 	includes = []
 	cflags = default_cflags
 	objs = [] # other object files to link against, used e.g. for
@@ -77,11 +83,11 @@ foreach l:libraries
 		shared_deps = ext_deps
 		static_deps = ext_deps
 		foreach d:deps
-			if not is_variable('shared_rte_' + d)
+			if not is_variable('static_rte_' + d)
 				error('Missing internal dependency "@0@" for @1@ [@2@]'
 						.format(d, name, 'lib/' + dir_name))
 			endif
-			shared_deps += [get_variable('shared_rte_' + d)]
+			# shared_deps += [get_variable('shared_rte_' + d)]
 			static_deps += [get_variable('static_rte_' + d)]
 		endforeach
 	endif
@@ -94,6 +100,12 @@ foreach l:libraries
 		dpdk_conf.set('RTE_LIBRTE_' + name.to_upper(), 1)
 		install_headers(headers)
 
+		if is_windows and (name == 'eal')
+			foreach dir, header : headers_compat
+				install_headers(header, subdir: dir)
+			endforeach
+		endif
+
 		libname = 'rte_' + name
 		includes += include_directories(dir_name)
 
@@ -171,29 +183,29 @@ foreach l:libraries
 					output: name + '.sym_chk')
 			endif
 
-			shared_lib = shared_library(libname,
-					sources,
-					objects: objs,
-					c_args: cflags,
-					dependencies: shared_deps,
-					include_directories: includes,
-					link_args: lk_args,
-					link_depends: lk_deps,
-					version: lib_version,
-					soversion: so_version,
-					install: true)
-			shared_dep = declare_dependency(link_with: shared_lib,
-					include_directories: includes,
-					dependencies: shared_deps)
-
-			dpdk_libraries = [shared_lib] + dpdk_libraries
+			# shared_lib = shared_library(libname,
+			# 		sources,
+			# 		objects: objs,
+			# 		c_args: cflags,
+			# 		dependencies: shared_deps,
+			# 		include_directories: includes,
+			# 		link_args: lk_args,
+			# 		link_depends: lk_deps,
+			# 		version: lib_version,
+			# 		soversion: so_version,
+			# 		install: true)
+			# shared_dep = declare_dependency(link_with: shared_lib,
+			# 		include_directories: includes,
+			# 		dependencies: shared_deps)
+
+			# dpdk_libraries = [shared_lib] + dpdk_libraries
 			dpdk_static_libraries = [static_lib] + dpdk_static_libraries
 			if libname == 'rte_node'
 				dpdk_graph_nodes = [static_lib]
 			endif
 		endif # sources.length() > 0
 
-		set_variable('shared_rte_' + name, shared_dep)
+		# set_variable('shared_rte_' + name, shared_dep)
 		set_variable('static_rte_' + name, static_dep)
 		message('lib/@0@: Defining dependency "@1@"'.format(
 				dir_name, name))


--
Dmitry Kozlyuk