I've spent some time getting this prepared for commit, so apologies if I am steamrolling over some of the latest discussion points. The majority of what I've changed amounts to what I'd characterize as relatively minor editorialization, but I'd imagine reasonable people could disagree. The bigger things I've changed include:
* I've moved the DSM registry adjustments to be a prerequisite patch. I noticed that we can also stop tracking the tranche names there since we always use the name of the DSM registry entry. * I modified the array of tranche names to be a char ** to simplify lookups. I also changed how it is first initialized in CreateLWLocks() a bit. * I've left out the tests for now. Those are great for the development phase, but I'm not completely sold on committing them. In any case, I'd probably treat that as a follow-up effort once this stuff is committed. I think this patch set will require reworking the "GetNamedLWLockTranche crashes on Windows in normal backend" patch [0], but AFAICT we can easily adjust it to scan through NamedLWLockTrancheNames instead. Overall, I'm pretty happy about these patches. They simplify the API and the code while also fixing the problem with tranche name visibility. [0] https://postgr.es/m/CAA5RZ0v1_15QPg5Sqd2Qz5rh_qcsyCeHHmRDY89xVHcy2yt5BQ%40mail.gmail.com -- nathan
>From 484ebb5cc6ae4300aea654b0f148889d63367256 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Wed, 27 Aug 2025 10:28:26 -0500 Subject: [PATCH v16 1/2] dsm_registry: Use one LWLock tranche for dshash table entries. XXX: Also use NAMEDATALEN for entry names. --- src/backend/storage/ipc/dsm_registry.c | 56 +++++++++----------------- 1 file changed, 18 insertions(+), 38 deletions(-) diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index 1682cc6d34c..332796465ff 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -48,12 +48,6 @@ #include "utils/builtins.h" #include "utils/memutils.h" -#define DSMR_NAME_LEN 128 - -#define DSMR_DSA_TRANCHE_SUFFIX " DSA" -#define DSMR_DSA_TRANCHE_SUFFIX_LEN (sizeof(DSMR_DSA_TRANCHE_SUFFIX) - 1) -#define DSMR_DSA_TRANCHE_NAME_LEN (DSMR_NAME_LEN + DSMR_DSA_TRANCHE_SUFFIX_LEN) - typedef struct DSMRegistryCtxStruct { dsa_handle dsah; @@ -72,15 +66,13 @@ typedef struct NamedDSAState { dsa_handle handle; int tranche; - char tranche_name[DSMR_DSA_TRANCHE_NAME_LEN]; } NamedDSAState; typedef struct NamedDSHState { - NamedDSAState dsa; - dshash_table_handle handle; + dsa_handle dsa_handle; + dshash_table_handle dsh_handle; int tranche; - char tranche_name[DSMR_NAME_LEN]; } NamedDSHState; typedef enum DSMREntryType @@ -99,7 +91,7 @@ static const char *const DSMREntryTypeNames[] = typedef struct DSMRegistryEntry { - char name[DSMR_NAME_LEN]; + char name[NAMEDATALEN]; DSMREntryType type; union { @@ -308,8 +300,7 @@ GetNamedDSA(const char *name, bool *found) /* Initialize the LWLock tranche for the DSA. */ state->tranche = LWLockNewTrancheId(); - strcpy(state->tranche_name, name); - LWLockRegisterTranche(state->tranche, state->tranche_name); + LWLockRegisterTranche(state->tranche, name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -331,7 +322,7 @@ GetNamedDSA(const char *name, bool *found) (errmsg("requested DSA already attached to current process"))); /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, state->tranche_name); + LWLockRegisterTranche(state->tranche, name); /* Attach to existing DSA. */ ret = dsa_attach(state->handle); @@ -348,11 +339,9 @@ GetNamedDSA(const char *name, bool *found) * Initialize or attach a named dshash table. * * This routine returns the address of the table. The tranche_id member of - * params is ignored; new tranche IDs will be generated if needed. Note that - * the DSA lock tranche will be registered with the provided name with " DSA" - * appended. The dshash lock tranche will be registered with the provided - * name. Also note that this should be called at most once for a given table - * in each backend. + * params is ignored; a new tranche ID will be generated if needed. Note that + * the lock tranche will be registered with the provided name. Also note that + * this should be called at most once for a given table in each backend. */ dshash_table * GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) @@ -381,25 +370,18 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry = dshash_find_or_insert(dsm_registry_table, name, found); if (!(*found)) { - NamedDSAState *dsa_state = &entry->data.dsh.dsa; NamedDSHState *dsh_state = &entry->data.dsh; dshash_parameters params_copy; dsa_area *dsa; entry->type = DSMR_ENTRY_TYPE_DSH; - /* Initialize the LWLock tranche for the DSA. */ - dsa_state->tranche = LWLockNewTrancheId(); - sprintf(dsa_state->tranche_name, "%s%s", name, DSMR_DSA_TRANCHE_SUFFIX); - LWLockRegisterTranche(dsa_state->tranche, dsa_state->tranche_name); - - /* Initialize the LWLock tranche for the dshash table. */ + /* Initialize the LWLock tranche for the hash table. */ dsh_state->tranche = LWLockNewTrancheId(); - strcpy(dsh_state->tranche_name, name); - LWLockRegisterTranche(dsh_state->tranche, dsh_state->tranche_name); + LWLockRegisterTranche(dsh_state->tranche, name); /* Initialize the DSA for the hash table. */ - dsa = dsa_create(dsa_state->tranche); + dsa = dsa_create(dsh_state->tranche); dsa_pin(dsa); dsa_pin_mapping(dsa); @@ -409,34 +391,32 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ret = dshash_create(dsa, ¶ms_copy, NULL); /* Store handles for other backends to use. */ - dsa_state->handle = dsa_get_handle(dsa); - dsh_state->handle = dshash_get_hash_table_handle(ret); + dsh_state->dsa_handle = dsa_get_handle(dsa); + dsh_state->dsh_handle = dshash_get_hash_table_handle(ret); } else if (entry->type != DSMR_ENTRY_TYPE_DSH) ereport(ERROR, (errmsg("requested DSHash does not match type of existing entry"))); else { - NamedDSAState *dsa_state = &entry->data.dsh.dsa; NamedDSHState *dsh_state = &entry->data.dsh; dsa_area *dsa; /* XXX: Should we verify params matches what table was created with? */ - if (dsa_is_attached(dsa_state->handle)) + if (dsa_is_attached(dsh_state->dsa_handle)) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranches for the DSA and dshash table. */ - LWLockRegisterTranche(dsa_state->tranche, dsa_state->tranche_name); - LWLockRegisterTranche(dsh_state->tranche, dsh_state->tranche_name); + /* Initialize existing LWLock tranche for the hash table. */ + LWLockRegisterTranche(dsh_state->tranche, name); /* Attach to existing DSA for the hash table. */ - dsa = dsa_attach(dsa_state->handle); + dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); /* Attach to existing dshash table. */ - ret = dshash_attach(dsa, params, dsh_state->handle, NULL); + ret = dshash_attach(dsa, params, dsh_state->dsh_handle, NULL); } dshash_release_lock(dsm_registry_table, entry); -- 2.39.5 (Apple Git-154)
>From 6ad1dac7633e4701bdfecc6885b3553ee33a40ab Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Wed, 27 Aug 2025 15:13:41 -0500 Subject: [PATCH v16 2/2] Move dynamically-allocated tranche names to shared memory. --- contrib/pg_prewarm/autoprewarm.c | 3 +- doc/src/sgml/xfunc.sgml | 15 +- src/backend/postmaster/launch_backend.c | 6 +- src/backend/storage/ipc/dsm_registry.c | 12 +- src/backend/storage/lmgr/lwlock.c | 179 ++++++++---------- src/include/storage/lwlock.h | 24 +-- src/test/modules/test_dsa/test_dsa.c | 6 +- .../test_dsm_registry/test_dsm_registry.c | 3 +- .../modules/test_radixtree/test_radixtree.c | 9 +- src/test/modules/test_slru/test_slru.c | 6 +- .../modules/test_tidstore/test_tidstore.c | 3 +- 11 files changed, 99 insertions(+), 167 deletions(-) diff --git a/contrib/pg_prewarm/autoprewarm.c b/contrib/pg_prewarm/autoprewarm.c index c01b9c7e6a4..880e897796a 100644 --- a/contrib/pg_prewarm/autoprewarm.c +++ b/contrib/pg_prewarm/autoprewarm.c @@ -864,7 +864,7 @@ apw_init_state(void *ptr) { AutoPrewarmSharedState *state = (AutoPrewarmSharedState *) ptr; - LWLockInitialize(&state->lock, LWLockNewTrancheId()); + LWLockInitialize(&state->lock, LWLockNewTrancheId("autoprewarm")); state->bgworker_pid = InvalidPid; state->pid_using_dumpfile = InvalidPid; } @@ -883,7 +883,6 @@ apw_init_shmem(void) sizeof(AutoPrewarmSharedState), apw_init_state, &found); - LWLockRegisterTranche(apw_state->lock.tranche, "autoprewarm"); return found; } diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index f116d0648e5..da21ef56891 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -3759,7 +3759,7 @@ LWLockPadded *GetNamedLWLockTranche(const char *tranche_name) <literal>shmem_request_hook</literal>. To do so, first allocate a <literal>tranche_id</literal> by calling: <programlisting> -int LWLockNewTrancheId(void) +int LWLockNewTrancheId(const char *name) </programlisting> Next, initialize each LWLock, passing the new <literal>tranche_id</literal> as an argument: @@ -3777,17 +3777,8 @@ void LWLockInitialize(LWLock *lock, int tranche_id) </para> <para> - Finally, each backend using the <literal>tranche_id</literal> should - associate it with a <literal>tranche_name</literal> by calling: -<programlisting> -void LWLockRegisterTranche(int tranche_id, const char *tranche_name) -</programlisting> - </para> - - <para> - A complete usage example of <function>LWLockNewTrancheId</function>, - <function>LWLockInitialize</function>, and - <function>LWLockRegisterTranche</function> can be found in + A complete usage example of <function>LWLockNewTrancheId</function> and + <function>LWLockInitialize</function> can be found in <filename>contrib/pg_prewarm/autoprewarm.c</filename> in the <productname>PostgreSQL</productname> source tree. </para> diff --git a/src/backend/postmaster/launch_backend.c b/src/backend/postmaster/launch_backend.c index bf6b55ee830..cbb44344b5a 100644 --- a/src/backend/postmaster/launch_backend.c +++ b/src/backend/postmaster/launch_backend.c @@ -101,7 +101,7 @@ typedef struct struct InjectionPointsCtl *ActiveInjectionPoints; #endif int NamedLWLockTrancheRequests; - NamedLWLockTranche *NamedLWLockTrancheArray; + char **NamedLWLockTrancheNames; LWLockPadded *MainLWLockArray; slock_t *ProcStructLock; PROC_HDR *ProcGlobal; @@ -760,7 +760,7 @@ save_backend_variables(BackendParameters *param, #endif param->NamedLWLockTrancheRequests = NamedLWLockTrancheRequests; - param->NamedLWLockTrancheArray = NamedLWLockTrancheArray; + param->NamedLWLockTrancheNames = NamedLWLockTrancheNames; param->MainLWLockArray = MainLWLockArray; param->ProcStructLock = ProcStructLock; param->ProcGlobal = ProcGlobal; @@ -1020,7 +1020,7 @@ restore_backend_variables(BackendParameters *param) #endif NamedLWLockTrancheRequests = param->NamedLWLockTrancheRequests; - NamedLWLockTrancheArray = param->NamedLWLockTrancheArray; + NamedLWLockTrancheNames = param->NamedLWLockTrancheNames; MainLWLockArray = param->MainLWLockArray; ProcStructLock = param->ProcStructLock; ProcGlobal = param->ProcGlobal; diff --git a/src/backend/storage/ipc/dsm_registry.c b/src/backend/storage/ipc/dsm_registry.c index 332796465ff..8999dee6761 100644 --- a/src/backend/storage/ipc/dsm_registry.c +++ b/src/backend/storage/ipc/dsm_registry.c @@ -299,8 +299,7 @@ GetNamedDSA(const char *name, bool *found) entry->type = DSMR_ENTRY_TYPE_DSA; /* Initialize the LWLock tranche for the DSA. */ - state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(state->tranche, name); + state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA. */ ret = dsa_create(state->tranche); @@ -321,9 +320,6 @@ GetNamedDSA(const char *name, bool *found) ereport(ERROR, (errmsg("requested DSA already attached to current process"))); - /* Initialize existing LWLock tranche for the DSA. */ - LWLockRegisterTranche(state->tranche, name); - /* Attach to existing DSA. */ ret = dsa_attach(state->handle); dsa_pin_mapping(ret); @@ -377,8 +373,7 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) entry->type = DSMR_ENTRY_TYPE_DSH; /* Initialize the LWLock tranche for the hash table. */ - dsh_state->tranche = LWLockNewTrancheId(); - LWLockRegisterTranche(dsh_state->tranche, name); + dsh_state->tranche = LWLockNewTrancheId(name); /* Initialize the DSA for the hash table. */ dsa = dsa_create(dsh_state->tranche); @@ -408,9 +403,6 @@ GetNamedDSHash(const char *name, const dshash_parameters *params, bool *found) ereport(ERROR, (errmsg("requested DSHash already attached to current process"))); - /* Initialize existing LWLock tranche for the hash table. */ - LWLockRegisterTranche(dsh_state->tranche, name); - /* Attach to existing DSA for the hash table. */ dsa = dsa_attach(dsh_state->dsa_handle); dsa_pin_mapping(dsa); diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index c80b43f1f55..39f17c387f3 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -126,8 +126,8 @@ StaticAssertDecl((LW_VAL_EXCLUSIVE & LW_FLAG_MASK) == 0, * in lwlocklist.h. We absorb the names of these tranches, too. * * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche - * or LWLockRegisterTranche. The names of these that are known in the current - * process appear in LWLockTrancheNames[]. + * or LWLockNewTrancheId. These names are stored in shared memory and can be + * accessed via NamedLWLockTrancheNames. * * All these names are user-visible as wait event names, so choose with care * ... and do not forget to update the documentation's list of wait events. @@ -146,11 +146,12 @@ StaticAssertDecl(lengthof(BuiltinTrancheNames) == /* * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and - * stores the names of all dynamically-created tranches known to the current - * process. Any unused entries in the array will contain NULL. + * points to the shared memory locations of the names of all + * dynamically-created tranches. Backends inherit the pointer by fork from the + * postmaster (except in the EXEC_BACKEND case, where we have special measures + * to pass it down). */ -static const char **LWLockTrancheNames = NULL; -static int LWLockTrancheNamesAllocated = 0; +char **NamedLWLockTrancheNames = NULL; /* * This points to the main array of LWLocks in shared memory. Backends inherit @@ -184,18 +185,18 @@ typedef struct NamedLWLockTrancheRequest } NamedLWLockTrancheRequest; static NamedLWLockTrancheRequest *NamedLWLockTrancheRequestArray = NULL; -static int NamedLWLockTrancheRequestsAllocated = 0; /* - * NamedLWLockTrancheRequests is both the valid length of the request array, - * and the length of the shared-memory NamedLWLockTrancheArray later on. - * This variable and NamedLWLockTrancheArray are non-static so that - * postmaster.c can copy them to child processes in EXEC_BACKEND builds. + * NamedLWLockTrancheRequests is the valid length of the request array. This + * variable is non-static so that postmaster.c can copy them to child processes + * in EXEC_BACKEND builds. */ int NamedLWLockTrancheRequests = 0; -/* points to data in shared memory: */ -NamedLWLockTranche *NamedLWLockTrancheArray = NULL; +/* backend-local counter of registered tranches */ +static int LocalLWLockCounter; + +#define MAX_NAMED_TRANCHES 256 static void InitializeLWLocks(void); static inline void LWLockReportWaitStart(LWLock *lock); @@ -391,7 +392,6 @@ Size LWLockShmemSize(void) { Size size; - int i; int numLocks = NUM_FIXED_LWLOCKS; /* Calculate total number of locks needed in the main array. */ @@ -404,18 +404,15 @@ LWLockShmemSize(void) size = add_size(size, sizeof(int) + LWLOCK_PADDED_SIZE); /* space for named tranches. */ - size = add_size(size, mul_size(NamedLWLockTrancheRequests, sizeof(NamedLWLockTranche))); - - /* space for name of each tranche. */ - for (i = 0; i < NamedLWLockTrancheRequests; i++) - size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, sizeof(char *))); + size = add_size(size, mul_size(MAX_NAMED_TRANCHES, NAMEDATALEN)); return size; } /* * Allocate shmem space for the main LWLock array and all tranches and - * initialize it. We also register extension LWLock tranches here. + * initialize it. */ void CreateLWLocks(void) @@ -429,6 +426,15 @@ CreateLWLocks(void) /* Allocate space */ ptr = (char *) ShmemAlloc(spaceLocks); + /* Initialize tranche names */ + NamedLWLockTrancheNames = (char **) ptr; + ptr += MAX_NAMED_TRANCHES * sizeof(char *); + for (int i = 0; i < MAX_NAMED_TRANCHES; i++) + { + NamedLWLockTrancheNames[i] = ptr; + ptr += NAMEDATALEN; + } + /* Leave room for dynamic allocation of tranches */ ptr += sizeof(int); @@ -447,11 +453,6 @@ CreateLWLocks(void) /* Initialize all LWLocks */ InitializeLWLocks(); } - - /* Register named extension LWLock tranches in the current process. */ - for (int i = 0; i < NamedLWLockTrancheRequests; i++) - LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId, - NamedLWLockTrancheArray[i].trancheName); } /* @@ -460,7 +461,6 @@ CreateLWLocks(void) static void InitializeLWLocks(void) { - int numNamedLocks = NumLWLocksForNamedTranches(); int id; int i; int j; @@ -491,32 +491,18 @@ InitializeLWLocks(void) */ if (NamedLWLockTrancheRequests > 0) { - char *trancheNames; - - NamedLWLockTrancheArray = (NamedLWLockTranche *) - &MainLWLockArray[NUM_FIXED_LWLOCKS + numNamedLocks]; - - trancheNames = (char *) NamedLWLockTrancheArray + - (NamedLWLockTrancheRequests * sizeof(NamedLWLockTranche)); lock = &MainLWLockArray[NUM_FIXED_LWLOCKS]; for (i = 0; i < NamedLWLockTrancheRequests; i++) { NamedLWLockTrancheRequest *request; - NamedLWLockTranche *tranche; - char *name; + int tranche; request = &NamedLWLockTrancheRequestArray[i]; - tranche = &NamedLWLockTrancheArray[i]; - - name = trancheNames; - trancheNames += strlen(request->tranche_name) + 1; - strcpy(name, request->tranche_name); - tranche->trancheId = LWLockNewTrancheId(); - tranche->trancheName = name; + tranche = LWLockNewTrancheId(request->tranche_name); for (j = 0; j < request->num_lwlocks; j++, lock++) - LWLockInitialize(&lock->lock, tranche->trancheId); + LWLockInitialize(&lock->lock, tranche); } } } @@ -568,61 +554,41 @@ GetNamedLWLockTranche(const char *tranche_name) } /* - * Allocate a new tranche ID. + * Allocate a new tranche ID with the provided name. */ int -LWLockNewTrancheId(void) +LWLockNewTrancheId(const char *name) { int result; int *LWLockCounter; + if (strlen(name) >= NAMEDATALEN) + ereport(ERROR, + (errcode(ERRCODE_NAME_TOO_LONG), + errmsg("tranche name too long"), + errdetail("LWLock tranche name must be less than %d bytes.", + NAMEDATALEN))); + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + /* We use the ShmemLock spinlock to protect LWLockCounter */ SpinLockAcquire(ShmemLock); - result = (*LWLockCounter)++; - SpinLockRelease(ShmemLock); - - return result; -} - -/* - * Register a dynamic tranche name in the lookup table of the current process. - * - * This routine will save a pointer to the tranche name passed as an argument, - * so the name should be allocated in a backend-lifetime context - * (shared memory, TopMemoryContext, static constant, or similar). - * - * The tranche name will be user-visible as a wait event name, so try to - * use a name that fits the style for those. - */ -void -LWLockRegisterTranche(int tranche_id, const char *tranche_name) -{ - /* This should only be called for user-defined tranches. */ - if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED) - return; - /* Convert to array index. */ - tranche_id -= LWTRANCHE_FIRST_USER_DEFINED; - - /* If necessary, create or enlarge array. */ - if (tranche_id >= LWLockTrancheNamesAllocated) + if (*LWLockCounter - LWTRANCHE_FIRST_USER_DEFINED >= MAX_NAMED_TRANCHES) { - int newalloc; + SpinLockRelease(ShmemLock); + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); + } - newalloc = pg_nextpower2_32(Max(8, tranche_id + 1)); + result = LocalLWLockCounter = (*LWLockCounter)++; + strcpy(NamedLWLockTrancheNames[result - LWTRANCHE_FIRST_USER_DEFINED], name); - if (LWLockTrancheNames == NULL) - LWLockTrancheNames = (const char **) - MemoryContextAllocZero(TopMemoryContext, - newalloc * sizeof(char *)); - else - LWLockTrancheNames = - repalloc0_array(LWLockTrancheNames, const char *, LWLockTrancheNamesAllocated, newalloc); - LWLockTrancheNamesAllocated = newalloc; - } + SpinLockRelease(ShmemLock); - LWLockTrancheNames[tranche_id] = tranche_name; + return result; } /* @@ -647,22 +613,17 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) if (NamedLWLockTrancheRequestArray == NULL) { - NamedLWLockTrancheRequestsAllocated = 16; NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) MemoryContextAlloc(TopMemoryContext, - NamedLWLockTrancheRequestsAllocated + MAX_NAMED_TRANCHES * sizeof(NamedLWLockTrancheRequest)); } - if (NamedLWLockTrancheRequests >= NamedLWLockTrancheRequestsAllocated) - { - int i = pg_nextpower2_32(NamedLWLockTrancheRequests + 1); - - NamedLWLockTrancheRequestArray = (NamedLWLockTrancheRequest *) - repalloc(NamedLWLockTrancheRequestArray, - i * sizeof(NamedLWLockTrancheRequest)); - NamedLWLockTrancheRequestsAllocated = i; - } + if (NamedLWLockTrancheRequests >= MAX_NAMED_TRANCHES) + ereport(ERROR, + (errmsg("maximum number of tranches already registered"), + errdetail("At most %d tranches may be registered.", + MAX_NAMED_TRANCHES))); request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests]; Assert(strlen(tranche_name) + 1 <= NAMEDATALEN); @@ -677,6 +638,9 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks) void LWLockInitialize(LWLock *lock, int tranche_id) { + /* verify the tranche_id is valid */ + (void) GetLWTrancheName(tranche_id); + pg_atomic_init_u32(&lock->state, LW_FLAG_RELEASE_OK); #ifdef LOCK_DEBUG pg_atomic_init_u32(&lock->nwaiters, 0); @@ -717,18 +681,27 @@ GetLWTrancheName(uint16 trancheId) if (trancheId < LWTRANCHE_FIRST_USER_DEFINED) return BuiltinTrancheNames[trancheId]; + /* verify the trancheId is valid */ + if (trancheId >= LocalLWLockCounter) + { + int *LWLockCounter; + + LWLockCounter = (int *) ((char *) MainLWLockArray - sizeof(int)); + + SpinLockAcquire(ShmemLock); + LocalLWLockCounter = *LWLockCounter; + SpinLockRelease(ShmemLock); + + if (trancheId >= LocalLWLockCounter) + elog(ERROR, "tranche %d is not registered", trancheId); + } + /* - * It's an extension tranche, so look in LWLockTrancheNames[]. However, - * it's possible that the tranche has never been registered in the current - * process, in which case give up and return "extension". + * It's an extension tranche, so look in NamedLWLockTrancheNames. */ trancheId -= LWTRANCHE_FIRST_USER_DEFINED; - if (trancheId >= LWLockTrancheNamesAllocated || - LWLockTrancheNames[trancheId] == NULL) - return "extension"; - - return LWLockTrancheNames[trancheId]; + return NamedLWLockTrancheNames[trancheId]; } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 5e717765764..9f9c4c7b5ca 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -73,14 +73,7 @@ typedef union LWLockPadded extern PGDLLIMPORT LWLockPadded *MainLWLockArray; -/* struct for storing named tranche information */ -typedef struct NamedLWLockTranche -{ - int trancheId; - char *trancheName; -} NamedLWLockTranche; - -extern PGDLLIMPORT NamedLWLockTranche *NamedLWLockTrancheArray; +extern PGDLLIMPORT char **NamedLWLockTrancheNames; extern PGDLLIMPORT int NamedLWLockTrancheRequests; /* @@ -157,18 +150,11 @@ extern LWLockPadded *GetNamedLWLockTranche(const char *tranche_name); /* * There is another, more flexible method of obtaining lwlocks. First, call - * LWLockNewTrancheId just once to obtain a tranche ID; this allocates from - * a shared counter. Next, each individual process using the tranche should - * call LWLockRegisterTranche() to associate that tranche ID with a name. - * Finally, LWLockInitialize should be called just once per lwlock, passing - * the tranche ID as an argument. - * - * It may seem strange that each process using the tranche must register it - * separately, but dynamic shared memory segments aren't guaranteed to be - * mapped at the same address in all coordinating backends, so storing the - * registration in the main shared memory segment wouldn't work for that case. + * LWLockNewTrancheId to obtain a tranche ID; this allocates from a shared + * counter. Second, LWLockInitialize should be called just once per lwlock, + * passing the tranche ID as an argument. */ -extern int LWLockNewTrancheId(void); +extern int LWLockNewTrancheId(const char *name); extern void LWLockRegisterTranche(int tranche_id, const char *tranche_name); extern void LWLockInitialize(LWLock *lock, int tranche_id); diff --git a/src/test/modules/test_dsa/test_dsa.c b/src/test/modules/test_dsa/test_dsa.c index cd24d0f4873..01d5c6fa67f 100644 --- a/src/test/modules/test_dsa/test_dsa.c +++ b/src/test/modules/test_dsa/test_dsa.c @@ -29,8 +29,7 @@ test_dsa_basic(PG_FUNCTION_ARGS) dsa_pointer p[100]; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); a = dsa_create(tranche_id); for (int i = 0; i < 100; i++) @@ -70,8 +69,7 @@ test_dsa_resowners(PG_FUNCTION_ARGS) ResourceOwner childowner; /* XXX: this tranche is leaked */ - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_dsa"); + tranche_id = LWLockNewTrancheId("test_dsa"); /* Create DSA in parent resource owner */ a = dsa_create(tranche_id); diff --git a/src/test/modules/test_dsm_registry/test_dsm_registry.c b/src/test/modules/test_dsm_registry/test_dsm_registry.c index 141c8ed1b34..4cc2ccdac3f 100644 --- a/src/test/modules/test_dsm_registry/test_dsm_registry.c +++ b/src/test/modules/test_dsm_registry/test_dsm_registry.c @@ -48,7 +48,7 @@ init_tdr_dsm(void *ptr) { TestDSMRegistryStruct *dsm = (TestDSMRegistryStruct *) ptr; - LWLockInitialize(&dsm->lck, LWLockNewTrancheId()); + LWLockInitialize(&dsm->lck, LWLockNewTrancheId("test_dsm_registry")); dsm->val = 0; } @@ -61,7 +61,6 @@ tdr_attach_shmem(void) sizeof(TestDSMRegistryStruct), init_tdr_dsm, &found); - LWLockRegisterTranche(tdr_dsm->lck.tranche, "test_dsm_registry"); if (tdr_dsa == NULL) tdr_dsa = GetNamedDSA("test_dsm_registry_dsa", &found); diff --git a/src/test/modules/test_radixtree/test_radixtree.c b/src/test/modules/test_radixtree/test_radixtree.c index 80ad0296164..787162c8793 100644 --- a/src/test/modules/test_radixtree/test_radixtree.c +++ b/src/test/modules/test_radixtree/test_radixtree.c @@ -124,10 +124,9 @@ test_empty(void) rt_iter *iter; uint64 key; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -167,10 +166,9 @@ test_basic(rt_node_class_test_elem *test_info, int shift, bool asc) uint64 *keys; int children = test_info->nkeys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else @@ -304,10 +302,9 @@ test_random(void) int num_keys = 100000; uint64 *keys; #ifdef TEST_SHARED_RT - int tranche_id = LWLockNewTrancheId(); + int tranche_id = LWLockNewTrancheId("test_radix_tree"); dsa_area *dsa; - LWLockRegisterTranche(tranche_id, "test_radix_tree"); dsa = dsa_create(tranche_id); radixtree = rt_create(dsa, tranche_id); #else diff --git a/src/test/modules/test_slru/test_slru.c b/src/test/modules/test_slru/test_slru.c index 32750930e43..8c0367eeee4 100644 --- a/src/test/modules/test_slru/test_slru.c +++ b/src/test/modules/test_slru/test_slru.c @@ -232,11 +232,9 @@ test_slru_shmem_startup(void) (void) MakePGDirectory(slru_dir_name); /* initialize the SLRU facility */ - test_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_slru_tranche"); + test_tranche_id = LWLockNewTrancheId("test_slru_tranche"); - test_buffer_tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(test_tranche_id, "test_buffer_tranche"); + test_buffer_tranche_id = LWLockNewTrancheId("test_buffer_tranche"); TestSlruCtl->PagePrecedes = test_slru_page_precedes_logically; SimpleLruInit(TestSlruCtl, "TestSLRU", diff --git a/src/test/modules/test_tidstore/test_tidstore.c b/src/test/modules/test_tidstore/test_tidstore.c index eb16e0fbfa6..0c8f43867e5 100644 --- a/src/test/modules/test_tidstore/test_tidstore.c +++ b/src/test/modules/test_tidstore/test_tidstore.c @@ -103,8 +103,7 @@ test_create(PG_FUNCTION_ARGS) { int tranche_id; - tranche_id = LWLockNewTrancheId(); - LWLockRegisterTranche(tranche_id, "test_tidstore"); + tranche_id = LWLockNewTrancheId("test_tidstore"); tidstore = TidStoreCreateShared(tidstore_max_size, tranche_id); -- 2.39.5 (Apple Git-154)
