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

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

> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
> Sent: Monday, 14 October 2024 09.44


> +struct lcore_var_buffer {
> +	char data[RTE_MAX_LCORE_VAR * RTE_MAX_LCORE];
> +	struct lcore_var_buffer *prev;
> +};

In relation to Jerin's request for using hugepages when available, the "data" field should be a pointer to the memory allocated from either the heap or through rte_malloc. You would also need to add a flag to indicate which it is, so the correct deallocation function can be used to free it on cleanup.

<feature creep>
Here's another (nice to have) idea, which does not need to be part of this series, but can be implemented in a separate patch:
If you move "offset" into this structure, new lcore variables can be allocated from any buffer, instead of only the most recently allocated buffer.
There might even be gains by picking the "optimal" buffer to allocate different size variables from.
</feature creep>

> +
> +static struct lcore_var_buffer *current_buffer;
> +
> +/* initialized to trigger buffer allocation on first allocation */
> +static size_t offset = RTE_MAX_LCORE_VAR;


> +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 is very slow path, please RTE_VERIFY instead of RTE_ASSERT in this function.


> +/**
> + * Get pointer to lcore variable instance with the specified lcore id.
> + *
> + * @param lcore_id
> + *   The lcore id specifying which of the @c RTE_MAX_LCORE value
> + *   instances should be accessed. The lcore id need not be valid
> + *   (e.g., may be @ref LCORE_ID_ANY), but in such a case, the pointer
> + *   is also not valid (and thus should not be dereferenced).
> + * @param handle
> + *   The lcore variable handle.
> + */
> +#define RTE_LCORE_VAR_LCORE_VALUE(lcore_id, handle)			\
> +	((typeof(handle))rte_lcore_var_lcore_ptr(lcore_id, handle))

Please remove the _VALUE suffix.

> +
> +/**
> + * Get pointer to lcore variable instance of the current thread.
> + *
> + * May only be used by EAL threads and registered non-EAL threads.
> + */
> +#define RTE_LCORE_VAR_VALUE(handle) \
> +	RTE_LCORE_VAR_LCORE_VALUE(rte_lcore_id(), handle)

Please remove the _VALUE suffix.

> +
> +/**
> + * Iterate over each lcore id's value for an lcore variable.
> + *
> + * @param lcore_id
> + *   An <code>unsigned int</code> variable successively set to the
> + *   lcore id of every valid lcore id (up to @c RTE_MAX_LCORE).
> + * @param value
> + *   A pointer variable successively set to point to lcore variable
> + *   value instance of the current lcore id being processed.
> + * @param handle
> + *   The lcore variable handle.
> + */
> +#define RTE_LCORE_VAR_FOREACH_VALUE(lcore_id, value, handle)

Please remove the _VALUE suffix.

> 	\
> +	for ((lcore_id) =						\
> +		     (((value) = RTE_LCORE_VAR_LCORE_VALUE(0, handle)), 0);
> \
> +	     (lcore_id) < RTE_MAX_LCORE;				\
> +	     (lcore_id)++, (value) = RTE_LCORE_VAR_LCORE_VALUE(lcore_id,
> \