Commit f65d21b2 authored by Tom Lane's avatar Tom Lane

Mostly-cosmetic improvements in memory chunk header alignment coding.

Add commentary about what we're doing and why.  Apply the method used for
padding in GenerationChunk to AllocChunkData, replacing the rather ad-hoc
solution used in commit 7e3aa03b.  Reorder fields in GenerationChunk so
that the padding calculation will work even if sizeof(size_t) is different
from sizeof(void *) --- likely that will never happen, but we don't need
the assumption if we do it like this.  Improve static assertions about
alignment.

In passing, fix a couple of oversights in the "large chunk" path in
GenerationAlloc().

Discussion: https://postgr.es/m/E1eHa4J-0006hI-Q8@gemulon.postgresql.org
parent cc3c4af4
...@@ -157,6 +157,14 @@ typedef struct AllocBlockData ...@@ -157,6 +157,14 @@ typedef struct AllocBlockData
/* /*
* AllocChunk * AllocChunk
* The prefix of each piece of memory in an AllocBlock * The prefix of each piece of memory in an AllocBlock
*
* Note: to meet the memory context APIs, the payload area of the chunk must
* be maxaligned, and the "aset" link must be immediately adjacent to the
* payload area (cf. GetMemoryChunkContext). We simplify matters for this
* module by requiring sizeof(AllocChunkData) to be maxaligned, and then
* we can ensure things work by adding any required alignment padding before
* the "aset" field. There is a static assertion below that the alignment
* is done correctly.
*/ */
typedef struct AllocChunkData typedef struct AllocChunkData
{ {
...@@ -166,15 +174,19 @@ typedef struct AllocChunkData ...@@ -166,15 +174,19 @@ typedef struct AllocChunkData
/* when debugging memory usage, also store actual requested size */ /* when debugging memory usage, also store actual requested size */
/* this is zero in a free chunk */ /* this is zero in a free chunk */
Size requested_size; Size requested_size;
#if MAXIMUM_ALIGNOF > 4 && SIZEOF_VOID_P == 4
Size padding;
#endif
#define ALLOCCHUNK_RAWSIZE (SIZEOF_SIZE_T * 2 + SIZEOF_VOID_P)
#else
#define ALLOCCHUNK_RAWSIZE (SIZEOF_SIZE_T + SIZEOF_VOID_P)
#endif /* MEMORY_CONTEXT_CHECKING */ #endif /* MEMORY_CONTEXT_CHECKING */
/* ensure proper alignment by adding padding if needed */
#if (ALLOCCHUNK_RAWSIZE % MAXIMUM_ALIGNOF) != 0
char padding[MAXIMUM_ALIGNOF - ALLOCCHUNK_RAWSIZE % MAXIMUM_ALIGNOF];
#endif
/* aset is the owning aset if allocated, or the freelist link if free */ /* aset is the owning aset if allocated, or the freelist link if free */
void *aset; void *aset;
/* there must not be any padding to reach a MAXALIGN boundary here! */ /* there must not be any padding to reach a MAXALIGN boundary here! */
} AllocChunkData; } AllocChunkData;
...@@ -327,8 +339,11 @@ AllocSetContextCreate(MemoryContext parent, ...@@ -327,8 +339,11 @@ AllocSetContextCreate(MemoryContext parent,
{ {
AllocSet set; AllocSet set;
/* Assert we padded AllocChunkData properly */
StaticAssertStmt(ALLOC_CHUNKHDRSZ == MAXALIGN(ALLOC_CHUNKHDRSZ),
"sizeof(AllocChunkData) is not maxaligned");
StaticAssertStmt(offsetof(AllocChunkData, aset) + sizeof(MemoryContext) == StaticAssertStmt(offsetof(AllocChunkData, aset) + sizeof(MemoryContext) ==
MAXALIGN(sizeof(AllocChunkData)), ALLOC_CHUNKHDRSZ,
"padding calculation in AllocChunkData is wrong"); "padding calculation in AllocChunkData is wrong");
/* /*
......
...@@ -46,19 +46,6 @@ ...@@ -46,19 +46,6 @@
#define Generation_BLOCKHDRSZ MAXALIGN(sizeof(GenerationBlock)) #define Generation_BLOCKHDRSZ MAXALIGN(sizeof(GenerationBlock))
#define Generation_CHUNKHDRSZ sizeof(GenerationChunk) #define Generation_CHUNKHDRSZ sizeof(GenerationChunk)
/* Portion of Generation_CHUNKHDRSZ examined outside generation.c. */
#define Generation_CHUNK_PUBLIC \
(offsetof(GenerationChunk, size) + sizeof(Size))
/* Portion of Generation_CHUNKHDRSZ excluding trailing padding. */
#ifdef MEMORY_CONTEXT_CHECKING
#define Generation_CHUNK_USED \
(offsetof(GenerationChunk, requested_size) + sizeof(Size))
#else
#define Generation_CHUNK_USED \
(offsetof(GenerationChunk, size) + sizeof(Size))
#endif
typedef struct GenerationBlock GenerationBlock; /* forward reference */ typedef struct GenerationBlock GenerationBlock; /* forward reference */
typedef struct GenerationChunk GenerationChunk; typedef struct GenerationChunk GenerationChunk;
...@@ -103,28 +90,35 @@ struct GenerationBlock ...@@ -103,28 +90,35 @@ struct GenerationBlock
/* /*
* GenerationChunk * GenerationChunk
* The prefix of each piece of memory in a GenerationBlock * The prefix of each piece of memory in a GenerationBlock
*
* Note: to meet the memory context APIs, the payload area of the chunk must
* be maxaligned, and the "context" link must be immediately adjacent to the
* payload area (cf. GetMemoryChunkContext). We simplify matters for this
* module by requiring sizeof(GenerationChunk) to be maxaligned, and then
* we can ensure things work by adding any required alignment padding before
* the pointer fields. There is a static assertion below that the alignment
* is done correctly.
*/ */
struct GenerationChunk struct GenerationChunk
{ {
/* block owning this chunk */
void *block;
/* size is always the size of the usable space in the chunk */ /* size is always the size of the usable space in the chunk */
Size size; Size size;
#ifdef MEMORY_CONTEXT_CHECKING #ifdef MEMORY_CONTEXT_CHECKING
/* when debugging memory usage, also store actual requested size */ /* when debugging memory usage, also store actual requested size */
/* this is zero in a free chunk */ /* this is zero in a free chunk */
Size requested_size; Size requested_size;
#define GENERATIONCHUNK_RAWSIZE (SIZEOF_VOID_P * 2 + SIZEOF_SIZE_T * 2)
#define GENERATIONCHUNK_RAWSIZE (SIZEOF_SIZE_T * 2 + SIZEOF_VOID_P * 2)
#else #else
#define GENERATIONCHUNK_RAWSIZE (SIZEOF_VOID_P * 2 + SIZEOF_SIZE_T) #define GENERATIONCHUNK_RAWSIZE (SIZEOF_SIZE_T + SIZEOF_VOID_P * 2)
#endif /* MEMORY_CONTEXT_CHECKING */ #endif /* MEMORY_CONTEXT_CHECKING */
/* ensure proper alignment by adding padding if needed */ /* ensure proper alignment by adding padding if needed */
#if (GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF) != 0 #if (GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF) != 0
char padding[MAXIMUM_ALIGNOF - (GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF)]; char padding[MAXIMUM_ALIGNOF - GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF];
#endif #endif
GenerationBlock *block; /* block owning this chunk */
GenerationContext *context; /* owning context */ GenerationContext *context; /* owning context */
/* there must not be any padding to reach a MAXALIGN boundary here! */ /* there must not be any padding to reach a MAXALIGN boundary here! */
}; };
...@@ -210,8 +204,11 @@ GenerationContextCreate(MemoryContext parent, ...@@ -210,8 +204,11 @@ GenerationContextCreate(MemoryContext parent,
{ {
GenerationContext *set; GenerationContext *set;
/* Assert we padded GenerationChunk properly */
StaticAssertStmt(Generation_CHUNKHDRSZ == MAXALIGN(Generation_CHUNKHDRSZ),
"sizeof(GenerationChunk) is not maxaligned");
StaticAssertStmt(offsetof(GenerationChunk, context) + sizeof(MemoryContext) == StaticAssertStmt(offsetof(GenerationChunk, context) + sizeof(MemoryContext) ==
MAXALIGN(sizeof(GenerationChunk)), Generation_CHUNKHDRSZ,
"padding calculation in GenerationChunk is wrong"); "padding calculation in GenerationChunk is wrong");
/* /*
...@@ -318,7 +315,6 @@ GenerationAlloc(MemoryContext context, Size size) ...@@ -318,7 +315,6 @@ GenerationAlloc(MemoryContext context, Size size)
GenerationContext *set = (GenerationContext *) context; GenerationContext *set = (GenerationContext *) context;
GenerationBlock *block; GenerationBlock *block;
GenerationChunk *chunk; GenerationChunk *chunk;
Size chunk_size = MAXALIGN(size); Size chunk_size = MAXALIGN(size);
/* is it an over-sized chunk? if yes, allocate special block */ /* is it an over-sized chunk? if yes, allocate special block */
...@@ -338,6 +334,7 @@ GenerationAlloc(MemoryContext context, Size size) ...@@ -338,6 +334,7 @@ GenerationAlloc(MemoryContext context, Size size)
block->freeptr = block->endptr = ((char *) block) + blksize; block->freeptr = block->endptr = ((char *) block) + blksize;
chunk = (GenerationChunk *) (((char *) block) + Generation_BLOCKHDRSZ); chunk = (GenerationChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
chunk->block = block;
chunk->context = set; chunk->context = set;
chunk->size = chunk_size; chunk->size = chunk_size;
...@@ -356,17 +353,16 @@ GenerationAlloc(MemoryContext context, Size size) ...@@ -356,17 +353,16 @@ GenerationAlloc(MemoryContext context, Size size)
/* add the block to the list of allocated blocks */ /* add the block to the list of allocated blocks */
dlist_push_head(&set->blocks, &block->node); dlist_push_head(&set->blocks, &block->node);
GenerationAllocInfo(set, chunk);
/* /*
* Chunk header public fields remain DEFINED. The requested * Chunk header fields remain DEFINED. The requested allocation
* allocation itself can be NOACCESS or UNDEFINED; our caller will * itself can be NOACCESS or UNDEFINED; our caller will soon make it
* soon make it UNDEFINED. Make extra space at the end of the chunk, * UNDEFINED. Make extra space at the end of the chunk, if any,
* if any, NOACCESS. * NOACCESS.
*/ */
VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + Generation_CHUNK_PUBLIC, VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + Generation_CHUNKHDRSZ + size,
chunk_size + Generation_CHUNKHDRSZ - Generation_CHUNK_PUBLIC); chunk_size - size);
GenerationAllocInfo(set, chunk);
return GenerationChunkGetPointer(chunk); return GenerationChunkGetPointer(chunk);
} }
...@@ -442,8 +438,8 @@ GenerationAlloc(MemoryContext context, Size size) ...@@ -442,8 +438,8 @@ GenerationAlloc(MemoryContext context, Size size)
/* /*
* GenerationFree * GenerationFree
* Update number of chunks on the block, and if all chunks on the block * Update number of chunks in the block, and if all chunks in the block
* are freeed then discard the block. * are now free then discard the block.
*/ */
static void static void
GenerationFree(MemoryContext context, void *pointer) GenerationFree(MemoryContext context, void *pointer)
......
...@@ -91,12 +91,18 @@ typedef struct SlabBlock ...@@ -91,12 +91,18 @@ typedef struct SlabBlock
/* /*
* SlabChunk * SlabChunk
* The prefix of each piece of memory in an SlabBlock * The prefix of each piece of memory in a SlabBlock
*
* Note: to meet the memory context APIs, the payload area of the chunk must
* be maxaligned, and the "slab" link must be immediately adjacent to the
* payload area (cf. GetMemoryChunkContext). Since we support no machines on
* which MAXALIGN is more than twice sizeof(void *), this happens without any
* special hacking in this struct declaration. But there is a static
* assertion below that the alignment is done correctly.
*/ */
typedef struct SlabChunk typedef struct SlabChunk
{ {
/* block owning this chunk */ SlabBlock *block; /* block owning this chunk */
void *block;
SlabContext *slab; /* owning context */ SlabContext *slab; /* owning context */
/* there must not be any padding to reach a MAXALIGN boundary here! */ /* there must not be any padding to reach a MAXALIGN boundary here! */
} SlabChunk; } SlabChunk;
...@@ -190,8 +196,11 @@ SlabContextCreate(MemoryContext parent, ...@@ -190,8 +196,11 @@ SlabContextCreate(MemoryContext parent,
Size freelistSize; Size freelistSize;
SlabContext *slab; SlabContext *slab;
/* Assert we padded SlabChunk properly */
StaticAssertStmt(sizeof(SlabChunk) == MAXALIGN(sizeof(SlabChunk)),
"sizeof(SlabChunk) is not maxaligned");
StaticAssertStmt(offsetof(SlabChunk, slab) + sizeof(MemoryContext) == StaticAssertStmt(offsetof(SlabChunk, slab) + sizeof(MemoryContext) ==
MAXALIGN(sizeof(SlabChunk)), sizeof(SlabChunk),
"padding calculation in SlabChunk is wrong"); "padding calculation in SlabChunk is wrong");
/* Make sure the linked list node fits inside a freed chunk */ /* Make sure the linked list node fits inside a freed chunk */
...@@ -199,7 +208,7 @@ SlabContextCreate(MemoryContext parent, ...@@ -199,7 +208,7 @@ SlabContextCreate(MemoryContext parent,
chunkSize = sizeof(int); chunkSize = sizeof(int);
/* chunk, including SLAB header (both addresses nicely aligned) */ /* chunk, including SLAB header (both addresses nicely aligned) */
fullChunkSize = MAXALIGN(sizeof(SlabChunk) + MAXALIGN(chunkSize)); fullChunkSize = sizeof(SlabChunk) + MAXALIGN(chunkSize);
/* Make sure the block can store at least one chunk. */ /* Make sure the block can store at least one chunk. */
if (blockSize - sizeof(SlabBlock) < fullChunkSize) if (blockSize - sizeof(SlabBlock) < fullChunkSize)
...@@ -443,7 +452,7 @@ SlabAlloc(MemoryContext context, Size size) ...@@ -443,7 +452,7 @@ SlabAlloc(MemoryContext context, Size size)
/* Prepare to initialize the chunk header. */ /* Prepare to initialize the chunk header. */
VALGRIND_MAKE_MEM_UNDEFINED(chunk, sizeof(SlabChunk)); VALGRIND_MAKE_MEM_UNDEFINED(chunk, sizeof(SlabChunk));
chunk->block = (void *) block; chunk->block = block;
chunk->slab = slab; chunk->slab = slab;
#ifdef MEMORY_CONTEXT_CHECKING #ifdef MEMORY_CONTEXT_CHECKING
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment