Re: [RFC v4 1/6] eal: add static per-lcore memory allocation facility

["Mattias Rönnblom" <hofors@lysator.liu.se>]

On 2024-02-27 10:58, Morten Brørup wrote:
>> From: Mattias Rönnblom [mailto:mattias.ronnblom@ericsson.com]
>> Sent: Sunday, 25 February 2024 16.03
> 
> [...]
> 
>> +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) {
> 
> This would be the usual comparison:
> if (lcore_buffer == NULL) {
> 
>> +		lcore_buffer = aligned_alloc(RTE_CACHE_LINE_SIZE,
>> +					     LCORE_BUFFER_SIZE);
>> +		RTE_VERIFY(lcore_buffer != NULL);
>> +
>> +		offset = 0;
>> +	}
> 
> [...]
> 
>> +/**
>> + * Define a lcore variable handle.
>> + *
>> + * This macro defines a variable which is used as a handle to access
>> + * the various per-lcore id instances of a per-lcore id variable.
>> + *
>> + * The aim with this macro is to make clear at the point of
>> + * declaration that this is an lcore handler, rather than a regular
>> + * pointer.
>> + *
>> + * Add @b static as a prefix in case the lcore variable are only to be
>> + * accessed from a particular translation unit.
>> + */
>> +#define RTE_LCORE_VAR_HANDLE(type, name)	\
>> +	RTE_LCORE_VAR_HANDLE_TYPE(type) name
>> +
> 
> The parameter is "name" here, and "handle" in other macros.
> Just mentioning to make sure you thought about it.
> 
>> +/**
>> + * Get pointer to lcore variable instance with the specified lcore id.
>> + */
>> +#define RTE_LCORE_VAR_LCORE_PTR(lcore_id, handle)			\
>> +	((typeof(handle))__rte_lcore_var_lcore_ptr(lcore_id, handle))
>> +
>> +/**
>> + * Get value of a lcore variable instance of the specified lcore id.
>> + */
>> +#define RTE_LCORE_VAR_LCORE_GET(lcore_id, handle)	\
>> +	(*(RTE_LCORE_VAR_LCORE_PTR(lcore_id, handle)))
>> +
>> +/**
>> + * Set the value of a lcore variable instance of the specified lcore id.
>> + */
>> +#define RTE_LCORE_VAR_LCORE_SET(lcore_id, handle, value)		\
>> +	(*(RTE_LCORE_VAR_LCORE_PTR(lcore_id, handle)) = (value))
> 
> I still think RTE_LCORE_VAR[_LCORE]_PTR() suffice, and RTE_LCORE_VAR[_LCORE]_GET/SET are superfluous.
> But I don't insist on their removal. :-)
> 

I'll remove them. One can always add them later. Nothing I've seen in 
the DPDK code base so far has been called for their use.

Should the RTE_LCORE_VAR_PTR() be renamed RTE_LCORE_VAR_VALUE() (and 
still return a pointer, obviously)? "PTR" seems a little superfluous 
(Hungarian). "RTE_LCORE_VAR()" would be short, but not very descriptive.

> With or without suggested changes...
> 
> For the series,
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 

Thanks for all help.