From e0dece127db86f40cdcf1fa63441eac93a1ba5ff Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 15 Oct 2006 22:04:08 +0000 Subject: [PATCH] Redesign the patch for allocation of shmem space and LWLocks for add-on modules; the first try was not usable in EXEC_BACKEND builds (e.g., Windows). Instead, just provide some entry points to increase the allocation requests during postmaster start, and provide a dedicated LWLock that can be used to synchronize allocation operations performed by backends. Per discussion with Marc Munro. --- src/backend/storage/ipc/ipci.c | 50 ++++++---- src/backend/storage/ipc/shmem.c | 155 +----------------------------- src/backend/storage/lmgr/lwlock.c | 103 ++++++-------------- src/include/storage/lwlock.h | 5 +- src/include/storage/pg_shmem.h | 9 +- src/include/storage/shmem.h | 10 +- 6 files changed, 73 insertions(+), 259 deletions(-) diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c index 716963f448..bf232eb8dc 100644 --- a/src/backend/storage/ipc/ipci.c +++ b/src/backend/storage/ipc/ipci.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/ipc/ipci.c,v 1.88 2006/10/04 00:29:57 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/ipc/ipci.c,v 1.89 2006/10/15 22:04:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -32,6 +32,30 @@ #include "storage/spin.h" +static Size total_addin_request = 0; +static bool addin_request_allowed = true; + + +/* + * RequestAddinShmemSpace + * Request that extra shmem space be allocated for use by + * a loadable module. + * + * This is only useful if called from the _PG_init hook of a library that + * is loaded into the postmaster via shared_preload_libraries. Once + * shared memory has been allocated, calls will be ignored. (We could + * raise an error, but it seems better to make it a no-op, so that + * libraries containing such calls can be reloaded if needed.) + */ +void +RequestAddinShmemSpace(Size size) +{ + if (IsUnderPostmaster || !addin_request_allowed) + return; /* too late */ + total_addin_request = add_size(total_addin_request, size); +} + + /* * CreateSharedMemoryAndSemaphores * Creates and initializes shared memory and semaphores. @@ -57,7 +81,6 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port) { PGShmemHeader *seghdr; Size size; - Size size_b4addins; int numSemas; /* @@ -91,16 +114,11 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port) size = add_size(size, ShmemBackendArraySize()); #endif - /* might as well round it off to a multiple of a typical page size */ - size = add_size(size, 8192 - (size % 8192)); + /* freeze the addin request size and include it */ + addin_request_allowed = false; + size = add_size(size, total_addin_request); - /* - * The shared memory for add-ins is treated as a separate segment, but - * in reality it is not. - */ - size_b4addins = size; - size = add_size(size, AddinShmemSize()); - /* round it off again */ + /* might as well round it off to a multiple of a typical page size */ size = add_size(size, 8192 - (size % 8192)); elog(DEBUG3, "invoking IpcMemoryCreate(size=%lu)", @@ -111,16 +129,6 @@ CreateSharedMemoryAndSemaphores(bool makePrivate, int port) */ seghdr = PGSharedMemoryCreate(size, makePrivate, port); - /* - * Modify hdr to show segment size before add-ins - */ - seghdr->totalsize = size_b4addins; - - /* - * Set up segment header sections in each Addin context - */ - InitAddinContexts((void *) ((char *) seghdr + size_b4addins)); - InitShmemAccess(seghdr); /* diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index 685912e157..b79f0f1191 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.97 2006/10/04 00:29:57 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/ipc/shmem.c,v 1.98 2006/10/15 22:04:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -53,7 +53,7 @@ * postmaster. However, this does not work in the EXEC_BACKEND case. * In ports using EXEC_BACKEND, new backends have to set up their local * pointers using the method described in (b) above. - + * * (d) memory allocation model: shared memory can never be * freed, once allocated. Each hash table has its own free list, * so hash buckets can be reused when an item is deleted. However, @@ -61,15 +61,6 @@ * cannot be redistributed to other tables. We could build a simple * hash bucket garbage collector if need be. Right now, it seems * unnecessary. - * - * (e) Add-ins can request their own logical shared memory segments - * by calling RegisterAddinContext() from the preload-libraries hook. - * Each call establishes a uniquely named add-in shared memopry - * context which will be set up as part of postgres intialisation. - * Memory can be allocated from these contexts using - * ShmemAllocFromContext(), and can be reset to its initial condition - * using ShmemResetContext(). Also, RegisterAddinLWLock(LWLockid *lock_ptr) - * can be used to request that a LWLock be allocated, placed into *lock_ptr. */ #include "postgres.h" @@ -95,19 +86,6 @@ slock_t *ShmemLock; /* spinlock for shared memory and LWLock static HTAB *ShmemIndex = NULL; /* primary index hashtable for shmem */ -/* Structures and globals for managing add-in shared memory contexts */ -typedef struct context -{ - char *name; - Size size; - PGShmemHeader *seg_hdr; - struct context *next; -} ContextNode; - -static ContextNode *addin_contexts = NULL; -static Size addin_contexts_size = 0; - - /* * InitShmemAccess() --- set up basic pointers to shared memory. @@ -162,99 +140,7 @@ InitShmemAllocation(void) } /* - * RegisterAddinContext -- Register the requirement for a named shared - * memory context. - */ -void -RegisterAddinContext(const char *name, Size size) -{ - char *newstr = malloc(strlen(name) + 1); - ContextNode *node = malloc(sizeof(ContextNode)); - - strcpy(newstr, name); - node->name = newstr; - - /* Round up to typical page size */ - node->size = add_size(size, 8192 - (size % 8192)); - node->next = addin_contexts; - - addin_contexts = node; - addin_contexts_size = add_size(addin_contexts_size, node->size); -} - - -/* - * ContextFromName -- Return the ContextNode for the given named - * context, or NULL if not found. - */ -static ContextNode * -ContextFromName(const char *name) -{ - ContextNode *context = addin_contexts; - - while (context) - { - if (strcmp(name, context->name) == 0) - return context; - context = context->next; - } - return NULL; -} - -/* - * InitAddinContexts -- Initialise the registered addin shared memory - * contexts. - */ -void -InitAddinContexts(void *start) -{ - PGShmemHeader *next_segment = (PGShmemHeader *) start; - ContextNode *context = addin_contexts; - - while (context) - { - context->seg_hdr = next_segment; - - next_segment->totalsize = context->size; - next_segment->freeoffset = MAXALIGN(sizeof(PGShmemHeader)); - - next_segment = (PGShmemHeader *) - ((char *) next_segment + context->size); - context = context->next; - } -} - -/* - * ShmemResetContext -- Re-initialise the named addin shared memory context. - */ -void -ShmemResetContext(const char *name) -{ - PGShmemHeader *segment; - ContextNode *context = ContextFromName(name); - - if (!context) - ereport(ERROR, - (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("cannot reset unknown shared memory context %s", - name))); - - segment = context->seg_hdr; - segment->freeoffset = MAXALIGN(sizeof(PGShmemHeader)); -} - -/* - * AddinShmemSize -- Report how much shared memory has been registered - * for add-ins. - */ -Size -AddinShmemSize(void) -{ - return addin_contexts_size; -} - -/* - * ShmemAllocFromContext -- allocate max-aligned chunk from shared memory + * ShmemAlloc -- allocate max-aligned chunk from shared memory * * Assumes ShmemLock and ShmemSegHdr are initialized. * @@ -263,30 +149,15 @@ AddinShmemSize(void) * to be compatible with malloc(). */ void * -ShmemAllocFromContext(Size size, const char *context_name) +ShmemAlloc(Size size) { Size newStart; Size newFree; void *newSpace; - ContextNode *context; /* use volatile pointer to prevent code rearrangement */ volatile PGShmemHeader *shmemseghdr = ShmemSegHdr; - /* - * if context_name is provided, allocate from the named context - */ - if (context_name) - { - context = ContextFromName(context_name); - if (!context) - ereport(ERROR, - (errcode(ERRCODE_INTERNAL_ERROR), - errmsg("cannot reset unknown shared memory context %s", - context_name))); - shmemseghdr = context->seg_hdr; - } - /* * ensure all space is adequately aligned. */ @@ -305,7 +176,7 @@ ShmemAllocFromContext(Size size, const char *context_name) newFree = newStart + size; if (newFree <= shmemseghdr->totalsize) { - newSpace = (void *) MAKE_PTRFROM((SHMEM_OFFSET) shmemseghdr, newStart); + newSpace = (void *) MAKE_PTR(newStart); shmemseghdr->freeoffset = newFree; } else @@ -321,22 +192,6 @@ ShmemAllocFromContext(Size size, const char *context_name) return newSpace; } -/* - * ShmemAlloc -- allocate max-aligned chunk from shared memory - * - * Assumes ShmemLock and ShmemSegHdr are initialized. - * - * Returns: real pointer to memory or NULL if we are out - * of space. Has to return a real pointer in order - * to be compatible with malloc(). - */ - -void * -ShmemAlloc(Size size) -{ - return ShmemAllocFromContext(size, NULL); -} - /* * ShmemIsValid -- test if an offset refers to valid shared memory * diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index c5b0d2af4d..51770e1c6d 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -15,7 +15,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/lmgr/lwlock.c,v 1.46 2006/10/04 00:29:57 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/storage/lmgr/lwlock.c,v 1.47 2006/10/15 22:04:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -30,10 +30,6 @@ #include "storage/spin.h" -static int NumAddinLWLocks(void); -static void AssignAddinLWLocks(void); - - /* We use the ShmemLock spinlock to protect LWLockAssign */ extern slock_t *ShmemLock; @@ -88,6 +84,9 @@ NON_EXEC_STATIC LWLockPadded *LWLockArray = NULL; static int num_held_lwlocks = 0; static LWLockId held_lwlocks[MAX_SIMUL_LWLOCKS]; +static int lock_addin_request = 0; +static bool lock_addin_request_allowed = true; + #ifdef LWLOCK_STATS static int counts_for_pid = 0; static int *sh_acquire_counts; @@ -95,63 +94,6 @@ static int *ex_acquire_counts; static int *block_counts; #endif -/* - * Structures and globals to allow add-ins to register for their own - * lwlocks from the preload-libraries hook. - */ -typedef struct LWLockNode -{ - LWLockId *lock; - struct LWLockNode *next; -} LWLockNode; - -static LWLockNode *addin_locks = NULL; -static int num_addin_locks = 0; - - -/* - * RegisterAddinLWLock() --- Allow an andd-in to request a LWLock - * from the preload-libraries hook. - */ -void -RegisterAddinLWLock(LWLockId *lock) -{ - LWLockNode *locknode = malloc(sizeof(LWLockNode)); - - locknode->next = addin_locks; - locknode->lock = lock; - - addin_locks = locknode; - num_addin_locks++; -} - -/* - * NumAddinLWLocks() --- Return the number of LWLocks requested by add-ins. - */ -static int -NumAddinLWLocks() -{ - return num_addin_locks; -} - -/* - * AssignAddinLWLocks() --- Assign LWLocks previously requested by add-ins. - */ -static void -AssignAddinLWLocks() -{ - LWLockNode *node = addin_locks; - - while (node) - { - *(node->lock) = LWLockAssign(); - node = node->next; - } -} - - - - #ifdef LOCK_DEBUG bool Trace_lwlocks = false; @@ -231,16 +173,38 @@ NumLWLocks(void) /* multixact.c needs two SLRU areas */ numLocks += NUM_MXACTOFFSET_BUFFERS + NUM_MXACTMEMBER_BUFFERS; - /* Leave a few extra for use by user-defined modules. */ - numLocks += NUM_USER_DEFINED_LWLOCKS; - - /* Add the number that have been explicitly requested by add-ins. */ - numLocks += NumAddinLWLocks(); + /* + * Add any requested by loadable modules; for backwards-compatibility + * reasons, allocate at least NUM_USER_DEFINED_LWLOCKS of them even + * if there are no explicit requests. + */ + lock_addin_request_allowed = false; + numLocks += Max(lock_addin_request, NUM_USER_DEFINED_LWLOCKS); return numLocks; } +/* + * RequestAddinLWLocks + * Request that extra LWLocks be allocated for use by + * a loadable module. + * + * This is only useful if called from the _PG_init hook of a library that + * is loaded into the postmaster via shared_preload_libraries. Once + * shared memory has been allocated, calls will be ignored. (We could + * raise an error, but it seems better to make it a no-op, so that + * libraries containing such calls can be reloaded if needed.) + */ +void +RequestAddinLWLocks(int n) +{ + if (IsUnderPostmaster || !lock_addin_request_allowed) + return; /* too late */ + lock_addin_request += n; +} + + /* * Compute shmem space needed for LWLocks. */ @@ -304,11 +268,6 @@ CreateLWLocks(void) LWLockCounter = (int *) ((char *) LWLockArray - 2 * sizeof(int)); LWLockCounter[0] = (int) NumFixedLWLocks; LWLockCounter[1] = numLocks; - - /* - * Allocate LWLocks for those add-ins that have explicitly requested them. - */ - AssignAddinLWLocks(); } diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 5c4db52892..47025fd2d6 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/lwlock.h,v 1.31 2006/08/01 19:03:11 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/lwlock.h,v 1.32 2006/10/15 22:04:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -60,6 +60,7 @@ typedef enum LWLockId TwoPhaseStateLock, TablespaceCreateLock, BtreeVacuumLock, + AddinShmemInitLock, FirstBufMappingLock, FirstLockMgrLock = FirstBufMappingLock + NUM_BUFFER_PARTITIONS, @@ -92,6 +93,6 @@ extern int NumLWLocks(void); extern Size LWLockShmemSize(void); extern void CreateLWLocks(void); -extern void RegisterAddinLWLock(LWLockId *lock); +extern void RequestAddinLWLocks(int n); #endif /* LWLOCK_H */ diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index 52de6a9404..c4e01090ad 100644 --- a/src/include/storage/pg_shmem.h +++ b/src/include/storage/pg_shmem.h @@ -17,7 +17,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.20 2006/10/04 00:30:10 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.21 2006/10/15 22:04:07 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -51,11 +51,4 @@ extern PGShmemHeader *PGSharedMemoryCreate(Size size, bool makePrivate, extern bool PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2); extern void PGSharedMemoryDetach(void); - -extern void RegisterAddinContext(const char *name, Size size); -extern Size AddinShmemSize(void); -extern void InitAddinContexts(void *start); -extern void *ShmemAllocFromContext(Size size, const char *name); -extern void ShmemResetContext(const char *name); - #endif /* PG_SHMEM_H */ diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h index bd55fbdb5c..ff6b0b7c21 100644 --- a/src/include/storage/shmem.h +++ b/src/include/storage/shmem.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/storage/shmem.h,v 1.48 2006/08/01 19:03:11 momjian Exp $ + * $PostgreSQL: pgsql/src/include/storage/shmem.h,v 1.49 2006/10/15 22:04:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,11 +38,6 @@ typedef unsigned long SHMEM_OFFSET; extern DLLIMPORT SHMEM_OFFSET ShmemBase; -/* coerce an offset into a pointer in a specified address space. This - * macro (only) is not confined to the primary shared memory region */ -#define MAKE_PTRFROM(base,xx_offs)\ - (base+((unsigned long)(xx_offs))) - /* coerce an offset into a pointer in this process's address space */ #define MAKE_PTR(xx_offs)\ (ShmemBase+((unsigned long)(xx_offs))) @@ -77,6 +72,9 @@ extern void *ShmemInitStruct(const char *name, Size size, bool *foundPtr); extern Size add_size(Size s1, Size s2); extern Size mul_size(Size s1, Size s2); +/* ipci.c */ +extern void RequestAddinShmemSpace(Size size); + /* size constants for the shmem index table */ /* max size of data structure string name */ #define SHMEM_INDEX_KEYSIZE (48)