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