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

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

On 2020-05-05 17:24 GMT+0100 Burakov, Anatoly wrote:
> On 29-Apr-20 12:50 AM, Dmitry Kozlyuk wrote:
> 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...

This was the main question of the cover letter :)
Dmitry Malloy explained to me recently that even internally Windows has
no notion of preallocated hugepages and "memory types" (as memseg_primary_init
describes it). Since Windows EAL is not going to support multi-process any
time soon (if ever), maybe these reservations are not needed and memory manger
should create MSLs and enforce socket limits dynamically? This way most of the
duplicated code can be removed, I think. Or does MSL reservation serve some
other purposes?

> > 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.

I can add a patch that makes each tracepoint a no-op on Windows.

We discussed this issue (spreading workarounds) 2020-04-30 on Windows
community call. The proper solution would be supporting trace on Windows, but
IIRC no one is yet directly assigned to do that.

[snip] 
> > +	/* 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.

OK, thanks.

-- 
Dmitry Kozlyuk