> From: Mattias Rönnblom [mailto:hof...@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.
> >
