Re: Reducing the chunk header sizes on all memory context types

[David Rowley]

On Tue, 30 Aug 2022 at 12:45, David Rowley <dgrowle...@gmail.com> wrote:
> I think the existing sentinel check looks wrong:
>
> if (!sentinel_ok(chunk, slab->chunkSize))
>
> shouldn't that be passing the pointer rather than the chunk?

Here's v2 of the slab-fix patch.

I've included the sentinel check fix.  This passes make check-world
for me when do a 32-bit build on my x86_64 machine and adjust
pg_config.h to set MAXIMUM_ALIGNOF to 8.

Any chance you could run make check-world on your 32-bit Raspberry PI?

I'm also wondering if this should also be backpatched back to v10,
providing the build farm likes it well enough on master.

diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index ae1a735b8c..02f365ff80 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -58,6 +58,8 @@
 #include "utils/memutils_memorychunk.h"
 #include "utils/memutils_internal.h"
 
+#define Slab_BLOCKHDRSZ        MAXALIGN(sizeof(SlabBlock))
+
 /*
  * SlabContext is a specialized implementation of MemoryContext.
  */
@@ -102,10 +104,10 @@ typedef struct SlabBlock
 #define SlabChunkGetPointer(chk)       \
        ((void *)(((char *)(chk)) + sizeof(MemoryChunk)))
 #define SlabBlockGetChunk(slab, block, idx) \
-       ((MemoryChunk *) ((char *) (block) + sizeof(SlabBlock)  \
+       ((MemoryChunk *) ((char *) (block) + Slab_BLOCKHDRSZ    \
                                        + (idx * slab->fullChunkSize)))
 #define SlabBlockStart(block)  \
-       ((char *) block + sizeof(SlabBlock))
+       ((char *) block + Slab_BLOCKHDRSZ)
 #define SlabChunkIndex(slab, block, chunk)     \
        (((char *) chunk - SlabBlockStart(block)) / slab->fullChunkSize)
 
@@ -146,12 +148,12 @@ SlabContextCreate(MemoryContext parent,
        fullChunkSize = Slab_CHUNKHDRSZ + MAXALIGN(chunkSize);
 
        /* Make sure the block can store at least one chunk. */
-       if (blockSize < fullChunkSize + sizeof(SlabBlock))
+       if (blockSize < fullChunkSize + Slab_BLOCKHDRSZ)
                elog(ERROR, "block size %zu for slab is too small for %zu 
chunks",
                         blockSize, chunkSize);
 
        /* Compute maximum number of chunks per block */
-       chunksPerBlock = (blockSize - sizeof(SlabBlock)) / fullChunkSize;
+       chunksPerBlock = (blockSize - Slab_BLOCKHDRSZ) / fullChunkSize;
 
        /* The freelist starts with 0, ends with chunksPerBlock. */
        freelistSize = sizeof(dlist_head) * (chunksPerBlock + 1);
@@ -733,6 +735,7 @@ SlabCheck(MemoryContext context)
                                {
                                        MemoryChunk *chunk = 
SlabBlockGetChunk(slab, block, j);
                                        SlabBlock  *chunkblock = (SlabBlock *) 
MemoryChunkGetBlock(chunk);
+                                       void       *pointer = 
MemoryChunkGetPointer(chunk);
 
                                        /*
                                         * check the chunk's blockoffset 
correctly points back to
@@ -744,7 +747,7 @@ SlabCheck(MemoryContext context)
 
                                        /* there might be sentinel (thanks to 
alignment) */
                                        if (slab->chunkSize < 
(slab->fullChunkSize - Slab_CHUNKHDRSZ))
-                                               if (!sentinel_ok(chunk, 
slab->chunkSize))
+                                               if (!sentinel_ok(pointer, 
slab->chunkSize))
                                                        elog(WARNING, "problem 
in slab %s: detected write past chunk end in block %p, chunk %p",
                                                                 name, block, 
chunk);
                                }