On Thu, Jan 29, 2026 at 9:11 PM Ashutosh Bapat <[email protected]> wrote: > > On Thu, Jan 29, 2026 at 5:58 PM Heikki Linnakangas <[email protected]> wrote: > > > > On 29/01/2026 11:56, Ashutosh Bapat wrote: > > > 0001: Adds assertions to InitShmemAccess() and InitShmemAllocation > > > which indicate the conditions, EXEC_BACKEND and IsUnderPostmaster, > > > these functions are expected to be called. I found these annotations > > > to be useful when modifying these functions to handle multiple Shmem > > > segments required by buffer pool resizing project [1]. > > > > > > 0002: We use two different methods to pass ShmemIndex and ShmemLock > > > respectively to new backends even though both the structures are > > > allocated before creating named structures in the shared memory. This > > > patch consistently uses the same method - passing via PGShmemHeader. > > > Further the patch gets rid of InitShmemAllocation and moves that code > > > inside InitShmemAccess() itself. Maybe that's overkill but at least we > > > would be able to call InitShmemAllocation() from InitShmemAccess() and > > > declare first as static within shmem.c. That way we avoid a minor risk > > > of InitShmemAllocation() being called twice. > > > > > > We may achieve consistency by passing ShmemIndex through > > > BackendParameter, but I haven't tried that. > > > > Hmm, I agree it's inconsistent currently. And I'd like to reduce the use > > of BackendParameters, I don't like that mechanism. > > > > I don't much like moving the 'shmem_lock' pointer to PGShmemHeader > > either though. It feels like a modularity violation, as no one else than > > shmem.c should be accessing those fields. The same goes for the existing > > 'index' and 'freeoffset' fields really. > > > > Also, does it make sense to have those fields in the "shim" shmem > > segment that PGSharedMemoryCreate() creates? It's a little confusing, we > > don't keep the 'freeoffset' in the shim up-to-date, for example. > > > > One idea is to move all those fields to another struct, see attached. > > What do you think? > > This looks good. Good that we got rid of ShmemAllocUnlocked() too. > > A nitpick should content_offset be contentOffset (like totalSize) or > content_offset (like dsm_control)? I am ok either way.
Huh, what was I looking at? there is no totalSize in PGShmemHeader. The new member's name fits the style of other names. Ignore this comment. > > A minor discomfort I have is ShmemBase, which is the starting address > that the allocator will use, remains outside of ShmemAllocatorData(). > But it doesn't change once set so no need to "share" it through the > memory and also that will create a self-referencing pointer within the > shared memory. So it's fine. > I think we can just get rid of ShmemBase and ShmemEnd. Their values can be derived from other variables at run time as done in the attached patch (0002). Alternatively, we could add them to ShmemAllocatorData itself to keep everything related to allocation together. But it's not really needed. I wanted to go as far as creating yet another structure to hold ShmemSegHdr and ShmemAllocator together. Having a structure will help in the shared buffer resizing project which needs multiple shared memory segments. But it can wait. What do you think? Added this to CF https://commitfest.postgresql.org/patch/6443/ to get exercised by CFBot. -- Best Wishes, Ashutosh Bapat
From 9600014545bef54334bc487fea5ebeac8e3815bf Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Fri, 30 Jan 2026 14:30:02 +0530 Subject: [PATCH v20260130 2/2] Remove ShmemBase and ShmemEnd We have a new structure ShmemAllocatorData that controls the new alllocations. Ideally ShmemBase and ShmemEnd should be part of that structure since they are used for allocation purposes. But adding them to the shared memory structure would increase the size of the shared memory consumed. Further it's easy to get rid of them since they can be derived at run time from other values. Author: Ashutosh Bapat <[email protected]> --- src/backend/storage/ipc/shmem.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index b0ea3d82386..61a5dda3c32 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -77,6 +77,8 @@ #include "utils/builtins.h" /* + * Structure controlling shared memory allocations. + * * This is the first data structure stored in the shared memory segment, at * the offset that PGShmemHeader->content_offset points to. Allocations by * ShmemAlloc() are carved out of the space after this. @@ -97,10 +99,9 @@ static void *ShmemAllocRaw(Size size, Size *allocated_size); /* shared memory global variables */ -static PGShmemHeader *ShmemSegHdr; /* shared mem segment header */ +static PGShmemHeader *ShmemSegHdr; /* shared mem segment header and also the + * base address of shared memory */ -static void *ShmemBase; /* start address of shared memory */ -static void *ShmemEnd; /* end+1 address of shared memory */ static ShmemAllocatorData *ShmemAllocator; slock_t *ShmemLock; /* points to ShmemAllocator->shmem_lock */ @@ -133,8 +134,6 @@ InitShmemAllocator(PGShmemHeader *seghdr) Assert(seghdr->content_offset == MAXALIGN(seghdr->content_offset)); ShmemSegHdr = seghdr; - ShmemBase = seghdr; - ShmemEnd = (char *) ShmemBase + seghdr->totalsize; #ifndef EXEC_BACKEND Assert(!IsUnderPostmaster); @@ -220,6 +219,7 @@ ShmemAllocRaw(Size size, Size *allocated_size) Size newStart; Size newFree; void *newSpace; + char *ShmemBase = (char *) ShmemSegHdr; /* * Ensure all space is adequately aligned. We used to only MAXALIGN this @@ -244,7 +244,7 @@ ShmemAllocRaw(Size size, Size *allocated_size) newFree = newStart + size; if (newFree <= ShmemSegHdr->totalsize) { - newSpace = (char *) ShmemBase + newStart; + newSpace = ShmemBase + newStart; ShmemAllocator->freeoffset = newFree; } else @@ -266,6 +266,9 @@ ShmemAllocRaw(Size size, Size *allocated_size) bool ShmemAddrIsValid(const void *addr) { + void *ShmemBase = ShmemSegHdr; + void *ShmemEnd = (char *) ShmemBase + ShmemSegHdr->totalsize; + return (addr >= ShmemBase) && (addr < ShmemEnd); } -- 2.34.1
From 0786c033ad2abcd3da06e274a290ffd6d84c389f Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <[email protected]> Date: Thu, 29 Jan 2026 11:16:45 +0530 Subject: [PATCH v20260130 1/2] Move shmem allocator fields to its own struct --- src/backend/port/sysv_shmem.c | 2 +- src/backend/postmaster/launch_backend.c | 7 +- src/backend/storage/ipc/ipci.c | 4 +- src/backend/storage/ipc/shmem.c | 161 +++++++++++------------- src/include/storage/pg_shmem.h | 4 +- src/include/storage/shmem.h | 3 +- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 82 insertions(+), 100 deletions(-) diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c index 3cd3544fa2b..2e3886cf9fe 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -855,7 +855,7 @@ PGSharedMemoryCreate(Size size, * Initialize space allocation status for segment. */ hdr->totalsize = size; - hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader)); + hdr->content_offset = MAXALIGN(sizeof(PGShmemHeader)); *shim = hdr; /* Save info for possible future use */ diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index cea229ad6a4..45690b11c99 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -96,7 +96,6 @@ typedef struct HANDLE UsedShmemSegID; #endif void *UsedShmemSegAddr; - slock_t *ShmemLock; #ifdef USE_INJECTION_POINTS struct InjectionPointsCtl *ActiveInjectionPoints; #endif @@ -676,7 +675,7 @@ SubPostmasterMain(int argc, char *argv[]) /* Restore basic shared memory pointers */ if (UsedShmemSegAddr != NULL) - InitShmemAccess(UsedShmemSegAddr); + InitShmemAllocator(UsedShmemSegAddr); /* * Run the appropriate Main function @@ -724,8 +723,6 @@ save_backend_variables(BackendParameters *param, param->UsedShmemSegID = UsedShmemSegID; param->UsedShmemSegAddr = UsedShmemSegAddr; - param->ShmemLock = ShmemLock; - #ifdef USE_INJECTION_POINTS param->ActiveInjectionPoints = ActiveInjectionPoints; #endif @@ -986,8 +983,6 @@ restore_backend_variables(BackendParameters *param) UsedShmemSegID = param->UsedShmemSegID; UsedShmemSegAddr = param->UsedShmemSegAddr; - ShmemLock = param->ShmemLock; - #ifdef USE_INJECTION_POINTS ActiveInjectionPoints = param->ActiveInjectionPoints; #endif diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index 2a3dfedf7e9..1f7e933d500 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -212,12 +212,10 @@ CreateSharedMemoryAndSemaphores(void) Assert(strcmp("unknown", GetConfigOption("huge_pages_status", false, false)) != 0); - InitShmemAccess(seghdr); - /* * Set up shared memory allocation mechanism */ - InitShmemAllocation(); + InitShmemAllocator(seghdr); /* Initialize subsystems */ CreateOrAttachShmemStructs(); diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index 1b536363152..b0ea3d82386 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -76,19 +76,34 @@ #include "storage/spin.h" #include "utils/builtins.h" +/* + * This is the first data structure stored in the shared memory segment, at + * the offset that PGShmemHeader->content_offset points to. Allocations by + * ShmemAlloc() are carved out of the space after this. + * + * For the base pointer and the total size of the shmem segment, we rely on + * the PGShmemHeader. + */ +typedef struct ShmemAllocatorData +{ + Size freeoffset; /* offset to first free space */ + HTAB *index; /* pointer to ShmemIndex table */ + + /* protects shared memory and LWLock allocation */ + slock_t shmem_lock; +} ShmemAllocatorData; + static void *ShmemAllocRaw(Size size, Size *allocated_size); -static void *ShmemAllocUnlocked(Size size); /* shared memory global variables */ static PGShmemHeader *ShmemSegHdr; /* shared mem segment header */ static void *ShmemBase; /* start address of shared memory */ - static void *ShmemEnd; /* end+1 address of shared memory */ +static ShmemAllocatorData *ShmemAllocator; -slock_t *ShmemLock; /* spinlock for shared memory and LWLock - * allocation */ +slock_t *ShmemLock; /* points to ShmemAllocator->shmem_lock */ static HTAB *ShmemIndex = NULL; /* primary index hashtable for shmem */ @@ -98,49 +113,64 @@ static bool firstNumaTouch = true; Datum pg_numa_available(PG_FUNCTION_ARGS); /* - * InitShmemAccess() --- set up basic pointers to shared memory. + * InitShmemAllocator() --- set up basic pointers to shared memory. + * + * Called at postmaster or stand-alone backend startup, to initialize the + * allocator's data structure in the shared memory segment. In EXEC_BACKEND, + * this is also called at backend startup, to set up pointers to the shared + * memory areas. */ void -InitShmemAccess(PGShmemHeader *seghdr) +InitShmemAllocator(PGShmemHeader *seghdr) { + Assert(seghdr != NULL); + + /* + * We assume the pointer and offset are MAXALIGN. Not a hard requirement, + * but it's true today and keeps the math below simpler. + */ + Assert(seghdr == (void *) MAXALIGN(seghdr)); + Assert(seghdr->content_offset == MAXALIGN(seghdr->content_offset)); + ShmemSegHdr = seghdr; ShmemBase = seghdr; ShmemEnd = (char *) ShmemBase + seghdr->totalsize; -} -/* - * InitShmemAllocation() --- set up shared-memory space allocation. - * - * This should be called only in the postmaster or a standalone backend. - */ -void -InitShmemAllocation(void) -{ - PGShmemHeader *shmhdr = ShmemSegHdr; - char *aligned; +#ifndef EXEC_BACKEND + Assert(!IsUnderPostmaster); +#endif + if (IsUnderPostmaster) + { + PGShmemHeader *shmhdr = ShmemSegHdr; - Assert(shmhdr != NULL); + ShmemAllocator = (ShmemAllocatorData *) ((char *) shmhdr + shmhdr->content_offset); + ShmemLock = &ShmemAllocator->shmem_lock; + } + else + { + Size offset; - /* - * Initialize the spinlock used by ShmemAlloc. We must use - * ShmemAllocUnlocked, since obviously ShmemAlloc can't be called yet. - */ - ShmemLock = (slock_t *) ShmemAllocUnlocked(sizeof(slock_t)); + /* + * Allocations after this point should go through ShmemAlloc, which + * expects to allocate everything on cache line boundaries. Make sure + * the first allocation begins on a cache line boundary. + */ + offset = CACHELINEALIGN(seghdr->content_offset + sizeof(ShmemAllocatorData)); + if (offset > seghdr->totalsize) + ereport(ERROR, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of shared memory (%zu bytes requested)", + offset))); - SpinLockInit(ShmemLock); + ShmemAllocator = (ShmemAllocatorData *) ((char *) seghdr + seghdr->content_offset); - /* - * Allocations after this point should go through ShmemAlloc, which - * expects to allocate everything on cache line boundaries. Make sure the - * first allocation begins on a cache line boundary. - */ - aligned = (char *) - (CACHELINEALIGN((((char *) shmhdr) + shmhdr->freeoffset))); - shmhdr->freeoffset = aligned - (char *) shmhdr; - - /* ShmemIndex can't be set up yet (need LWLocks first) */ - shmhdr->index = NULL; - ShmemIndex = (HTAB *) NULL; + SpinLockInit(&ShmemAllocator->shmem_lock); + ShmemLock = &ShmemAllocator->shmem_lock; + ShmemAllocator->freeoffset = offset; + /* ShmemIndex can't be set up yet (need LWLocks first) */ + ShmemAllocator->index = NULL; + ShmemIndex = (HTAB *) NULL; + } } /* @@ -209,13 +239,13 @@ ShmemAllocRaw(Size size, Size *allocated_size) SpinLockAcquire(ShmemLock); - newStart = ShmemSegHdr->freeoffset; + newStart = ShmemAllocator->freeoffset; newFree = newStart + size; if (newFree <= ShmemSegHdr->totalsize) { newSpace = (char *) ShmemBase + newStart; - ShmemSegHdr->freeoffset = newFree; + ShmemAllocator->freeoffset = newFree; } else newSpace = NULL; @@ -228,45 +258,6 @@ ShmemAllocRaw(Size size, Size *allocated_size) return newSpace; } -/* - * ShmemAllocUnlocked -- allocate max-aligned chunk from shared memory - * - * Allocate space without locking ShmemLock. This should be used for, - * and only for, allocations that must happen before ShmemLock is ready. - * - * We consider maxalign, rather than cachealign, sufficient here. - */ -static void * -ShmemAllocUnlocked(Size size) -{ - Size newStart; - Size newFree; - void *newSpace; - - /* - * Ensure allocated space is adequately aligned. - */ - size = MAXALIGN(size); - - Assert(ShmemSegHdr != NULL); - - newStart = ShmemSegHdr->freeoffset; - - newFree = newStart + size; - if (newFree > ShmemSegHdr->totalsize) - ereport(ERROR, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of shared memory (%zu bytes requested)", - size))); - ShmemSegHdr->freeoffset = newFree; - - newSpace = (char *) ShmemBase + newStart; - - Assert(newSpace == (void *) MAXALIGN(newSpace)); - - return newSpace; -} - /* * ShmemAddrIsValid -- test if an address refers to shared memory * @@ -395,16 +386,14 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) if (!ShmemIndex) { - PGShmemHeader *shmemseghdr = ShmemSegHdr; - /* Must be trying to create/attach to ShmemIndex itself */ Assert(strcmp(name, "ShmemIndex") == 0); if (IsUnderPostmaster) { /* Must be initializing a (non-standalone) backend */ - Assert(shmemseghdr->index != NULL); - structPtr = shmemseghdr->index; + Assert(ShmemAllocator->index != NULL); + structPtr = ShmemAllocator->index; *foundPtr = true; } else @@ -417,9 +406,9 @@ ShmemInitStruct(const char *name, Size size, bool *foundPtr) * index has been initialized. This should be OK because no other * process can be accessing shared memory yet. */ - Assert(shmemseghdr->index == NULL); + Assert(ShmemAllocator->index == NULL); structPtr = ShmemAlloc(size); - shmemseghdr->index = structPtr; + ShmemAllocator->index = structPtr; *foundPtr = false; } LWLockRelease(ShmemIndexLock); @@ -553,15 +542,15 @@ pg_get_shmem_allocations(PG_FUNCTION_ARGS) /* output shared memory allocated but not counted via the shmem index */ values[0] = CStringGetTextDatum("<anonymous>"); nulls[1] = true; - values[2] = Int64GetDatum(ShmemSegHdr->freeoffset - named_allocated); + values[2] = Int64GetDatum(ShmemAllocator->freeoffset - named_allocated); values[3] = values[2]; tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls); /* output as-of-yet unused shared memory */ nulls[0] = true; - values[1] = Int64GetDatum(ShmemSegHdr->freeoffset); + values[1] = Int64GetDatum(ShmemAllocator->freeoffset); nulls[1] = false; - values[2] = Int64GetDatum(ShmemSegHdr->totalsize - ShmemSegHdr->freeoffset); + values[2] = Int64GetDatum(ShmemSegHdr->totalsize - ShmemAllocator->freeoffset); values[3] = values[2]; tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc, values, nulls); diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index 3aeada554b2..10c7b065861 100644 --- a/src/include/storage/pg_shmem.h +++ b/src/include/storage/pg_shmem.h @@ -32,9 +32,9 @@ typedef struct PGShmemHeader /* standard header for all Postgres shmem */ #define PGShmemMagic 679834894 pid_t creatorPID; /* PID of creating process (set but unread) */ Size totalsize; /* total size of segment */ - Size freeoffset; /* offset to first free space */ + Size content_offset; /* offset to the data, i.e. size of this + * header */ dsm_handle dsm_control; /* ID of dynamic shared memory control seg */ - void *index; /* pointer to ShmemIndex table */ #ifndef WIN32 /* Windows doesn't have useful inode#s */ dev_t device; /* device data directory is on */ ino_t inode; /* inode number of data directory */ diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h index e71a51dfe84..89d45287c17 100644 --- a/src/include/storage/shmem.h +++ b/src/include/storage/shmem.h @@ -29,8 +29,7 @@ extern PGDLLIMPORT slock_t *ShmemLock; typedef struct PGShmemHeader PGShmemHeader; /* avoid including * storage/pg_shmem.h here */ -extern void InitShmemAccess(PGShmemHeader *seghdr); -extern void InitShmemAllocation(void); +extern void InitShmemAllocator(PGShmemHeader *seghdr); extern void *ShmemAlloc(Size size); extern void *ShmemAllocNoError(Size size); extern bool ShmemAddrIsValid(const void *addr); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 34374df0d67..9f5ee8fd482 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -2804,6 +2804,7 @@ SharedTypmodTableEntry Sharedsort ShellTypeInfo ShippableCacheEntry +ShmemAllocatorData ShippableCacheKey ShmemIndexEnt ShutdownForeignScan_function base-commit: bb26a81ee28c9d9c64e6f233fafa2792768ece1b -- 2.34.1
