RE: [PATCH v2 1/6] eal: add static per-lcore memory allocation facility

["Morten Brørup" <mb@smartsharesystems.com>]

> +#define LCORE_BUFFER_SIZE (RTE_MAX_LCORE_VAR * RTE_MAX_LCORE)

Considering hugepages...

Lcore variables may be allocated before DPDK's memory allocator (rte_malloc()) is ready, so rte_malloc() cannot be used for lcore variables.

And lcore variables are not usable (shared) for DPDK multi-process, so the lcore_buffer could be allocated through the O/S APIs as anonymous hugepages, instead of using rte_malloc().

The alternative, using rte_malloc(), would disallow allocating lcore variables before DPDK's memory allocator has been initialized, which I think is too late.

Anyway, hugepages is not a "must have" here, it is a "nice to have". It can be added to the lcore variables subsystem at a later time.


Here are some thoughts about optimizing for TLB entry usage...

If lcore variables use hugepages, and LCORE_BUFFER_SIZE matches the hugepage size (2 MB), all the lcore variables will only consume 1 hugepage TLB entry.
However, this may limit the max size of an lcore variable (RTE_MAX_LCORE_VAR) too much, if the system supports many lcores (RTE_MAX_LCORE).
E.g. with 1024 lcores, the max size of an lcore variable would be 2048 bytes.
And with 128 lcores, the max size of an lcore variable would be 16 KB.

So if we want to optimize for hugepage TLB entry usage, the question becomes: What is a reasonable max size of an lcore variable?

And although hugepages is only a "nice to have", the max size of an lcore variable (RTE_MAX_LCORE_VAR) is part of the API/ABI, so we should consider it now, if we want to optimize for hugepage TLB entry usage in the future.


A few more comments below, not related to hugepages.

> +
> +static void *lcore_buffer;
> +static size_t offset = RTE_MAX_LCORE_VAR;
> +
> +static void *
> +lcore_var_alloc(size_t size, size_t align)
> +{
> +	void *handle;
> +	void *value;
> +
> +	offset = RTE_ALIGN_CEIL(offset, align);
> +
> +	if (offset + size > RTE_MAX_LCORE_VAR) {
> +#ifdef RTE_EXEC_ENV_WINDOWS
> +		lcore_buffer = _aligned_malloc(LCORE_BUFFER_SIZE,
> +					       RTE_CACHE_LINE_SIZE);
> +#else
> +		lcore_buffer = aligned_alloc(RTE_CACHE_LINE_SIZE,
> +					     LCORE_BUFFER_SIZE);
> +#endif
> +		RTE_VERIFY(lcore_buffer != NULL);
> +
> +		offset = 0;
> +	}
> +
> +	handle = RTE_PTR_ADD(lcore_buffer, offset);
> +
> +	offset += size;
> +
> +	RTE_LCORE_VAR_FOREACH_VALUE(value, handle)
> +		memset(value, 0, size);
> +
> +	EAL_LOG(DEBUG, "Allocated %"PRIuPTR" bytes of per-lcore data with
> a "
> +		"%"PRIuPTR"-byte alignment", size, align);
> +
> +	return handle;
> +}
> +
> +void *
> +rte_lcore_var_alloc(size_t size, size_t align)
> +{
> +	/* Having the per-lcore buffer size aligned on cache lines
> +	 * assures as well as having the base pointer aligned on cache
> +	 * size assures that aligned offsets also translate to alipgned
> +	 * pointers across all values.
> +	 */
> +	RTE_BUILD_BUG_ON(RTE_MAX_LCORE_VAR % RTE_CACHE_LINE_SIZE != 0);
> +	RTE_ASSERT(align <= RTE_CACHE_LINE_SIZE);
> +	RTE_ASSERT(size <= RTE_MAX_LCORE_VAR);

This specific RTE_ASSERT() should be upgraded to RTE_VERIFY(), so it is checked in non-debug builds too.
The code is slow path and not inline, and if this check doesn't pass, accessing the lcore variable will cause a buffer overrun. Prefer failing early.

> +
> +	/* '0' means asking for worst-case alignment requirements */
> +	if (align == 0)
> +		align = alignof(max_align_t);
> +
> +	RTE_ASSERT(rte_is_power_of_2(align));
> +
> +	return lcore_var_alloc(size, align);
> +}


> +/**
> + * Allocate space in the per-lcore id buffers for an lcore variable.
> + *
> + * The pointer returned is only an opaque identifer of the variable. To
> + * get an actual pointer to a particular instance of the variable use
> + * @ref RTE_LCORE_VAR_VALUE or @ref RTE_LCORE_VAR_LCORE_VALUE.
> + *
> + * The lcore variable values' memory is set to zero.
> + *
> + * The allocation is always successful, barring a fatal exhaustion of
> + * the per-lcore id buffer space.
> + *
> + * rte_lcore_var_alloc() is not multi-thread safe.
> + *
> + * @param size
> + *   The size (in bytes) of the variable's per-lcore id value. Must be
> > 0.
> + * @param align
> + *   If 0, the values will be suitably aligned for any kind of type
> + *   (i.e., alignof(max_align_t)). Otherwise, the values will be
> aligned
> + *   on a multiple of *align*, which must be a power of 2 and equal or
> + *   less than @c RTE_CACHE_LINE_SIZE.
> + * @return
> + *   The variable's handle, stored in a void pointer value. The value
> + *   is always non-NULL.
> + */
> +__rte_experimental

I don't know how useful these are, but consider adding:
#ifndef RTE_TOOLCHAIN_MSVC
__attribute__((malloc))
__attribute__((alloc_size(1)))
__attribute__((alloc_align(2)))
__attribute__((returns_nonnull))
#endif

> +void *
> +rte_lcore_var_alloc(size_t size, size_t align);