Re: Add palloc_aligned() to allow arbitrary power of 2 memory alignment

Hi,

On 2022-11-02 00:28:46 +1300, David Rowley wrote:
> Part of the work that Thomas mentions in [1], regarding Direct I/O,
> diff --git a/src/backend/utils/mmgr/alignedalloc.c b/src/backend/utils/mmgr/alignedalloc.c
> new file mode 100644
> index 0000000000..e581772758
> --- /dev/null
> +++ b/src/backend/utils/mmgr/alignedalloc.c
> @@ -0,0 +1,93 @@
> +/*-------------------------------------------------------------------------
> + *
> + * alignedalloc.c
> + *      Allocator functions to implement palloc_aligned

FWIW, to me this is really more part of mcxt.c than its own
allocator... Particularly because MemoryContextAllocAligned() et al are
implemented there.


> +void *
> +AlignedAllocRealloc(void *pointer, Size size)

I doubtthere's ever a need to realloc such a pointer? Perhaps we could just
elog(ERROR)?


> +/*
> + * MemoryContextAllocAligned
> + *        Allocate 'size' bytes of memory in 'context' aligned to 'alignto'
> + *        bytes.
> + *
> + * 'flags' may be 0 or set the same as MemoryContextAllocExtended().
> + * 'alignto' must be a power of 2.
> + */
> +void *
> +MemoryContextAllocAligned(MemoryContext context,
> +                          Size size, Size alignto, int flags)
> +{
> +    Size        alloc_size;
> +    void       *unaligned;
> +    void       *aligned;
> +
> +    /* wouldn't make much sense to waste that much space */
> +    Assert(alignto < (128 * 1024 * 1024));
> +
> +    /* ensure alignto is a power of 2 */
> +    Assert((alignto & (alignto - 1)) == 0);

Hm, not that I can see a case for ever not using a power of two
alignment... There's not really a "need" for the restriction, right? Perhaps
we should note that?


> +    /*
> +     * We implement aligned pointers by simply allocating enough memory for
> +     * the requested size plus the alignment and an additional MemoryChunk.
> +     * This additional MemoryChunk is required for operations such as pfree
> +     * when used on the pointer returned by this function.  We use this
> +     * "redirection" MemoryChunk in order to find the pointer to the memory
> +     * that was returned by the MemoryContextAllocExtended call below. We do
> +     * that by "borrowing" the block offset field and instead of using that to
> +     * find the offset into the owning block, we use it to find the original
> +     * allocated address.
> +     *
> +     * Here we must allocate enough extra memory so that we can still align
> +     * the pointer returned by MemoryContextAllocExtended and also have enough
> +     * space for the redirection MemoryChunk.
> +     */
> +    alloc_size = size + alignto + sizeof(MemoryChunk);
> +
> +    /* perform the actual allocation */
> +    unaligned = MemoryContextAllocExtended(context, alloc_size, flags);

Should we handle the case where we get a suitably aligned pointer from
MemoryContextAllocExtended() differently?


> +    /* XXX: should we adjust valgrind state here? */

Probably still need to do this... Kinda hard to get right without the code
getting exercised. Wonder if there's some minimal case we could actually use
it for?

Thanks,

Andres