From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: Tomas Vondra <tv@fuzzy.cz>
Cc: pgsql-hackers@postgresql.org
Subject: Re: SlabCheck leaks memory into TopMemoryContext
Date: Thu, 16 Jan 2020 17:25:00 +0100
Message-ID: <20200116162500.pcgxmykirm677mt6@development> (raw)
In-Reply-To: <19743.1579188421@sss.pgh.pa.us>
References: <20200116044119.g45f7pmgz4jmodxj@alap3.anarazel.de>
<23952.1579151393@sss.pgh.pa.us>
<20200116061732.vvpyexenaortpu5y@alap3.anarazel.de>
<27213.1579155900@sss.pgh.pa.us>
<20200116064143.opcu2vhcktddpzbh@alap3.anarazel.de>
<19743.1579188421@sss.pgh.pa.us>
On Thu, Jan 16, 2020 at 10:27:01AM -0500, Tom Lane wrote:
>Andres Freund <andres@anarazel.de> writes:
>> ... I thought you were asking whether
>> any additional memory could just be avoided...
>
>Well, I was kind of wondering that, but if it's not practical then
>preallocating the space instead will do.
>
I don't think it's practical to rework the checks in a way that would
not require allocations. Maybe it's possible, but I think it's not worth
the extra complexity.
The attached fix should do the trick - it pre-allocates the space when
creating the context. There is a bit of complexity because we want to
allocate the space as part of the context header, but nothin too bad. We
might optimize it a bit by using a regular bitmap (instead of just an
array of bools), but I haven't done that.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:slab-leak-fix.patch (text/plain, 3.0K) downloaddiff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.cindex c5866d9cc3..7f9749a73f 100644--- a/src/backend/utils/mmgr/slab.c+++ b/src/backend/utils/mmgr/slab.c@@ -70,6 +70,9 @@ typedef struct SlabContext
int chunksPerBlock; /* number of chunks per block */
int minFreeChunks; /* min number of free chunks in any block */
int nblocks; /* number of blocks allocated */
+#ifdef MEMORY_CONTEXT_CHECKING+ char *freechunks; /* bitmap of free chunks on a block */+#endif
/* blocks with free space, grouped by number of free chunks: */
dlist_head freelist[FLEXIBLE_ARRAY_MEMBER];
} SlabContext;
@@ -229,6 +232,15 @@ SlabContextCreate(MemoryContext parent,
/* Size of the memory context header */
headerSize = offsetof(SlabContext, freelist) + freelistSize;
+#ifdef MEMORY_CONTEXT_CHECKING+ /*+ * With memory checking, we need to allocate extra space for the bitmap+ * of free chunks. The space is allocated at the end, and we need proper+ * alignment (it's an array of bools, so maybe MAXALIGN is not needed).+ */+ headerSize = MAXALIGN(headerSize) + chunksPerBlock * sizeof(bool);+#endif+
slab = (SlabContext *) malloc(headerSize);
if (slab == NULL)
{
@@ -258,6 +270,12 @@ SlabContextCreate(MemoryContext parent,
for (i = 0; i < (slab->chunksPerBlock + 1); i++)
dlist_init(&slab->freelist[i]);
+#ifdef MEMORY_CONTEXT_CHECKING+ /* set the freechunks pointer after the freelists array (aligned) */+ slab->freechunks = (char *) slab ++ MAXALIGN(offsetof(SlabContext, freelist) + freelistSize);+#endif+
/* Finally, do the type-independent part of context creation */
MemoryContextCreate((MemoryContext) slab,
T_SlabContext,
@@ -701,14 +719,10 @@ SlabCheck(MemoryContext context)
int i;
SlabContext *slab = castNode(SlabContext, context);
const char *name = slab->header.name;
- char *freechunks;
Assert(slab);
Assert(slab->chunksPerBlock > 0);
- /* bitmap of free chunks on a block */- freechunks = palloc(slab->chunksPerBlock * sizeof(bool));-
/* walk all the freelists */
for (i = 0; i <= slab->chunksPerBlock; i++)
{
@@ -731,7 +745,7 @@ SlabCheck(MemoryContext context)
name, block->nfree, block, i);
/* reset the bitmap of free chunks for this block */
- memset(freechunks, 0, (slab->chunksPerBlock * sizeof(bool)));+ memset(slab->freechunks, 0, (slab->chunksPerBlock * sizeof(bool)));
idx = block->firstFreeChunk;
/*
@@ -748,7 +762,7 @@ SlabCheck(MemoryContext context)
/* count the chunk as free, add it to the bitmap */
nfree++;
- freechunks[idx] = true;+ slab->freechunks[idx] = true;
/* read index of the next free chunk */
chunk = SlabBlockGetChunk(slab, block, idx);
@@ -759,7 +773,7 @@ SlabCheck(MemoryContext context)
for (j = 0; j < slab->chunksPerBlock; j++)
{
/* non-zero bit in the bitmap means chunk the chunk is used */
- if (!freechunks[j])+ if (!slab->freechunks[j])
{
SlabChunk *chunk = SlabBlockGetChunk(slab, block, j);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, tgl@sss.pgh.pa.us, andres@anarazel.de, tv@fuzzy.cz
Subject: Re: SlabCheck leaks memory into TopMemoryContext
In-Reply-To: <20200116162500.pcgxmykirm677mt6@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox