On Sun, 2019-09-29 at 00:22 +0200, Tomas Vondra wrote:
> Notice that when CLOBBER_FREED_MEMORY is defined, the code first
> > calls
> > wipe_mem and then accesses fields of the (wiped) block.
> > Interesringly
> > enough, the regression tests don't seem to exercise these bits -
> > I've
> > tried adding elog(ERROR) and it still passes. For (2) that's not
> > very
> > surprising because Generation context is only really used in
> > logical
> > decoding (and we don't delete the context I think). Not sure about
> > (1)
> > but it might be because AllocSetReset does the right thing and only
> > leaves behind the keeper block.
> >
> > I'm pretty sure a custom function calling the contexts explicitly
> > would
> > fall over, but I haven't tried.
> >
Fixed.
I tested with some custom use of memory contexts. The reason
AllocSetDelete() didn't fail before is that most memory contexts use
the free lists (the list of free memory contexts, not the free list of
chunks), so you need to specify a non-default minsize in order to
prevent that and trigger the bug.
AllocSetReset() worked, but it was reading the header of the keeper
block after wiping the contents of the keeper block. It technically
worked, because the header of the keeper block was not wiped, but it
seems more clear to explicitly save the size of the keeper block. In
AllocSetDelete(), saving the keeper size is required, because it wipes
the block headers in addition to the contents.
> Oh, and one more thing - this probably needs to add at least some
> basic
> explanation of the accounting to src/backend/mmgr/README.
Added.
Regards,
Jeff Davis
From 7184df8a928d6c0c6e41d171968a54d46f256a22 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jda...@postgresql.org>
Date: Fri, 1 Jun 2018 13:35:21 -0700
Subject: [PATCH] Memory accounting at block level.
Track the memory allocated to a memory context in the form of blocks,
and introduce a new function MemoryContextMemAllocated() to report it
to the caller in bytes. This does not track individual chunks.
---
src/backend/utils/mmgr/README | 4 +++
src/backend/utils/mmgr/aset.c | 38 +++++++++++++++++++++++++++++
src/backend/utils/mmgr/generation.c | 19 ++++++++++++---
src/backend/utils/mmgr/mcxt.c | 24 ++++++++++++++++++
src/backend/utils/mmgr/slab.c | 10 ++++++++
src/include/nodes/memnodes.h | 1 +
src/include/utils/memutils.h | 1 +
7 files changed, 93 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/mmgr/README b/src/backend/utils/mmgr/README
index 7e6541d0dee..bd5fa6d8e53 100644
--- a/src/backend/utils/mmgr/README
+++ b/src/backend/utils/mmgr/README
@@ -23,6 +23,10 @@ The basic operations on a memory context are:
* reset a context (free all memory allocated in the context, but not the
context object itself)
+* inquire about the total amount of memory allocated to the context
+ (the raw memory from which the context allocates chunks; not the
+ chunks themselves)
+
Given a chunk of memory previously allocated from a context, one can
free it or reallocate it larger or smaller (corresponding to standard C
library's free() and realloc() routines). These operations return memory
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 6b63d6f85d0..90f370570fe 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -458,6 +458,9 @@ AllocSetContextCreateInternal(MemoryContext parent,
parent,
name);
+ ((MemoryContext) set)->mem_allocated =
+ set->keeper->endptr - ((char *) set);
+
return (MemoryContext) set;
}
}
@@ -546,6 +549,8 @@ AllocSetContextCreateInternal(MemoryContext parent,
parent,
name);
+ ((MemoryContext) set)->mem_allocated = firstBlockSize;
+
return (MemoryContext) set;
}
@@ -566,6 +571,7 @@ AllocSetReset(MemoryContext context)
{
AllocSet set = (AllocSet) context;
AllocBlock block;
+ Size keepersize = set->keeper->endptr - ((char *) set);
AssertArg(AllocSetIsValid(set));
@@ -604,6 +610,8 @@ AllocSetReset(MemoryContext context)
else
{
/* Normal case, release the block */
+ context->mem_allocated -= block->endptr - ((char*) block);
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
@@ -612,6 +620,8 @@ AllocSetReset(MemoryContext context)
block = next;
}
+ Assert(context->mem_allocated == keepersize);
+
/* Reset block size allocation sequence, too */
set->nextBlockSize = set->initBlockSize;
}
@@ -628,6 +638,7 @@ AllocSetDelete(MemoryContext context)
{
AllocSet set = (AllocSet) context;
AllocBlock block = set->blocks;
+ Size keepersize = set->keeper->endptr - ((char *) set);
AssertArg(AllocSetIsValid(set));
@@ -683,6 +694,9 @@ AllocSetDelete(MemoryContext context)
{
AllocBlock next = block->next;
+ if (block != set->keeper)
+ context->mem_allocated -= block->endptr - ((char *) block);
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
@@ -693,6 +707,8 @@ AllocSetDelete(MemoryContext context)
block = next;
}
+ Assert(context->mem_allocated == keepersize);
+
/* Finally, free the context header, including the keeper block */
free(set);
}
@@ -733,6 +749,9 @@ AllocSetAlloc(MemoryContext context, Size size)
block = (AllocBlock) malloc(blksize);
if (block == NULL)
return NULL;
+
+ context->mem_allocated += blksize;
+
block->aset = set;
block->freeptr = block->endptr = ((char *) block) + blksize;
@@ -928,6 +947,8 @@ AllocSetAlloc(MemoryContext context, Size size)
if (block == NULL)
return NULL;
+ context->mem_allocated += blksize;
+
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize;
@@ -1028,6 +1049,9 @@ AllocSetFree(MemoryContext context, void *pointer)
set->blocks = block->next;
if (block->next)
block->next->prev = block->prev;
+
+ context->mem_allocated -= block->endptr - ((char*) block);
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
@@ -1144,6 +1168,7 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ);
Size chksize;
Size blksize;
+ Size oldblksize;
/*
* Try to verify that we have a sane block pointer: it should
@@ -1159,6 +1184,8 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
/* Do the realloc */
chksize = MAXALIGN(size);
blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
+ oldblksize = block->endptr - ((char *)block);
+
block = (AllocBlock) realloc(block, blksize);
if (block == NULL)
{
@@ -1166,6 +1193,9 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOCCHUNK_PRIVATE_LEN);
return NULL;
}
+
+ context->mem_allocated += blksize - oldblksize;
+
block->freeptr = block->endptr = ((char *) block) + blksize;
/* Update pointers since block has likely been moved */
@@ -1383,6 +1413,7 @@ AllocSetCheck(MemoryContext context)
const char *name = set->header.name;
AllocBlock prevblock;
AllocBlock block;
+ int64 total_allocated = 0;
for (prevblock = NULL, block = set->blocks;
block != NULL;
@@ -1393,6 +1424,11 @@ AllocSetCheck(MemoryContext context)
long blk_data = 0;
long nchunks = 0;
+ if (set->keeper == block)
+ total_allocated += block->endptr - ((char *) set);
+ else
+ total_allocated += block->endptr - ((char *) block);
+
/*
* Empty block - empty can be keeper-block only
*/
@@ -1479,6 +1515,8 @@ AllocSetCheck(MemoryContext context)
elog(WARNING, "problem in alloc set %s: found inconsistent memory block %p",
name, block);
}
+
+ Assert(total_allocated == context->mem_allocated);
}
#endif /* MEMORY_CONTEXT_CHECKING */
diff --git a/src/backend/utils/mmgr/generation.c b/src/backend/utils/mmgr/generation.c
index eaacafb7be5..f1aed7e9384 100644
--- a/src/backend/utils/mmgr/generation.c
+++ b/src/backend/utils/mmgr/generation.c
@@ -297,10 +297,11 @@ GenerationReset(MemoryContext context)
dlist_delete(miter.cur);
+ context->mem_allocated -= block->blksize;
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->blksize);
#endif
-
free(block);
}
@@ -352,6 +353,8 @@ GenerationAlloc(MemoryContext context, Size size)
if (block == NULL)
return NULL;
+ context->mem_allocated += blksize;
+
/* block with a single (used) chunk */
block->blksize = blksize;
block->nchunks = 1;
@@ -407,6 +410,8 @@ GenerationAlloc(MemoryContext context, Size size)
if (block == NULL)
return NULL;
+ context->mem_allocated += blksize;
+
block->blksize = blksize;
block->nchunks = 0;
block->nfree = 0;
@@ -522,6 +527,7 @@ GenerationFree(MemoryContext context, void *pointer)
if (set->block == block)
set->block = NULL;
+ context->mem_allocated -= block->blksize;
free(block);
}
@@ -743,9 +749,10 @@ GenerationStats(MemoryContext context,
static void
GenerationCheck(MemoryContext context)
{
- GenerationContext *gen = (GenerationContext *) context;
- const char *name = context->name;
- dlist_iter iter;
+ GenerationContext *gen = (GenerationContext *) context;
+ const char *name = context->name;
+ dlist_iter iter;
+ int64 total_allocated = 0;
/* walk all blocks in this context */
dlist_foreach(iter, &gen->blocks)
@@ -755,6 +762,8 @@ GenerationCheck(MemoryContext context)
nchunks;
char *ptr;
+ total_allocated += block->blksize;
+
/*
* nfree > nchunks is surely wrong, and we don't expect to see
* equality either, because such a block should have gotten freed.
@@ -833,6 +842,8 @@ GenerationCheck(MemoryContext context)
elog(WARNING, "problem in Generation %s: number of free chunks %d in block %p does not match header %d",
name, nfree, block, block->nfree);
}
+
+ Assert(total_allocated == context->mem_allocated);
}
#endif /* MEMORY_CONTEXT_CHECKING */
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index b07be122369..27417af548d 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -462,6 +462,29 @@ MemoryContextIsEmpty(MemoryContext context)
return context->methods->is_empty(context);
}
+/*
+ * Find the memory allocated to blocks for this memory context. If recurse is
+ * true, also include children.
+ */
+int64
+MemoryContextMemAllocated(MemoryContext context, bool recurse)
+{
+ int64 total = context->mem_allocated;
+
+ AssertArg(MemoryContextIsValid(context));
+
+ if (recurse)
+ {
+ MemoryContext child = context->firstchild;
+ for (child = context->firstchild;
+ child != NULL;
+ child = child->nextchild)
+ total += MemoryContextMemAllocated(child, true);
+ }
+
+ return total;
+}
+
/*
* MemoryContextStats
* Print statistics about the named context and all its descendants.
@@ -736,6 +759,7 @@ MemoryContextCreate(MemoryContext node,
node->methods = methods;
node->parent = parent;
node->firstchild = NULL;
+ node->mem_allocated = 0;
node->prevchild = NULL;
node->name = name;
node->ident = NULL;
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index 700a91a2a37..50deb354c28 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -305,12 +305,14 @@ SlabReset(MemoryContext context)
#endif
free(block);
slab->nblocks--;
+ context->mem_allocated -= slab->blockSize;
}
}
slab->minFreeChunks = 0;
Assert(slab->nblocks == 0);
+ Assert(context->mem_allocated == 0);
}
/*
@@ -388,6 +390,7 @@ SlabAlloc(MemoryContext context, Size size)
slab->minFreeChunks = slab->chunksPerBlock;
slab->nblocks += 1;
+ context->mem_allocated += slab->blockSize;
}
/* grab the block from the freelist (even the new block is there) */
@@ -480,6 +483,9 @@ SlabAlloc(MemoryContext context, Size size)
#endif
SlabAllocInfo(slab, chunk);
+
+ Assert(slab->nblocks * slab->blockSize == context->mem_allocated);
+
return SlabChunkGetPointer(chunk);
}
@@ -555,11 +561,13 @@ SlabFree(MemoryContext context, void *pointer)
{
free(block);
slab->nblocks--;
+ context->mem_allocated -= slab->blockSize;
}
else
dlist_push_head(&slab->freelist[block->nfree], &block->node);
Assert(slab->nblocks >= 0);
+ Assert(slab->nblocks * slab->blockSize == context->mem_allocated);
}
/*
@@ -782,6 +790,8 @@ SlabCheck(MemoryContext context)
name, block->nfree, block, nfree);
}
}
+
+ Assert(slab->nblocks * slab->blockSize == context->mem_allocated);
}
#endif /* MEMORY_CONTEXT_CHECKING */
diff --git a/src/include/nodes/memnodes.h b/src/include/nodes/memnodes.h
index dbae98d3d9f..df0ae3625cb 100644
--- a/src/include/nodes/memnodes.h
+++ b/src/include/nodes/memnodes.h
@@ -79,6 +79,7 @@ typedef struct MemoryContextData
/* these two fields are placed here to minimize alignment wastage: */
bool isReset; /* T = no space alloced since last reset */
bool allowInCritSection; /* allow palloc in critical section */
+ int64 mem_allocated; /* track memory allocated for this context */
const MemoryContextMethods *methods; /* virtual function table */
MemoryContext parent; /* NULL if no parent (toplevel context) */
MemoryContext firstchild; /* head of linked list of children */
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index ffe6de536e2..6a837bc9902 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -82,6 +82,7 @@ extern void MemoryContextSetParent(MemoryContext context,
extern Size GetMemoryChunkSpace(void *pointer);
extern MemoryContext MemoryContextGetParent(MemoryContext context);
extern bool MemoryContextIsEmpty(MemoryContext context);
+extern int64 MemoryContextMemAllocated(MemoryContext context, bool recurse);
extern void MemoryContextStats(MemoryContext context);
extern void MemoryContextStatsDetail(MemoryContext context, int max_children);
extern void MemoryContextAllowInCriticalSection(MemoryContext context,
--
2.17.1
