On Mon, 25 Mar 2024 at 22:44, Tom Lane <t...@sss.pgh.pa.us> wrote: > > David Rowley <dgrowle...@gmail.com> writes: > > On Tue, 26 Mar 2024 at 03:53, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> Could we move the knowledge of exactly which context type it is out > >> of the per-chunk header and keep it in the block header? > > > I wasn't 100% clear on your opinion about using 010 vs expanding the > > bit-space. Based on the following it sounded like you were not > > outright rejecting the idea of consuming the 010 pattern. > > What I said earlier was that 010 was the least bad choice if we > fail to do any expansibility work; but I'm not happy with failing > to do that.
Okay. > Basically, I'm not happy with consuming the last reasonably-available > pattern for a memory context type that has little claim to being the > Last Context Type We Will Ever Want. Rather than making a further > dent in our ability to detect corrupted chunks, we should do something > towards restoring the expansibility that existed in the original > design. Then we can add bump contexts and whatever else we want. So, would something like the attached make enough IDs available so that we can add the bump context anyway? It extends memory context IDs to 5 bits (32 values), of which - 8 have glibc's malloc pattern of 001/010; - 1 is unused memory's 00000 - 1 is wipe_mem's 11111 - 4 are used by existing contexts (Aset/Generation/Slab/AlignedRedirect) - 18 are newly available. Kind regards, Matthias
From 4c0b4b1af98fcfecf80a30ea1834668b59d543a5 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent <boekewurm+postg...@gmail.com> Date: Thu, 4 Apr 2024 21:34:46 +0200 Subject: [PATCH v0] Add bitspace for more memory context types in MemoryChunk's hdrmask Assuming we don't want to use patterns from unused memory, common glibc malloc patterns, and wipe_mem-ed memory, we now have 18 new IDs to work with again, from 0. In passing, simplify/clean up initialization of unused memory contexts in the mcxt_methods array. Inspiration: https://postgr.es/m/CAApHDvqGSpCU95TmM%3DBp%3D6xjL_nLys4zdZOpfNyWBk97Xrdj2w%40mail.gmail.com --- src/backend/utils/mmgr/mcxt.c | 55 +++++++++++++++--------- src/include/utils/memutils_internal.h | 35 ++++++++++++--- src/include/utils/memutils_memorychunk.h | 15 ++++--- 3 files changed, 73 insertions(+), 32 deletions(-) diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c index 5d426795d9..9373f782ce 100644 --- a/src/backend/utils/mmgr/mcxt.c +++ b/src/backend/utils/mmgr/mcxt.c @@ -102,26 +102,41 @@ static const MemoryContextMethods mcxt_methods[] = { * seems sufficient to provide routines for the methods that might get * invoked from inspection of a chunk (see MCXT_METHOD calls below). */ - - [MCTX_UNUSED1_ID].free_p = BogusFree, - [MCTX_UNUSED1_ID].realloc = BogusRealloc, - [MCTX_UNUSED1_ID].get_chunk_context = BogusGetChunkContext, - [MCTX_UNUSED1_ID].get_chunk_space = BogusGetChunkSpace, - - [MCTX_UNUSED2_ID].free_p = BogusFree, - [MCTX_UNUSED2_ID].realloc = BogusRealloc, - [MCTX_UNUSED2_ID].get_chunk_context = BogusGetChunkContext, - [MCTX_UNUSED2_ID].get_chunk_space = BogusGetChunkSpace, - - [MCTX_UNUSED3_ID].free_p = BogusFree, - [MCTX_UNUSED3_ID].realloc = BogusRealloc, - [MCTX_UNUSED3_ID].get_chunk_context = BogusGetChunkContext, - [MCTX_UNUSED3_ID].get_chunk_space = BogusGetChunkSpace, - - [MCTX_UNUSED4_ID].free_p = BogusFree, - [MCTX_UNUSED4_ID].realloc = BogusRealloc, - [MCTX_UNUSED4_ID].get_chunk_context = BogusGetChunkContext, - [MCTX_UNUSED4_ID].get_chunk_space = BogusGetChunkSpace, +#define BOGUS_MCTX(id) \ + [id].free_p = BogusFree, \ + [id].realloc = BogusRealloc, \ + [id].get_chunk_context = BogusGetChunkContext, \ + [id].get_chunk_space = BogusGetChunkSpace + + BOGUS_MCTX(MCTX_UNUSED1_ID), + BOGUS_MCTX(MCTX_UNUSED2_ID), + BOGUS_MCTX(MCTX_UNUSED3_ID), + BOGUS_MCTX(MCTX_UNUSED4_ID), + BOGUS_MCTX(MCTX_UNUSED5_ID), + BOGUS_MCTX(MCTX_UNUSED6_ID), + BOGUS_MCTX(MCTX_UNUSED7_ID), + BOGUS_MCTX(MCTX_UNUSED8_ID), + BOGUS_MCTX(MCTX_UNUSED9_ID), + BOGUS_MCTX(MCTX_UNUSED10_ID), + BOGUS_MCTX(MCTX_UNUSED11_ID), + BOGUS_MCTX(MCTX_UNUSED12_ID), + BOGUS_MCTX(MCTX_UNUSED13_ID), + BOGUS_MCTX(MCTX_UNUSED14_ID), + BOGUS_MCTX(MCTX_UNUSED15_ID), + BOGUS_MCTX(MCTX_UNUSED16_ID), + BOGUS_MCTX(MCTX_UNUSED17_ID), + BOGUS_MCTX(MCTX_UNUSED18_ID), + BOGUS_MCTX(MCTX_UNUSED19_ID), + BOGUS_MCTX(MCTX_UNUSED20_ID), + BOGUS_MCTX(MCTX_UNUSED21_ID), + BOGUS_MCTX(MCTX_UNUSED22_ID), + BOGUS_MCTX(MCTX_UNUSED23_ID), + BOGUS_MCTX(MCTX_UNUSED24_ID), + BOGUS_MCTX(MCTX_UNUSED25_ID), + BOGUS_MCTX(MCTX_UNUSED26_ID), + BOGUS_MCTX(MCTX_UNUSED27_ID), + BOGUS_MCTX(MCTX_UNUSED28_ID) +#undef BOGUS_MCTX }; /* diff --git a/src/include/utils/memutils_internal.h b/src/include/utils/memutils_internal.h index ad1048fd82..fbf5ecd111 100644 --- a/src/include/utils/memutils_internal.h +++ b/src/include/utils/memutils_internal.h @@ -104,21 +104,46 @@ extern Size AlignedAllocGetChunkSpace(void *pointer); */ typedef enum MemoryContextMethodID { - MCTX_UNUSED1_ID, /* 000 occurs in never-used memory */ - MCTX_UNUSED2_ID, /* glibc malloc'd chunks usually match 001 */ - MCTX_UNUSED3_ID, /* glibc malloc'd chunks > 128kB match 010 */ + MCTX_UNUSED1_ID, /* 00000 occurs in never-used memory */ + MCTX_UNUSED2_ID, /* glibc malloc'd chunks usually match XX001 */ + MCTX_UNUSED3_ID, /* glibc malloc'd chunks > 128kB match XX010 */ MCTX_ASET_ID, MCTX_GENERATION_ID, MCTX_SLAB_ID, MCTX_ALIGNED_REDIRECT_ID, - MCTX_UNUSED4_ID, /* 111 occurs in wipe_mem'd memory */ + MCTX_UNUSED4_ID, /* 00111 */ + MCTX_UNUSED5_ID, /* 01000 */ + MCTX_UNUSED6_ID, /* glibc malloc'd chunks usually match XX001 */ + MCTX_UNUSED7_ID, /* glibc malloc'd chunks > 128kB match XX010 */ + MCTX_UNUSED8_ID, /* 01011 */ + MCTX_UNUSED9_ID, /* 01100 */ + MCTX_UNUSED10_ID, /* 01101 */ + MCTX_UNUSED11_ID, /* 01110 */ + MCTX_UNUSED12_ID, /* 01111 */ + MCTX_UNUSED13_ID, /* 10000 */ + MCTX_UNUSED14_ID, /* glibc malloc'd chunks usually match XX001 */ + MCTX_UNUSED15_ID, /* glibc malloc'd chunks > 128kB match XX010 */ + MCTX_UNUSED16_ID, /* 10011 */ + MCTX_UNUSED17_ID, /* 10100 */ + MCTX_UNUSED18_ID, /* 10101 */ + MCTX_UNUSED19_ID, /* 10110 */ + MCTX_UNUSED20_ID, /* 10111 */ + MCTX_UNUSED21_ID, /* 11000 */ + MCTX_UNUSED22_ID, /* glibc malloc'd chunks usually match XX001 */ + MCTX_UNUSED23_ID, /* glibc malloc'd chunks > 128kB match XX010 */ + MCTX_UNUSED24_ID, /* 11011 */ + MCTX_UNUSED25_ID, /* 11100 */ + MCTX_UNUSED26_ID, /* 11101 */ + MCTX_UNUSED27_ID, /* 11110 */ + MCTX_UNUSED28_ID /* 11111 occurs in wipe_mem'd memory (0x7F) */ +#define MaxMemoryContextMethodID (MCTX_UNUSED28_ID) } MemoryContextMethodID; /* * The number of bits that 8-byte memory chunk headers can use to encode the * MemoryContextMethodID. */ -#define MEMORY_CONTEXT_METHODID_BITS 3 +#define MEMORY_CONTEXT_METHODID_BITS 5 #define MEMORY_CONTEXT_METHODID_MASK \ ((((uint64) 1) << MEMORY_CONTEXT_METHODID_BITS) - 1) diff --git a/src/include/utils/memutils_memorychunk.h b/src/include/utils/memutils_memorychunk.h index 38296abe1b..86ed28afab 100644 --- a/src/include/utils/memutils_memorychunk.h +++ b/src/include/utils/memutils_memorychunk.h @@ -25,14 +25,14 @@ * used to encode 4 separate pieces of information. Starting with the least * significant bits of 'hdrmask', the bit space is reserved as follows: * - * 1. 3-bits to indicate the MemoryContextMethodID as defined by + * 1. 5-bits to indicate the MemoryContextMethodID as defined by * MEMORY_CONTEXT_METHODID_MASK * 2. 1-bit to denote an "external" chunk (see below) * 3. 30-bits reserved for the MemoryContext to use for anything it * requires. Most MemoryContext likely want to store the size of the * chunk here. - * 4. 30-bits for the number of bytes that must be subtracted from the chunk - * to obtain the address of the block that the chunk is stored on. + * 4. 28-bits for the multiple of 4 bytes that must be subtracted from the + * chunk to obtain the address of the block that the chunk is stored on. * * In some cases, for example when memory allocations become large, it's * possible fields 3 and 4 above are not large enough to store the values @@ -91,7 +91,7 @@ * The maximum distance in bytes that a MemoryChunk can be offset from the * block that is storing the chunk. Must be 1 less than a power of 2. */ -#define MEMORYCHUNK_MAX_BLOCKOFFSET UINT64CONST(0x3FFFFFFF) +#define MEMORYCHUNK_MAX_BLOCKOFFSET UINT64CONST(0xFFFFFFF) /* define the least significant base-0 bit of each portion of the hdrmask */ #define MEMORYCHUNK_EXTERNAL_BASEBIT MEMORY_CONTEXT_METHODID_BITS @@ -135,7 +135,7 @@ typedef struct MemoryChunk * optimize out the & MEMORYCHUNK_MAX_BLOCKOFFSET. */ #define HdrMaskBlockOffset(hdrmask) \ - (((hdrmask) >> MEMORYCHUNK_BLOCKOFFSET_BASEBIT) & MEMORYCHUNK_MAX_BLOCKOFFSET) + ((((hdrmask) >> MEMORYCHUNK_BLOCKOFFSET_BASEBIT) & MEMORYCHUNK_MAX_BLOCKOFFSET) * 4) /* For external chunks only, check the magic number matches */ #define HdrMaskCheckMagic(hdrmask) \ @@ -157,11 +157,12 @@ MemoryChunkSetHdrMask(MemoryChunk *chunk, void *block, Size blockoffset = (char *) chunk - (char *) block; Assert((char *) chunk >= (char *) block); - Assert(blockoffset <= MEMORYCHUNK_MAX_BLOCKOFFSET); + Assert((blockoffset % 4) == 0); + Assert((blockoffset / 4) <= MEMORYCHUNK_MAX_BLOCKOFFSET); Assert(value <= MEMORYCHUNK_MAX_VALUE); Assert((int) methodid <= MEMORY_CONTEXT_METHODID_MASK); - chunk->hdrmask = (((uint64) blockoffset) << MEMORYCHUNK_BLOCKOFFSET_BASEBIT) | + chunk->hdrmask = (((uint64) (blockoffset / 4)) << MEMORYCHUNK_BLOCKOFFSET_BASEBIT) | (((uint64) value) << MEMORYCHUNK_VALUE_BASEBIT) | methodid; } -- 2.40.1
