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

[Burakov, Anatoly]

On 29-Apr-20 12:50 AM, Dmitry Kozlyuk wrote:
> Basic memory management supports core libraries and PMDs operating in
> IOVA as PA mode. It uses a kernel-mode driver, virt2phys, to obtain
> IOVAs of hugepages allocated from user-mode. Multi-process mode is not
> implemented and is forcefully disabled at startup.
> 
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozliuk at gmail.com>
> ---

Lots of duplication... I wonder if it would be possible to share at 
least some of this code in common. Tracking down bugs because of 
duplicated code desync is always a pain...

<snip>

> diff --git a/lib/librte_eal/common/eal_common_memzone.c b/lib/librte_eal/common/eal_common_memzone.c
> index 7c21aa921..9fa7bf352 100644
> --- a/lib/librte_eal/common/eal_common_memzone.c
> +++ b/lib/librte_eal/common/eal_common_memzone.c
> @@ -19,7 +19,14 @@
>   #include <rte_errno.h>
>   #include <rte_string_fns.h>
>   #include <rte_common.h>
> +
> +#ifndef RTE_EXEC_ENV_WINDOWS
>   #include <rte_eal_trace.h>
> +#else
> +#define rte_eal_trace_memzone_reserve(...)
> +#define rte_eal_trace_memzone_lookup(...)
> +#define rte_eal_trace_memzone_free(...)
> +#endif
>   

Is it possible for rte_eal_trace.h to implement this workaround instead? 
It wouldn't be very wise to have to have this in each file that depends 
on rte_eal_trace.h.

>   #include "malloc_heap.h"
>   #include "malloc_elem.h"
> diff --git a/lib/librte_eal/common/meson.build b/lib/librte_eal/common/meson.build
> index 155da29b4..9bb234009 100644
> --- a/lib/librte_eal/common/meson.build
> +++ b/lib/librte_eal/common/meson.build
> @@ -9,11 +9,21 @@ if is_windows
>   		'eal_common_class.c',
>   		'eal_common_devargs.c',
>   		'eal_common_errno.c',

<snip>

>    /* Launch threads, called at application init(). */
>   int
>   rte_eal_init(int argc, char **argv)
> @@ -245,6 +346,13 @@ rte_eal_init(int argc, char **argv)
>   	if (fctret < 0)
>   		exit(1);
>   
> +	/* Prevent creation of shared memory files. */
> +	if (internal_config.no_shconf == 0) {
> +		RTE_LOG(WARNING, EAL, "Multi-process support is requested, "
> +			"but not available.\n");
> +		internal_config.no_shconf = 1;

In the future i would like to deprecate no_shconf because it's a strict 
subset of in_memory mode and serves the same purpose. Might i suggest 
using in_memory flag instead? IIRC no_shconf is automatically set when 
you set in_memory mode.

-- 
Thanks,
Anatoly