From f65d21b258085bdc8ef2cc282ab1ff12da9c595c Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 24 Nov 2017 15:50:22 -0500
Subject: [PATCH] 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 7e3aa03b4. 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
---
src/backend/utils/mmgr/aset.c | 25 ++++++++++---
src/backend/utils/mmgr/generation.c | 58 ++++++++++++++---------------
src/backend/utils/mmgr/slab.c | 21 ++++++++---
3 files changed, 62 insertions(+), 42 deletions(-)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 7033042e2d8..0b017a40316 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -157,6 +157,14 @@ typedef struct AllocBlockData
/*
* AllocChunk
* 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
{
@@ -166,15 +174,19 @@ typedef struct AllocChunkData
/* when debugging memory usage, also store actual requested size */
/* this is zero in a free chunk */
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 */
+ /* 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 */
void *aset;
-
/* there must not be any padding to reach a MAXALIGN boundary here! */
} AllocChunkData;
@@ -327,8 +339,11 @@ AllocSetContextCreate(MemoryContext parent,
{
AllocSet set;
+ /* Assert we padded AllocChunkData properly */
+ StaticAssertStmt(ALLOC_CHUNKHDRSZ == MAXALIGN(ALLOC_CHUNKHDRSZ),
+ "sizeof(AllocChunkData) is not maxaligned");
StaticAssertStmt(offsetof(AllocChunkData, aset) + sizeof(MemoryContext) ==
- MAXALIGN(sizeof(AllocChunkData)),
+ ALLOC_CHUNKHDRSZ,
"padding calculation in AllocChunkData is wrong");
/*
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index a748ee266c2..8dd0a350956 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -46,19 +46,6 @@
#define Generation_BLOCKHDRSZ MAXALIGN(sizeof(GenerationBlock))
#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 GenerationChunk GenerationChunk;
@@ -103,28 +90,35 @@ struct GenerationBlock
/*
* GenerationChunk
* 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
{
- /* block owning this chunk */
- void *block;
-
/* size is always the size of the usable space in the chunk */
Size size;
#ifdef MEMORY_CONTEXT_CHECKING
/* when debugging memory usage, also store actual requested size */
/* this is zero in a free chunk */
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
-#define GENERATIONCHUNK_RAWSIZE (SIZEOF_VOID_P * 2 + SIZEOF_SIZE_T)
+#define GENERATIONCHUNK_RAWSIZE (SIZEOF_SIZE_T + SIZEOF_VOID_P * 2)
#endif /* MEMORY_CONTEXT_CHECKING */
/* ensure proper alignment by adding padding if needed */
#if (GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF) != 0
- char padding[MAXIMUM_ALIGNOF - (GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF)];
+ char padding[MAXIMUM_ALIGNOF - GENERATIONCHUNK_RAWSIZE % MAXIMUM_ALIGNOF];
#endif
+ GenerationBlock *block; /* block owning this chunk */
GenerationContext *context; /* owning context */
/* there must not be any padding to reach a MAXALIGN boundary here! */
};
@@ -210,8 +204,11 @@ GenerationContextCreate(MemoryContext parent,
{
GenerationContext *set;
+ /* Assert we padded GenerationChunk properly */
+ StaticAssertStmt(Generation_CHUNKHDRSZ == MAXALIGN(Generation_CHUNKHDRSZ),
+ "sizeof(GenerationChunk) is not maxaligned");
StaticAssertStmt(offsetof(GenerationChunk, context) + sizeof(MemoryContext) ==
- MAXALIGN(sizeof(GenerationChunk)),
+ Generation_CHUNKHDRSZ,
"padding calculation in GenerationChunk is wrong");
/*
@@ -318,7 +315,6 @@ GenerationAlloc(MemoryContext context, Size size)
GenerationContext *set = (GenerationContext *) context;
GenerationBlock *block;
GenerationChunk *chunk;
-
Size chunk_size = MAXALIGN(size);
/* is it an over-sized chunk? if yes, allocate special block */
@@ -338,6 +334,7 @@ GenerationAlloc(MemoryContext context, Size size)
block->freeptr = block->endptr = ((char *) block) + blksize;
chunk = (GenerationChunk *) (((char *) block) + Generation_BLOCKHDRSZ);
+ chunk->block = block;
chunk->context = set;
chunk->size = chunk_size;
@@ -356,17 +353,16 @@ GenerationAlloc(MemoryContext context, Size size)
/* add the block to the list of allocated blocks */
dlist_push_head(&set->blocks, &block->node);
- GenerationAllocInfo(set, chunk);
-
/*
- * Chunk header public fields remain DEFINED. The requested
- * allocation itself can be NOACCESS or UNDEFINED; our caller will
- * soon make it UNDEFINED. Make extra space at the end of the chunk,
- * if any, NOACCESS.
+ * Chunk header fields remain DEFINED. The requested allocation
+ * itself can be NOACCESS or UNDEFINED; our caller will soon make it
+ * UNDEFINED. Make extra space at the end of the chunk, if any,
+ * NOACCESS.
*/
- VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + Generation_CHUNK_PUBLIC,
- chunk_size + Generation_CHUNKHDRSZ - Generation_CHUNK_PUBLIC);
+ VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + Generation_CHUNKHDRSZ + size,
+ chunk_size - size);
+ GenerationAllocInfo(set, chunk);
return GenerationChunkGetPointer(chunk);
}
@@ -442,8 +438,8 @@ GenerationAlloc(MemoryContext context, Size size)
/*
* GenerationFree
- * Update number of chunks on the block, and if all chunks on the block
- * are freeed then discard the block.
+ * Update number of chunks in the block, and if all chunks in the block
+ * are now free then discard the block.
*/
static void
GenerationFree(MemoryContext context, void *pointer)
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 35de6b6d82a..ee2175278d2 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -91,12 +91,18 @@ typedef struct SlabBlock
/*
* 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
{
- /* block owning this chunk */
- void *block;
+ SlabBlock *block; /* block owning this chunk */
SlabContext *slab; /* owning context */
/* there must not be any padding to reach a MAXALIGN boundary here! */
} SlabChunk;
@@ -190,8 +196,11 @@ SlabContextCreate(MemoryContext parent,
Size freelistSize;
SlabContext *slab;
+ /* Assert we padded SlabChunk properly */
+ StaticAssertStmt(sizeof(SlabChunk) == MAXALIGN(sizeof(SlabChunk)),
+ "sizeof(SlabChunk) is not maxaligned");
StaticAssertStmt(offsetof(SlabChunk, slab) + sizeof(MemoryContext) ==
- MAXALIGN(sizeof(SlabChunk)),
+ sizeof(SlabChunk),
"padding calculation in SlabChunk is wrong");
/* Make sure the linked list node fits inside a freed chunk */
@@ -199,7 +208,7 @@ SlabContextCreate(MemoryContext parent,
chunkSize = sizeof(int);
/* 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. */
if (blockSize - sizeof(SlabBlock) < fullChunkSize)
@@ -443,7 +452,7 @@ SlabAlloc(MemoryContext context, Size size)
/* Prepare to initialize the chunk header. */
VALGRIND_MAKE_MEM_UNDEFINED(chunk, sizeof(SlabChunk));
- chunk->block = (void *) block;
+ chunk->block = block;
chunk->slab = slab;
#ifdef MEMORY_CONTEXT_CHECKING
--
GitLab