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

[Morten Brørup]

> From: Mattias Rönnblom [mailto:hofors at lysator.liu.se]
> Sent: Tuesday, 15 October 2024 21.03
> 
> On 2024-10-15 12:13, Morten Brørup wrote:
> >> +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_VERIFY(align <= RTE_CACHE_LINE_SIZE);
> >> +	RTE_VERIFY(size <= RTE_MAX_LCORE_VAR);
> >> +
> >> +	/* '0' means asking for worst-case alignment requirements */
> >> +	if (align == 0)
> >> +#ifdef RTE_TOOLCHAIN_MSVC
> >> +		/* MSVC <stddef.h> is missing the max_align_t typedef */
> >> +		align = alignof(double);
> >> +#else
> >> +		align = alignof(max_align_t);
> >> +#endif
> >
> > Do we need worst-case alignment, or does automatic alignment suffice:
> >
> 
> I think the term is "natural alignment." As I think I mentioned at some
> point, I don't really have an opinion.

Exactly; "natural alignment" was the term I was looking for.

> 
> Worst case (max_alignt_t) alignment is the same as malloc(), so
> potentially what the user may expect.

For this type of variables, which are more like "static" variables, I don't think the user expects malloc()-like alignment; I think the user expects natural alignment.
And if the user requires any special alignment, the user will specify it explicitly.

> On the other hand, I can't see why
> natural alignment (or alignof(max_align_t), whichever is smallest)
> would
> not always suffice. 

Yes, that was exactly my point.

> It is a bit harder to explain in the API docs what
> alignment you actually get in case you don't go for worst-case
> alignment.

Yeah... using "natural alignment" instead of "worst-case alignment" doesn't really cut it; e.g. if the lcore variable is a struct of two uint16_t, the natural alignment is 2 byte, but it will be 4 byte aligned due to the size.
Maybe "automatic alignment" could be used here... with an explanation that it is the minimum of the size, rounded up to a power of two, or max_align_t.
Anyway, in case of doubt, the developer can look at the implementation - it's one of the benefits of having the source code available. :-)

> 
> I think it doesn't matter much, because the user will very likely use
> the typed macros (and get whatever alignment the compiler deems
> appropriate for that type).

Probably.
But the function allowing alignment=0 should still behave 1) as expected by its users, and 2) optimally.

I hope this library is going to be a widely used core component in DPDK, and getting all the small details right will improve the probability of success.

> 
> > 	/* '0' means asking for automatic alignment requirements */
> > 	if (align == 0) {
> > #ifdef RTE_ARCH_64
> > 		align = rte_align64pow2(size);
> > #else
> > 		align = rte_align32pow2(size);
> > #endif
> > #ifdef RTE_TOOLCHAIN_MSVC
> > 		/* MSVC <stddef.h> is missing the max_align_t typedef */
> > 		align = RTE_MIN(align, alignof(double));
> > #else
> > 		align = RTE_MIN(align, alignof(max_align_t));
> > #endif
> > 	}
> >
> > It will pack small-size lcore variables even tighter.
> >