Thread: Minor lmgr code cleanup
Try to reduce confusion about what is a lock method identifier, a lock method control structure, or a table of control structures. . Use type LOCKMASK where an int is not a counter. . Get rid of INVALID_TABLEID, use INVALID_LOCKMETHOD instead. . Use INVALID_LOCKMETHOD instead of (LOCKMETHOD) NULL, because LOCKMETHOD is not a pointer. . Define and use macro LockMethodIsValid. . Rename LOCKMETHOD to LOCKMETHODID. . Remove global variable LongTermTableId in lmgr.c, because it is never used. . Make LockTableId static in lmgr.c, because it is used nowhere else. Why not remove it and use DEFAULT_LOCKMETHOD? . Rename the lock method control structure from LOCKMETHODTABLE to LockMethodData. Introduce a pointer type named LockMethod. . Remove elog(FATAL) after InitLockTable() call in CreateSharedMemoryAndSemaphores(), because if something goes wrong, there is elog(FATAL) in LockMethodTableInit(), and if this doesn't help, an elog(ERROR) in InitLockTable() is promoted to FATAL. . Make InitLockTable() void, because its only caller does not use its return value any more. . Rename variables in lock.c to avoid statements like LockMethodTable[NumLockMethods] = lockMethodTable; lockMethodTable = LockMethodTable[lockmethod]; . Change LOCKMETHODID type to uint16 to fit into struct LOCKTAG. . Remove static variables BITS_OFF and BITS_ON from lock.c, because I agree to this doubt: * XXX is a fetch from a static array really faster than a shift? ====================== All 93 tests passed. ====================== Servus Manfred diff -ruN ../base/src/backend/storage/ipc/ipci.c src/backend/storage/ipc/ipci.c --- ../base/src/backend/storage/ipc/ipci.c 2003-08-04 04:40:03.000000000 +0200 +++ src/backend/storage/ipc/ipci.c 2003-09-06 16:53:54.000000000 +0200 @@ -111,8 +111,7 @@ * Set up lock manager */ InitLocks(); - if (InitLockTable(maxBackends) == INVALID_TABLEID) - elog(FATAL, "could not create the lock table"); + InitLockTable(maxBackends); /* * Set up process table diff -ruN ../base/src/backend/storage/lmgr/deadlock.c src/backend/storage/lmgr/deadlock.c --- ../base/src/backend/storage/lmgr/deadlock.c 2003-08-08 23:42:00.000000000 +0200 +++ src/backend/storage/lmgr/deadlock.c 2003-09-06 22:25:46.000000000 +0200 @@ -428,7 +428,7 @@ LOCK *lock; PROCLOCK *proclock; SHM_QUEUE *lockHolders; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; PROC_QUEUE *waitQueue; int queue_size; int conflictMask; diff -ruN ../base/src/backend/storage/lmgr/lmgr.c src/backend/storage/lmgr/lmgr.c --- ../base/src/backend/storage/lmgr/lmgr.c 2003-08-18 00:41:12.000000000 +0200 +++ src/backend/storage/lmgr/lmgr.c 2003-09-06 18:10:48.000000000 +0200 @@ -65,26 +65,24 @@ }; -LOCKMETHOD LockTableId = (LOCKMETHOD) NULL; -LOCKMETHOD LongTermTableId = (LOCKMETHOD) NULL; +static LOCKMETHODID LockTableId = INVALID_LOCKMETHOD; /* * Create the lock table described by LockConflicts */ -LOCKMETHOD +void InitLockTable(int maxBackends) { - int lockmethod; + LOCKMETHODID LongTermTableId; /* number of lock modes is lengthof()-1 because of dummy zero */ - lockmethod = LockMethodTableInit("LockTable", - LockConflicts, - lengthof(LockConflicts) - 1, - maxBackends); - LockTableId = lockmethod; - - if (!(LockTableId)) + LockTableId = LockMethodTableInit("LockTable", + LockConflicts, + lengthof(LockConflicts) - 1, + maxBackends); + if (!LockMethodIsValid(LockTableId)) elog(ERROR, "could not initialize lock table"); + Assert(LockTableId == DEFAULT_LOCKMETHOD); #ifdef USER_LOCKS @@ -92,11 +90,10 @@ * Allocate another tableId for long-term locks */ LongTermTableId = LockMethodTableRename(LockTableId); - if (!(LongTermTableId)) + if (!LockMethodIsValid(LongTermTableId)) elog(ERROR, "could not rename long-term lock table"); + Assert(LongTermTableId == USER_LOCKMETHOD); #endif - - return LockTableId; } /* diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c --- ../base/src/backend/storage/lmgr/lock.c 2003-08-18 00:41:12.000000000 +0200 +++ src/backend/storage/lmgr/lock.c 2003-09-06 23:01:05.000000000 +0200 @@ -46,7 +46,7 @@ #define NLOCKENTS(maxBackends) (max_locks_per_xact * (maxBackends)) -static int WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, +static int WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock); static void LockCountMyLocks(SHMEM_OFFSET lockOffset, PGPROC *proc, int *myHolding); @@ -150,19 +150,15 @@ /* - * These are to simplify/speed up some bit arithmetic. - * - * XXX is a fetch from a static array really faster than a shift? - * Wouldn't bet on it... + * These are to simplify some bit arithmetic. */ - -static LOCKMASK BITS_OFF[MAX_LOCKMODES]; -static LOCKMASK BITS_ON[MAX_LOCKMODES]; +#define BITS_ON(i) (1 << (i)) +#define BITS_OFF(i) ~(1 << (i)) /* - * map from lockmethod to the lock table structure + * map from lock method id to the lock table structure */ -static LOCKMETHODTABLE *LockMethodTable[MAX_LOCK_METHODS]; +static LockMethod LockMethods[MAX_LOCK_METHODS]; static int NumLockMethods; @@ -173,28 +169,20 @@ void InitLocks(void) { - int i; - int bit; - - bit = 1; - for (i = 0; i < MAX_LOCKMODES; i++, bit <<= 1) - { - BITS_ON[i] = bit; - BITS_OFF[i] = ~bit; - } + /* NOP */ } /* * Fetch the lock method table associated with a given lock */ -LOCKMETHODTABLE * +LockMethod GetLocksMethodTable(LOCK *lock) { - LOCKMETHOD lockmethod = LOCK_LOCKMETHOD(*lock); + LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*lock); - Assert(lockmethod > 0 && lockmethod < NumLockMethods); - return LockMethodTable[lockmethod]; + Assert(0 < lockmethodid && lockmethodid < NumLockMethods); + return LockMethods[lockmethodid]; } @@ -205,7 +193,7 @@ * Notes: just copying. Should only be called once. */ static void -LockMethodInit(LOCKMETHODTABLE *lockMethodTable, +LockMethodInit(LockMethod lockMethodTable, LOCKMASK *conflictsP, int numModes) { @@ -226,13 +214,13 @@ * by the postmaster are inherited by each backend, so they must be in * TopMemoryContext. */ -LOCKMETHOD +LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, int numModes, int maxBackends) { - LOCKMETHODTABLE *lockMethodTable; + LockMethod newLockMethod; char *shmemName; HASHCTL info; int hash_flags; @@ -254,10 +242,10 @@ /* each lock table has a header in shared memory */ sprintf(shmemName, "%s (lock method table)", tabName); - lockMethodTable = (LOCKMETHODTABLE *) - ShmemInitStruct(shmemName, sizeof(LOCKMETHODTABLE), &found); + newLockMethod = (LockMethod) + ShmemInitStruct(shmemName, sizeof(LockMethodData), &found); - if (!lockMethodTable) + if (!newLockMethod) elog(FATAL, "could not initialize lock table \"%s\"", tabName); /* @@ -275,15 +263,15 @@ */ if (!found) { - MemSet(lockMethodTable, 0, sizeof(LOCKMETHODTABLE)); - lockMethodTable->masterLock = LockMgrLock; - lockMethodTable->lockmethod = NumLockMethods; + MemSet(newLockMethod, 0, sizeof(LockMethodData)); + newLockMethod->masterLock = LockMgrLock; + newLockMethod->lockmethodid = NumLockMethods; } /* * other modules refer to the lock table by a lockmethod ID */ - LockMethodTable[NumLockMethods] = lockMethodTable; + LockMethods[NumLockMethods] = newLockMethod; NumLockMethods++; Assert(NumLockMethods <= MAX_LOCK_METHODS); @@ -297,15 +285,15 @@ hash_flags = (HASH_ELEM | HASH_FUNCTION); sprintf(shmemName, "%s (lock hash)", tabName); - lockMethodTable->lockHash = ShmemInitHash(shmemName, - init_table_size, - max_table_size, - &info, - hash_flags); + newLockMethod->lockHash = ShmemInitHash(shmemName, + init_table_size, + max_table_size, + &info, + hash_flags); - if (!lockMethodTable->lockHash) + if (!newLockMethod->lockHash) elog(FATAL, "could not initialize lock table \"%s\"", tabName); - Assert(lockMethodTable->lockHash->hash == tag_hash); + Assert(newLockMethod->lockHash->hash == tag_hash); /* * allocate a hash table for PROCLOCK structs. This is used to store @@ -317,23 +305,23 @@ hash_flags = (HASH_ELEM | HASH_FUNCTION); sprintf(shmemName, "%s (proclock hash)", tabName); - lockMethodTable->proclockHash = ShmemInitHash(shmemName, - init_table_size, - max_table_size, - &info, - hash_flags); + newLockMethod->proclockHash = ShmemInitHash(shmemName, + init_table_size, + max_table_size, + &info, + hash_flags); - if (!lockMethodTable->proclockHash) + if (!newLockMethod->proclockHash) elog(FATAL, "could not initialize lock table \"%s\"", tabName); /* init data structures */ - LockMethodInit(lockMethodTable, conflictsP, numModes); + LockMethodInit(newLockMethod, conflictsP, numModes); LWLockRelease(LockMgrLock); pfree(shmemName); - return lockMethodTable->lockmethod; + return newLockMethod->lockmethodid; } /* @@ -349,22 +337,22 @@ * short term and long term locks, yet store them all in one hashtable. */ -LOCKMETHOD -LockMethodTableRename(LOCKMETHOD lockmethod) +LOCKMETHODID +LockMethodTableRename(LOCKMETHODID lockmethodid) { - LOCKMETHOD newLockMethod; + LOCKMETHODID newLockMethodId; if (NumLockMethods >= MAX_LOCK_METHODS) return INVALID_LOCKMETHOD; - if (LockMethodTable[lockmethod] == INVALID_LOCKMETHOD) + if (LockMethods[lockmethodid] == INVALID_LOCKMETHOD) return INVALID_LOCKMETHOD; /* other modules refer to the lock table by a lockmethod ID */ - newLockMethod = NumLockMethods; + newLockMethodId = NumLockMethods; NumLockMethods++; - LockMethodTable[newLockMethod] = LockMethodTable[lockmethod]; - return newLockMethod; + LockMethods[newLockMethodId] = LockMethods[lockmethodid]; + return newLockMethodId; } /* @@ -412,7 +400,7 @@ * * normal lock user lock * - * lockmethod 1 2 + * lockmethodid 1 2 * tag.dbId database oid database oid * tag.relId rel oid or 0 0 * tag.objId block id lock id2 @@ -429,7 +417,7 @@ */ bool -LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, +LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode, bool dontWait) { PROCLOCK *proclock; @@ -438,25 +426,25 @@ bool found; LOCK *lock; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; int status; int myHolding[MAX_LOCKMODES]; int i; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) elog(LOG, "LockAcquire: user lock [%u] %s", locktag->objId.blkno, lock_mode_names[lockmode]); #endif /* ???????? This must be changed when short term locks will be used */ - locktag->lockmethod = lockmethod; + locktag->lockmethodid = lockmethodid; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { - elog(WARNING, "bad lock table id: %d", lockmethod); + elog(WARNING, "bad lock table id: %d", lockmethodid); return FALSE; } @@ -682,7 +670,7 @@ /* * Sleep till someone wakes me up. */ - status = WaitOnLock(lockmethod, lockmode, lock, proclock); + status = WaitOnLock(lockmethodid, lockmode, lock, proclock); /* * NOTE: do not do any material change of state between here and @@ -729,7 +717,7 @@ * known. If NULL is passed then these values will be computed internally. */ int -LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, +LockCheckConflicts(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock, @@ -842,9 +830,9 @@ { lock->nGranted++; lock->granted[lockmode]++; - lock->grantMask |= BITS_ON[lockmode]; + lock->grantMask |= BITS_ON(lockmode); if (lock->granted[lockmode] == lock->requested[lockmode]) - lock->waitMask &= BITS_OFF[lockmode]; + lock->waitMask &= BITS_OFF(lockmode); LOCK_PRINT("GrantLock", lock, lockmode); Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); Assert(lock->nGranted <= lock->nRequested); @@ -862,14 +850,14 @@ * The locktable's masterLock must be held at entry. */ static int -WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, +WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock) { - LOCKMETHODTABLE *lockMethodTable = LockMethodTable[lockmethod]; + LockMethod lockMethodTable = LockMethods[lockmethodid]; char *new_status, *old_status; - Assert(lockmethod < NumLockMethods); + Assert(lockmethodid < NumLockMethods); LOCK_PRINT("WaitOnLock: sleeping on lock", lock, lockmode); @@ -957,7 +945,7 @@ waitLock->requested[lockmode]--; /* don't forget to clear waitMask bit if appropriate */ if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) - waitLock->waitMask &= BITS_OFF[lockmode]; + waitLock->waitMask &= BITS_OFF(lockmode); /* Clean up the proc's own state */ proc->waitLock = NULL; @@ -968,7 +956,7 @@ } /* - * LockRelease -- look up 'locktag' in lock table 'lockmethod' and + * LockRelease -- look up 'locktag' in lock table 'lockmethodid' and * release one 'lockmode' lock on it. * * Side Effects: find any waiting processes that are now wakable, @@ -978,27 +966,27 @@ * come along and request the lock.) */ bool -LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, +LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode) { LOCK *lock; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; PROCLOCK *proclock; PROCLOCKTAG proclocktag; HTAB *proclockTable; bool wakeupNeeded = false; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) elog(LOG, "LockRelease: user lock tag [%u] %d", locktag->objId.blkno, lockmode); #endif /* ???????? This must be changed when short term locks will be used */ - locktag->lockmethod = lockmethod; + locktag->lockmethodid = lockmethodid; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { elog(WARNING, "lockMethodTable is null in LockRelease"); @@ -1045,7 +1033,7 @@ { LWLockRelease(masterLock); #ifdef USER_LOCKS - if (lockmethod == USER_LOCKMETHOD) + if (lockmethodid == USER_LOCKMETHOD) elog(WARNING, "no lock with this tag"); else #endif @@ -1083,7 +1071,7 @@ if (lock->granted[lockmode] == 0) { /* change the conflict mask. No more of this lock type. */ - lock->grantMask &= BITS_OFF[lockmode]; + lock->grantMask &= BITS_OFF(lockmode); } LOCK_PRINT("LockRelease: updated", lock, lockmode); @@ -1173,29 +1161,29 @@ * specified XID are released. */ bool -LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, +LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, bool allxids, TransactionId xid) { SHM_QUEUE *procHolders = &(proc->procHolders); PROCLOCK *proclock; PROCLOCK *nextHolder; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; int i, numLockModes; LOCK *lock; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) elog(LOG, "LockReleaseAll: lockmethod=%d, pid=%d", - lockmethod, proc->pid); + lockmethodid, proc->pid); #endif - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { - elog(WARNING, "bad lock method: %d", lockmethod); + elog(WARNING, "bad lock method: %d", lockmethodid); return FALSE; } @@ -1220,7 +1208,7 @@ lock = (LOCK *) MAKE_PTR(proclock->tag.lock); /* Ignore items that are not of the lockmethod to be removed */ - if (LOCK_LOCKMETHOD(*lock) != lockmethod) + if (LOCK_LOCKMETHOD(*lock) != lockmethodid) goto next_item; /* If not allxids, ignore items that are of the wrong xid */ @@ -1249,7 +1237,7 @@ lock->granted[i] -= proclock->holding[i]; Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); if (lock->granted[i] == 0) - lock->grantMask &= BITS_OFF[i]; + lock->grantMask &= BITS_OFF(i); /* * Read comments in LockRelease @@ -1331,7 +1319,7 @@ LWLockRelease(masterLock); #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) elog(LOG, "LockReleaseAll done"); #endif @@ -1346,7 +1334,7 @@ size += MAXALIGN(sizeof(PROC_HDR)); /* ProcGlobal */ size += maxBackends * MAXALIGN(sizeof(PGPROC)); /* each MyProc */ - size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LOCKMETHODTABLE)); /* each lockMethodTable */ + size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LockMethodData)); /* each lock method */ /* lockHash table */ size += hash_estimate_size(max_table_size, sizeof(LOCK)); @@ -1390,7 +1378,7 @@ LWLockAcquire(LockMgrLock, LW_EXCLUSIVE); - proclockTable = LockMethodTable[DEFAULT_LOCKMETHOD]->proclockHash; + proclockTable = LockMethods[DEFAULT_LOCKMETHOD]->proclockHash; data->nelements = i = proclockTable->hctl->nentries; @@ -1446,8 +1434,8 @@ SHM_QUEUE *procHolders; PROCLOCK *proclock; LOCK *lock; - int lockmethod = DEFAULT_LOCKMETHOD; - LOCKMETHODTABLE *lockMethodTable; + int lockmethodid = DEFAULT_LOCKMETHOD; + LockMethod lockMethodTable; proc = MyProc; if (proc == NULL) @@ -1455,8 +1443,8 @@ procHolders = &proc->procHolders; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) return; @@ -1489,8 +1477,8 @@ PGPROC *proc; PROCLOCK *proclock; LOCK *lock; - int lockmethod = DEFAULT_LOCKMETHOD; - LOCKMETHODTABLE *lockMethodTable; + int lockmethodid = DEFAULT_LOCKMETHOD; + LockMethod lockMethodTable; HTAB *proclockTable; HASH_SEQ_STATUS status; @@ -1498,8 +1486,8 @@ if (proc == NULL) return; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) return; diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c --- ../base/src/backend/storage/lmgr/proc.c 2003-08-04 04:40:03.000000000 +0200 +++ src/backend/storage/lmgr/proc.c 2003-09-06 22:43:50.000000000 +0200 @@ -524,7 +524,7 @@ * semaphore is normally zero, so when we try to acquire it, we sleep. */ int -ProcSleep(LOCKMETHODTABLE *lockMethodTable, +ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock) @@ -750,7 +750,7 @@ * for lock, waken any that are no longer blocked. */ void -ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock) +ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) { PROC_QUEUE *waitQueue = &(lock->waitProcs); int queue_size = waitQueue->size; diff -ruN ../base/src/include/storage/lmgr.h src/include/storage/lmgr.h --- ../base/src/include/storage/lmgr.h 2003-08-04 04:40:14.000000000 +0200 +++ src/include/storage/lmgr.h 2003-09-06 18:02:43.000000000 +0200 @@ -40,10 +40,7 @@ * so increase that if you want to add more modes. */ -extern LOCKMETHOD LockTableId; - - -extern LOCKMETHOD InitLockTable(int maxBackends); +extern void InitLockTable(int maxBackends); extern void RelationInitLockInfo(Relation relation); /* Lock a relation */ diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h --- ../base/src/include/storage/lock.h 2003-08-04 04:40:14.000000000 +0200 +++ src/include/storage/lock.h 2003-09-06 22:45:16.000000000 +0200 @@ -42,22 +42,20 @@ typedef int LOCKMASK; - typedef int LOCKMODE; -typedef int LOCKMETHOD; - /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ #define MAX_LOCKMODES 10 +typedef uint16 LOCKMETHODID; /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ #define MAX_LOCK_METHODS 3 -#define INVALID_TABLEID 0 - -#define INVALID_LOCKMETHOD INVALID_TABLEID +#define INVALID_LOCKMETHOD 0 #define DEFAULT_LOCKMETHOD 1 #define USER_LOCKMETHOD 2 +#define LockMethodIsValid(lockmethodid) ((lockmethodid) != INVALID_LOCKMETHOD) + /* * There is normally only one lock method, the default one. * If user locks are enabled, an additional lock method is present. @@ -83,15 +81,16 @@ * masterLock -- synchronizes access to the table * */ -typedef struct LOCKMETHODTABLE +typedef struct LockMethodData { - HTAB *lockHash; - HTAB *proclockHash; - LOCKMETHOD lockmethod; - int numLockModes; - int conflictTab[MAX_LOCKMODES]; - LWLockId masterLock; -} LOCKMETHODTABLE; + HTAB *lockHash; + HTAB *proclockHash; + LOCKMETHODID lockmethodid; + int numLockModes; + LOCKMASK conflictTab[MAX_LOCKMODES]; + LWLockId masterLock; +} LockMethodData; +typedef LockMethodData *LockMethod; /* @@ -115,7 +114,7 @@ */ OffsetNumber offnum; - uint16 lockmethod; /* needed by userlocks */ + LOCKMETHODID lockmethodid; /* needed by userlocks */ } LOCKTAG; @@ -139,8 +138,8 @@ LOCKTAG tag; /* unique identifier of lockable object */ /* data */ - int grantMask; /* bitmask for lock types already granted */ - int waitMask; /* bitmask for lock types awaited */ + LOCKMASK grantMask; /* bitmask for lock types already granted */ + LOCKMASK waitMask; /* bitmask for lock types awaited */ SHM_QUEUE lockHolders; /* list of PROCLOCK objects assoc. with * lock */ PROC_QUEUE waitProcs; /* list of PGPROC objects waiting on lock */ @@ -151,7 +150,7 @@ int nGranted; /* total of granted[] array */ } LOCK; -#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethod) +#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethodid) /* @@ -204,7 +203,7 @@ } PROCLOCK; #define PROCLOCK_LOCKMETHOD(proclock) \ - (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethod) + (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethodid) /* * This struct holds information passed from lmgr internals to the lock @@ -227,17 +226,17 @@ * function prototypes */ extern void InitLocks(void); -extern LOCKMETHODTABLE *GetLocksMethodTable(LOCK *lock); -extern LOCKMETHOD LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, +extern LockMethod GetLocksMethodTable(LOCK *lock); +extern LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, int numModes, int maxBackends); -extern LOCKMETHOD LockMethodTableRename(LOCKMETHOD lockmethod); -extern bool LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, +extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid); +extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode, bool dontWait); -extern bool LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, +extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode); -extern bool LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, +extern bool LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, bool allxids, TransactionId xid); -extern int LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, +extern int LockCheckConflicts(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock, PGPROC *proc, int *myHolding); diff -ruN ../base/src/include/storage/proc.h src/include/storage/proc.h --- ../base/src/include/storage/proc.h 2003-08-04 04:40:15.000000000 +0200 +++ src/include/storage/proc.h 2003-09-06 22:45:54.000000000 +0200 @@ -101,10 +101,10 @@ extern void ProcReleaseLocks(bool isCommit); extern void ProcQueueInit(PROC_QUEUE *queue); -extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode, +extern int ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock); extern PGPROC *ProcWakeup(PGPROC *proc, int errType); -extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock); +extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); extern bool LockWaitCancel(void); extern void ProcWaitForSignal(void);
Manfred Koizar <mkoi-pg@aon.at> writes: > [large patch] Looks reasonable except > +#define BITS_OFF(i) ~(1 << (i)) I'd put another pair of parens around that. Also, it might be worth moving into a header file. There is at least one place in proc.c that manipulates lock masks using explicit shifts, because BITS_ON/BITS_OFF weren't visible outside lock.c. It'd be good to fix it. BTW, did you check that the code still compiles with LOCK_DEBUG enabled? How about contrib/userlock? regards, tom lane
On Sun, 07 Sep 2003 10:19:07 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> +#define BITS_OFF(i) ~(1 << (i)) > >I'd put another pair of parens around that. Also, it might be worth >moving into a header file. There is at least one place in proc.c that >manipulates lock masks using explicit shifts, because BITS_ON/BITS_OFF >weren't visible outside lock.c. Done. Small patch attached, should be applied after the large patch. Big fat all-in-one patch available on request. >BTW, did you check that the code still compiles with LOCK_DEBUG enabled? No, I didn't and it didn't. :-( Corrected. >How about contrib/userlock? Unaffected by my changes, still works AFAICT. Servus Manfred diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c --- ../base/src/backend/storage/lmgr/lock.c 2003-09-06 23:01:05.000000000 +0200 +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 @@ -111,7 +111,7 @@ "req(%d,%d,%d,%d,%d,%d,%d)=%d " "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", where, MAKE_OFFSET(lock), - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, lock->tag.objId.blkno, lock->grantMask, lock->requested[1], lock->requested[2], lock->requested[3], lock->requested[4], lock->requested[5], lock->requested[6], @@ -150,12 +150,6 @@ /* - * These are to simplify some bit arithmetic. - */ -#define BITS_ON(i) (1 << (i)) -#define BITS_OFF(i) ~(1 << (i)) - -/* * map from lock method id to the lock table structure */ static LockMethod LockMethods[MAX_LOCK_METHODS]; @@ -654,15 +648,12 @@ * Construct bitmask of locks this process holds on this object. */ { - int heldLocks = 0; - int tmpMask; + LOCKMASK heldLocks = 0; - for (i = 1, tmpMask = 2; - i <= lockMethodTable->numLockModes; - i++, tmpMask <<= 1) + for (i = 1; i <= lockMethodTable->numLockModes; i++) { if (myHolding[i] > 0) - heldLocks |= tmpMask; + heldLocks |= LOCKBIT_ON(i); } MyProc->heldLocks = heldLocks; } @@ -725,9 +716,8 @@ int *myHolding) /* myHolding[] array or NULL */ { int numLockModes = lockMethodTable->numLockModes; - int bitmask; - int i, - tmpMask; + LOCKMASK bitmask; + int i; int localHolding[MAX_LOCKMODES]; /* @@ -760,11 +750,10 @@ /* Compute mask of lock types held by other processes */ bitmask = 0; - tmpMask = 2; - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) + for (i = 1; i <= numLockModes; i++) { if (lock->granted[i] != myHolding[i]) - bitmask |= tmpMask; + bitmask |= LOCKBIT_ON(i); } /* @@ -830,9 +819,9 @@ { lock->nGranted++; lock->granted[lockmode]++; - lock->grantMask |= BITS_ON(lockmode); + lock->grantMask |= LOCKBIT_ON(lockmode); if (lock->granted[lockmode] == lock->requested[lockmode]) - lock->waitMask &= BITS_OFF(lockmode); + lock->waitMask &= LOCKBIT_OFF(lockmode); LOCK_PRINT("GrantLock", lock, lockmode); Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); Assert(lock->nGranted <= lock->nRequested); @@ -945,7 +934,7 @@ waitLock->requested[lockmode]--; /* don't forget to clear waitMask bit if appropriate */ if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) - waitLock->waitMask &= BITS_OFF(lockmode); + waitLock->waitMask &= LOCKBIT_OFF(lockmode); /* Clean up the proc's own state */ proc->waitLock = NULL; @@ -1071,7 +1060,7 @@ if (lock->granted[lockmode] == 0) { /* change the conflict mask. No more of this lock type. */ - lock->grantMask &= BITS_OFF(lockmode); + lock->grantMask &= LOCKBIT_OFF(lockmode); } LOCK_PRINT("LockRelease: updated", lock, lockmode); @@ -1237,7 +1226,7 @@ lock->granted[i] -= proclock->holding[i]; Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); if (lock->granted[i] == 0) - lock->grantMask &= BITS_OFF(i); + lock->grantMask &= LOCKBIT_OFF(i); /* * Read comments in LockRelease diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c --- ../base/src/backend/storage/lmgr/proc.c 2003-09-06 22:43:50.000000000 +0200 +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 @@ -531,7 +531,7 @@ { LWLockId masterLock = lockMethodTable->masterLock; PROC_QUEUE *waitQueue = &(lock->waitProcs); - int myHeldLocks = MyProc->heldLocks; + LOCKMASK myHeldLocks = MyProc->heldLocks; bool early_deadlock = false; PGPROC *proc; int i; @@ -556,7 +556,7 @@ */ if (myHeldLocks != 0) { - int aheadRequests = 0; + LOCKMASK aheadRequests = 0; proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); for (i = 0; i < waitQueue->size; i++) @@ -596,7 +596,7 @@ break; } /* Nope, so advance to next waiter */ - aheadRequests |= (1 << proc->waitLockMode); + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); proc = (PGPROC *) MAKE_PTR(proc->links.next); } @@ -618,7 +618,7 @@ SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); waitQueue->size++; - lock->waitMask |= (1 << lockmode); + lock->waitMask |= LOCKBIT_ON(lockmode); /* Set up wait information in PGPROC object, too */ MyProc->waitLock = lock; @@ -755,7 +755,7 @@ PROC_QUEUE *waitQueue = &(lock->waitProcs); int queue_size = waitQueue->size; PGPROC *proc; - int aheadRequests = 0; + LOCKMASK aheadRequests = 0; Assert(queue_size >= 0); @@ -796,7 +796,7 @@ * Cannot wake this guy. Remember his request for later * checks. */ - aheadRequests |= (1 << lockmode); + aheadRequests |= LOCKBIT_ON(lockmode); proc = (PGPROC *) MAKE_PTR(proc->links.next); } } diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h --- ../base/src/include/storage/lock.h 2003-09-06 22:45:16.000000000 +0200 +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 @@ -46,6 +46,9 @@ /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ #define MAX_LOCKMODES 10 +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) + typedef uint16 LOCKMETHODID; /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ #define MAX_LOCK_METHODS 3
Manfred Koizar <mkoi-pg@aon.at> writes: >> I'd put another pair of parens around that. [etc] > Done. Small patch attached, should be applied after the large patch. > Big fat all-in-one patch available on request. Looks good to me. Bruce, please add to pending queue for 7.5? regards, tom lane
This has been saved for the 7.5 release: http:/momjian.postgresql.org/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Manfred Koizar wrote: > On Sun, 07 Sep 2003 10:19:07 -0400, Tom Lane <tgl@sss.pgh.pa.us> > wrote: > >> +#define BITS_OFF(i) ~(1 << (i)) > > > >I'd put another pair of parens around that. Also, it might be worth > >moving into a header file. There is at least one place in proc.c that > >manipulates lock masks using explicit shifts, because BITS_ON/BITS_OFF > >weren't visible outside lock.c. > > Done. Small patch attached, should be applied after the large patch. > Big fat all-in-one patch available on request. > > >BTW, did you check that the code still compiles with LOCK_DEBUG enabled? > > No, I didn't and it didn't. :-( Corrected. > > >How about contrib/userlock? > > Unaffected by my changes, still works AFAICT. > > Servus > Manfred > diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c > --- ../base/src/backend/storage/lmgr/lock.c 2003-09-06 23:01:05.000000000 +0200 > +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 > @@ -111,7 +111,7 @@ > "req(%d,%d,%d,%d,%d,%d,%d)=%d " > "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", > where, MAKE_OFFSET(lock), > - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, > + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, > lock->tag.objId.blkno, lock->grantMask, > lock->requested[1], lock->requested[2], lock->requested[3], > lock->requested[4], lock->requested[5], lock->requested[6], > @@ -150,12 +150,6 @@ > > > /* > - * These are to simplify some bit arithmetic. > - */ > -#define BITS_ON(i) (1 << (i)) > -#define BITS_OFF(i) ~(1 << (i)) > - > -/* > * map from lock method id to the lock table structure > */ > static LockMethod LockMethods[MAX_LOCK_METHODS]; > @@ -654,15 +648,12 @@ > * Construct bitmask of locks this process holds on this object. > */ > { > - int heldLocks = 0; > - int tmpMask; > + LOCKMASK heldLocks = 0; > > - for (i = 1, tmpMask = 2; > - i <= lockMethodTable->numLockModes; > - i++, tmpMask <<= 1) > + for (i = 1; i <= lockMethodTable->numLockModes; i++) > { > if (myHolding[i] > 0) > - heldLocks |= tmpMask; > + heldLocks |= LOCKBIT_ON(i); > } > MyProc->heldLocks = heldLocks; > } > @@ -725,9 +716,8 @@ > int *myHolding) /* myHolding[] array or NULL */ > { > int numLockModes = lockMethodTable->numLockModes; > - int bitmask; > - int i, > - tmpMask; > + LOCKMASK bitmask; > + int i; > int localHolding[MAX_LOCKMODES]; > > /* > @@ -760,11 +750,10 @@ > > /* Compute mask of lock types held by other processes */ > bitmask = 0; > - tmpMask = 2; > - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) > + for (i = 1; i <= numLockModes; i++) > { > if (lock->granted[i] != myHolding[i]) > - bitmask |= tmpMask; > + bitmask |= LOCKBIT_ON(i); > } > > /* > @@ -830,9 +819,9 @@ > { > lock->nGranted++; > lock->granted[lockmode]++; > - lock->grantMask |= BITS_ON(lockmode); > + lock->grantMask |= LOCKBIT_ON(lockmode); > if (lock->granted[lockmode] == lock->requested[lockmode]) > - lock->waitMask &= BITS_OFF(lockmode); > + lock->waitMask &= LOCKBIT_OFF(lockmode); > LOCK_PRINT("GrantLock", lock, lockmode); > Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); > Assert(lock->nGranted <= lock->nRequested); > @@ -945,7 +934,7 @@ > waitLock->requested[lockmode]--; > /* don't forget to clear waitMask bit if appropriate */ > if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) > - waitLock->waitMask &= BITS_OFF(lockmode); > + waitLock->waitMask &= LOCKBIT_OFF(lockmode); > > /* Clean up the proc's own state */ > proc->waitLock = NULL; > @@ -1071,7 +1060,7 @@ > if (lock->granted[lockmode] == 0) > { > /* change the conflict mask. No more of this lock type. */ > - lock->grantMask &= BITS_OFF(lockmode); > + lock->grantMask &= LOCKBIT_OFF(lockmode); > } > > LOCK_PRINT("LockRelease: updated", lock, lockmode); > @@ -1237,7 +1226,7 @@ > lock->granted[i] -= proclock->holding[i]; > Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); > if (lock->granted[i] == 0) > - lock->grantMask &= BITS_OFF(i); > + lock->grantMask &= LOCKBIT_OFF(i); > > /* > * Read comments in LockRelease > diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c > --- ../base/src/backend/storage/lmgr/proc.c 2003-09-06 22:43:50.000000000 +0200 > +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 > @@ -531,7 +531,7 @@ > { > LWLockId masterLock = lockMethodTable->masterLock; > PROC_QUEUE *waitQueue = &(lock->waitProcs); > - int myHeldLocks = MyProc->heldLocks; > + LOCKMASK myHeldLocks = MyProc->heldLocks; > bool early_deadlock = false; > PGPROC *proc; > int i; > @@ -556,7 +556,7 @@ > */ > if (myHeldLocks != 0) > { > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); > for (i = 0; i < waitQueue->size; i++) > @@ -596,7 +596,7 @@ > break; > } > /* Nope, so advance to next waiter */ > - aheadRequests |= (1 << proc->waitLockMode); > + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > > @@ -618,7 +618,7 @@ > SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); > waitQueue->size++; > > - lock->waitMask |= (1 << lockmode); > + lock->waitMask |= LOCKBIT_ON(lockmode); > > /* Set up wait information in PGPROC object, too */ > MyProc->waitLock = lock; > @@ -755,7 +755,7 @@ > PROC_QUEUE *waitQueue = &(lock->waitProcs); > int queue_size = waitQueue->size; > PGPROC *proc; > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > Assert(queue_size >= 0); > > @@ -796,7 +796,7 @@ > * Cannot wake this guy. Remember his request for later > * checks. > */ > - aheadRequests |= (1 << lockmode); > + aheadRequests |= LOCKBIT_ON(lockmode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > } > diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h > --- ../base/src/include/storage/lock.h 2003-09-06 22:45:16.000000000 +0200 > +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 > @@ -46,6 +46,9 @@ > /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ > #define MAX_LOCKMODES 10 > > +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) > +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) > + > typedef uint16 LOCKMETHODID; > /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ > #define MAX_LOCK_METHODS 3 > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Tom Lane wrote: > Manfred Koizar <mkoi-pg@aon.at> writes: > >> I'd put another pair of parens around that. [etc] > > > Done. Small patch attached, should be applied after the large patch. > > Big fat all-in-one patch available on request. > > Looks good to me. Bruce, please add to pending queue for 7.5? Done. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > This has been saved for the 7.5 release: > http:/momjian.postgresql.org/cgi-bin/pgpatches2 BTW, that page still calls itself "PostgreSQL Items For 7.4." regards, tom lane
On Mon, 8 Sep 2003 00:51:45 -0400 (EDT), Bruce Momjian <pgman@candle.pha.pa.us> wrote: > >This has been saved for the 7.5 release: > > http:/momjian.postgresql.org/cgi-bin/pgpatches2 Here is the combined patch. Prior patches in this thread are hereby obsolete. Servus Manfred diff -ruN ../base/src/backend/storage/ipc/ipci.c src/backend/storage/ipc/ipci.c --- ../base/src/backend/storage/ipc/ipci.c 2003-08-04 04:40:03.000000000 +0200 +++ src/backend/storage/ipc/ipci.c 2003-09-06 16:53:54.000000000 +0200 @@ -111,8 +111,7 @@ * Set up lock manager */ InitLocks(); - if (InitLockTable(maxBackends) == INVALID_TABLEID) - elog(FATAL, "could not create the lock table"); + InitLockTable(maxBackends); /* * Set up process table diff -ruN ../base/src/backend/storage/lmgr/deadlock.c src/backend/storage/lmgr/deadlock.c --- ../base/src/backend/storage/lmgr/deadlock.c 2003-08-08 23:42:00.000000000 +0200 +++ src/backend/storage/lmgr/deadlock.c 2003-09-06 22:25:46.000000000 +0200 @@ -428,7 +428,7 @@ LOCK *lock; PROCLOCK *proclock; SHM_QUEUE *lockHolders; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; PROC_QUEUE *waitQueue; int queue_size; int conflictMask; diff -ruN ../base/src/backend/storage/lmgr/lmgr.c src/backend/storage/lmgr/lmgr.c --- ../base/src/backend/storage/lmgr/lmgr.c 2003-08-18 00:41:12.000000000 +0200 +++ src/backend/storage/lmgr/lmgr.c 2003-09-06 18:10:48.000000000 +0200 @@ -65,26 +65,24 @@ }; -LOCKMETHOD LockTableId = (LOCKMETHOD) NULL; -LOCKMETHOD LongTermTableId = (LOCKMETHOD) NULL; +static LOCKMETHODID LockTableId = INVALID_LOCKMETHOD; /* * Create the lock table described by LockConflicts */ -LOCKMETHOD +void InitLockTable(int maxBackends) { - int lockmethod; + LOCKMETHODID LongTermTableId; /* number of lock modes is lengthof()-1 because of dummy zero */ - lockmethod = LockMethodTableInit("LockTable", - LockConflicts, - lengthof(LockConflicts) - 1, - maxBackends); - LockTableId = lockmethod; - - if (!(LockTableId)) + LockTableId = LockMethodTableInit("LockTable", + LockConflicts, + lengthof(LockConflicts) - 1, + maxBackends); + if (!LockMethodIsValid(LockTableId)) elog(ERROR, "could not initialize lock table"); + Assert(LockTableId == DEFAULT_LOCKMETHOD); #ifdef USER_LOCKS @@ -92,11 +90,10 @@ * Allocate another tableId for long-term locks */ LongTermTableId = LockMethodTableRename(LockTableId); - if (!(LongTermTableId)) + if (!LockMethodIsValid(LongTermTableId)) elog(ERROR, "could not rename long-term lock table"); + Assert(LongTermTableId == USER_LOCKMETHOD); #endif - - return LockTableId; } /* diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c --- ../base/src/backend/storage/lmgr/lock.c 2003-08-18 00:41:12.000000000 +0200 +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 @@ -46,7 +46,7 @@ #define NLOCKENTS(maxBackends) (max_locks_per_xact * (maxBackends)) -static int WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, +static int WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock); static void LockCountMyLocks(SHMEM_OFFSET lockOffset, PGPROC *proc, int *myHolding); @@ -111,7 +111,7 @@ "req(%d,%d,%d,%d,%d,%d,%d)=%d " "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", where, MAKE_OFFSET(lock), - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, lock->tag.objId.blkno, lock->grantMask, lock->requested[1], lock->requested[2], lock->requested[3], lock->requested[4], lock->requested[5], lock->requested[6], @@ -150,19 +150,9 @@ /* - * These are to simplify/speed up some bit arithmetic. - * - * XXX is a fetch from a static array really faster than a shift? - * Wouldn't bet on it... + * map from lock method id to the lock table structure */ - -static LOCKMASK BITS_OFF[MAX_LOCKMODES]; -static LOCKMASK BITS_ON[MAX_LOCKMODES]; - -/* - * map from lockmethod to the lock table structure - */ -static LOCKMETHODTABLE *LockMethodTable[MAX_LOCK_METHODS]; +static LockMethod LockMethods[MAX_LOCK_METHODS]; static int NumLockMethods; @@ -173,28 +163,20 @@ void InitLocks(void) { - int i; - int bit; - - bit = 1; - for (i = 0; i < MAX_LOCKMODES; i++, bit <<= 1) - { - BITS_ON[i] = bit; - BITS_OFF[i] = ~bit; - } + /* NOP */ } /* * Fetch the lock method table associated with a given lock */ -LOCKMETHODTABLE * +LockMethod GetLocksMethodTable(LOCK *lock) { - LOCKMETHOD lockmethod = LOCK_LOCKMETHOD(*lock); + LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*lock); - Assert(lockmethod > 0 && lockmethod < NumLockMethods); - return LockMethodTable[lockmethod]; + Assert(0 < lockmethodid && lockmethodid < NumLockMethods); + return LockMethods[lockmethodid]; } @@ -205,7 +187,7 @@ * Notes: just copying. Should only be called once. */ static void -LockMethodInit(LOCKMETHODTABLE *lockMethodTable, +LockMethodInit(LockMethod lockMethodTable, LOCKMASK *conflictsP, int numModes) { @@ -226,13 +208,13 @@ * by the postmaster are inherited by each backend, so they must be in * TopMemoryContext. */ -LOCKMETHOD +LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, int numModes, int maxBackends) { - LOCKMETHODTABLE *lockMethodTable; + LockMethod newLockMethod; char *shmemName; HASHCTL info; int hash_flags; @@ -254,10 +236,10 @@ /* each lock table has a header in shared memory */ sprintf(shmemName, "%s (lock method table)", tabName); - lockMethodTable = (LOCKMETHODTABLE *) - ShmemInitStruct(shmemName, sizeof(LOCKMETHODTABLE), &found); + newLockMethod = (LockMethod) + ShmemInitStruct(shmemName, sizeof(LockMethodData), &found); - if (!lockMethodTable) + if (!newLockMethod) elog(FATAL, "could not initialize lock table \"%s\"", tabName); /* @@ -275,15 +257,15 @@ */ if (!found) { - MemSet(lockMethodTable, 0, sizeof(LOCKMETHODTABLE)); - lockMethodTable->masterLock = LockMgrLock; - lockMethodTable->lockmethod = NumLockMethods; + MemSet(newLockMethod, 0, sizeof(LockMethodData)); + newLockMethod->masterLock = LockMgrLock; + newLockMethod->lockmethodid = NumLockMethods; } /* * other modules refer to the lock table by a lockmethod ID */ - LockMethodTable[NumLockMethods] = lockMethodTable; + LockMethods[NumLockMethods] = newLockMethod; NumLockMethods++; Assert(NumLockMethods <= MAX_LOCK_METHODS); @@ -297,15 +279,15 @@ hash_flags = (HASH_ELEM | HASH_FUNCTION); sprintf(shmemName, "%s (lock hash)", tabName); - lockMethodTable->lockHash = ShmemInitHash(shmemName, - init_table_size, - max_table_size, - &info, - hash_flags); + newLockMethod->lockHash = ShmemInitHash(shmemName, + init_table_size, + max_table_size, + &info, + hash_flags); - if (!lockMethodTable->lockHash) + if (!newLockMethod->lockHash) elog(FATAL, "could not initialize lock table \"%s\"", tabName); - Assert(lockMethodTable->lockHash->hash == tag_hash); + Assert(newLockMethod->lockHash->hash == tag_hash); /* * allocate a hash table for PROCLOCK structs. This is used to store @@ -317,23 +299,23 @@ hash_flags = (HASH_ELEM | HASH_FUNCTION); sprintf(shmemName, "%s (proclock hash)", tabName); - lockMethodTable->proclockHash = ShmemInitHash(shmemName, - init_table_size, - max_table_size, - &info, - hash_flags); + newLockMethod->proclockHash = ShmemInitHash(shmemName, + init_table_size, + max_table_size, + &info, + hash_flags); - if (!lockMethodTable->proclockHash) + if (!newLockMethod->proclockHash) elog(FATAL, "could not initialize lock table \"%s\"", tabName); /* init data structures */ - LockMethodInit(lockMethodTable, conflictsP, numModes); + LockMethodInit(newLockMethod, conflictsP, numModes); LWLockRelease(LockMgrLock); pfree(shmemName); - return lockMethodTable->lockmethod; + return newLockMethod->lockmethodid; } /* @@ -349,22 +331,22 @@ * short term and long term locks, yet store them all in one hashtable. */ -LOCKMETHOD -LockMethodTableRename(LOCKMETHOD lockmethod) +LOCKMETHODID +LockMethodTableRename(LOCKMETHODID lockmethodid) { - LOCKMETHOD newLockMethod; + LOCKMETHODID newLockMethodId; if (NumLockMethods >= MAX_LOCK_METHODS) return INVALID_LOCKMETHOD; - if (LockMethodTable[lockmethod] == INVALID_LOCKMETHOD) + if (LockMethods[lockmethodid] == INVALID_LOCKMETHOD) return INVALID_LOCKMETHOD; /* other modules refer to the lock table by a lockmethod ID */ - newLockMethod = NumLockMethods; + newLockMethodId = NumLockMethods; NumLockMethods++; - LockMethodTable[newLockMethod] = LockMethodTable[lockmethod]; - return newLockMethod; + LockMethods[newLockMethodId] = LockMethods[lockmethodid]; + return newLockMethodId; } /* @@ -412,7 +394,7 @@ * * normal lock user lock * - * lockmethod 1 2 + * lockmethodid 1 2 * tag.dbId database oid database oid * tag.relId rel oid or 0 0 * tag.objId block id lock id2 @@ -429,7 +411,7 @@ */ bool -LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, +LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode, bool dontWait) { PROCLOCK *proclock; @@ -438,25 +420,25 @@ bool found; LOCK *lock; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; int status; int myHolding[MAX_LOCKMODES]; int i; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) elog(LOG, "LockAcquire: user lock [%u] %s", locktag->objId.blkno, lock_mode_names[lockmode]); #endif /* ???????? This must be changed when short term locks will be used */ - locktag->lockmethod = lockmethod; + locktag->lockmethodid = lockmethodid; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { - elog(WARNING, "bad lock table id: %d", lockmethod); + elog(WARNING, "bad lock table id: %d", lockmethodid); return FALSE; } @@ -666,15 +648,12 @@ * Construct bitmask of locks this process holds on this object. */ { - int heldLocks = 0; - int tmpMask; + LOCKMASK heldLocks = 0; - for (i = 1, tmpMask = 2; - i <= lockMethodTable->numLockModes; - i++, tmpMask <<= 1) + for (i = 1; i <= lockMethodTable->numLockModes; i++) { if (myHolding[i] > 0) - heldLocks |= tmpMask; + heldLocks |= LOCKBIT_ON(i); } MyProc->heldLocks = heldLocks; } @@ -682,7 +661,7 @@ /* * Sleep till someone wakes me up. */ - status = WaitOnLock(lockmethod, lockmode, lock, proclock); + status = WaitOnLock(lockmethodid, lockmode, lock, proclock); /* * NOTE: do not do any material change of state between here and @@ -729,7 +708,7 @@ * known. If NULL is passed then these values will be computed internally. */ int -LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, +LockCheckConflicts(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock, @@ -737,9 +716,8 @@ int *myHolding) /* myHolding[] array or NULL */ { int numLockModes = lockMethodTable->numLockModes; - int bitmask; - int i, - tmpMask; + LOCKMASK bitmask; + int i; int localHolding[MAX_LOCKMODES]; /* @@ -772,11 +750,10 @@ /* Compute mask of lock types held by other processes */ bitmask = 0; - tmpMask = 2; - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) + for (i = 1; i <= numLockModes; i++) { if (lock->granted[i] != myHolding[i]) - bitmask |= tmpMask; + bitmask |= LOCKBIT_ON(i); } /* @@ -842,9 +819,9 @@ { lock->nGranted++; lock->granted[lockmode]++; - lock->grantMask |= BITS_ON[lockmode]; + lock->grantMask |= LOCKBIT_ON(lockmode); if (lock->granted[lockmode] == lock->requested[lockmode]) - lock->waitMask &= BITS_OFF[lockmode]; + lock->waitMask &= LOCKBIT_OFF(lockmode); LOCK_PRINT("GrantLock", lock, lockmode); Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); Assert(lock->nGranted <= lock->nRequested); @@ -862,14 +839,14 @@ * The locktable's masterLock must be held at entry. */ static int -WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, +WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock) { - LOCKMETHODTABLE *lockMethodTable = LockMethodTable[lockmethod]; + LockMethod lockMethodTable = LockMethods[lockmethodid]; char *new_status, *old_status; - Assert(lockmethod < NumLockMethods); + Assert(lockmethodid < NumLockMethods); LOCK_PRINT("WaitOnLock: sleeping on lock", lock, lockmode); @@ -957,7 +934,7 @@ waitLock->requested[lockmode]--; /* don't forget to clear waitMask bit if appropriate */ if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) - waitLock->waitMask &= BITS_OFF[lockmode]; + waitLock->waitMask &= LOCKBIT_OFF(lockmode); /* Clean up the proc's own state */ proc->waitLock = NULL; @@ -968,7 +945,7 @@ } /* - * LockRelease -- look up 'locktag' in lock table 'lockmethod' and + * LockRelease -- look up 'locktag' in lock table 'lockmethodid' and * release one 'lockmode' lock on it. * * Side Effects: find any waiting processes that are now wakable, @@ -978,27 +955,27 @@ * come along and request the lock.) */ bool -LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, +LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode) { LOCK *lock; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; PROCLOCK *proclock; PROCLOCKTAG proclocktag; HTAB *proclockTable; bool wakeupNeeded = false; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) elog(LOG, "LockRelease: user lock tag [%u] %d", locktag->objId.blkno, lockmode); #endif /* ???????? This must be changed when short term locks will be used */ - locktag->lockmethod = lockmethod; + locktag->lockmethodid = lockmethodid; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { elog(WARNING, "lockMethodTable is null in LockRelease"); @@ -1045,7 +1022,7 @@ { LWLockRelease(masterLock); #ifdef USER_LOCKS - if (lockmethod == USER_LOCKMETHOD) + if (lockmethodid == USER_LOCKMETHOD) elog(WARNING, "no lock with this tag"); else #endif @@ -1083,7 +1060,7 @@ if (lock->granted[lockmode] == 0) { /* change the conflict mask. No more of this lock type. */ - lock->grantMask &= BITS_OFF[lockmode]; + lock->grantMask &= LOCKBIT_OFF(lockmode); } LOCK_PRINT("LockRelease: updated", lock, lockmode); @@ -1173,29 +1150,29 @@ * specified XID are released. */ bool -LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, +LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, bool allxids, TransactionId xid) { SHM_QUEUE *procHolders = &(proc->procHolders); PROCLOCK *proclock; PROCLOCK *nextHolder; LWLockId masterLock; - LOCKMETHODTABLE *lockMethodTable; + LockMethod lockMethodTable; int i, numLockModes; LOCK *lock; #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) elog(LOG, "LockReleaseAll: lockmethod=%d, pid=%d", - lockmethod, proc->pid); + lockmethodid, proc->pid); #endif - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) { - elog(WARNING, "bad lock method: %d", lockmethod); + elog(WARNING, "bad lock method: %d", lockmethodid); return FALSE; } @@ -1220,7 +1197,7 @@ lock = (LOCK *) MAKE_PTR(proclock->tag.lock); /* Ignore items that are not of the lockmethod to be removed */ - if (LOCK_LOCKMETHOD(*lock) != lockmethod) + if (LOCK_LOCKMETHOD(*lock) != lockmethodid) goto next_item; /* If not allxids, ignore items that are of the wrong xid */ @@ -1249,7 +1226,7 @@ lock->granted[i] -= proclock->holding[i]; Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); if (lock->granted[i] == 0) - lock->grantMask &= BITS_OFF[i]; + lock->grantMask &= LOCKBIT_OFF(i); /* * Read comments in LockRelease @@ -1331,7 +1308,7 @@ LWLockRelease(masterLock); #ifdef LOCK_DEBUG - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) elog(LOG, "LockReleaseAll done"); #endif @@ -1346,7 +1323,7 @@ size += MAXALIGN(sizeof(PROC_HDR)); /* ProcGlobal */ size += maxBackends * MAXALIGN(sizeof(PGPROC)); /* each MyProc */ - size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LOCKMETHODTABLE)); /* each lockMethodTable */ + size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LockMethodData)); /* each lock method */ /* lockHash table */ size += hash_estimate_size(max_table_size, sizeof(LOCK)); @@ -1390,7 +1367,7 @@ LWLockAcquire(LockMgrLock, LW_EXCLUSIVE); - proclockTable = LockMethodTable[DEFAULT_LOCKMETHOD]->proclockHash; + proclockTable = LockMethods[DEFAULT_LOCKMETHOD]->proclockHash; data->nelements = i = proclockTable->hctl->nentries; @@ -1446,8 +1423,8 @@ SHM_QUEUE *procHolders; PROCLOCK *proclock; LOCK *lock; - int lockmethod = DEFAULT_LOCKMETHOD; - LOCKMETHODTABLE *lockMethodTable; + int lockmethodid = DEFAULT_LOCKMETHOD; + LockMethod lockMethodTable; proc = MyProc; if (proc == NULL) @@ -1455,8 +1432,8 @@ procHolders = &proc->procHolders; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) return; @@ -1489,8 +1466,8 @@ PGPROC *proc; PROCLOCK *proclock; LOCK *lock; - int lockmethod = DEFAULT_LOCKMETHOD; - LOCKMETHODTABLE *lockMethodTable; + int lockmethodid = DEFAULT_LOCKMETHOD; + LockMethod lockMethodTable; HTAB *proclockTable; HASH_SEQ_STATUS status; @@ -1498,8 +1475,8 @@ if (proc == NULL) return; - Assert(lockmethod < NumLockMethods); - lockMethodTable = LockMethodTable[lockmethod]; + Assert(lockmethodid < NumLockMethods); + lockMethodTable = LockMethods[lockmethodid]; if (!lockMethodTable) return; diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c --- ../base/src/backend/storage/lmgr/proc.c 2003-08-04 04:40:03.000000000 +0200 +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 @@ -524,14 +524,14 @@ * semaphore is normally zero, so when we try to acquire it, we sleep. */ int -ProcSleep(LOCKMETHODTABLE *lockMethodTable, +ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock) { LWLockId masterLock = lockMethodTable->masterLock; PROC_QUEUE *waitQueue = &(lock->waitProcs); - int myHeldLocks = MyProc->heldLocks; + LOCKMASK myHeldLocks = MyProc->heldLocks; bool early_deadlock = false; PGPROC *proc; int i; @@ -556,7 +556,7 @@ */ if (myHeldLocks != 0) { - int aheadRequests = 0; + LOCKMASK aheadRequests = 0; proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); for (i = 0; i < waitQueue->size; i++) @@ -596,7 +596,7 @@ break; } /* Nope, so advance to next waiter */ - aheadRequests |= (1 << proc->waitLockMode); + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); proc = (PGPROC *) MAKE_PTR(proc->links.next); } @@ -618,7 +618,7 @@ SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); waitQueue->size++; - lock->waitMask |= (1 << lockmode); + lock->waitMask |= LOCKBIT_ON(lockmode); /* Set up wait information in PGPROC object, too */ MyProc->waitLock = lock; @@ -750,12 +750,12 @@ * for lock, waken any that are no longer blocked. */ void -ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock) +ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) { PROC_QUEUE *waitQueue = &(lock->waitProcs); int queue_size = waitQueue->size; PGPROC *proc; - int aheadRequests = 0; + LOCKMASK aheadRequests = 0; Assert(queue_size >= 0); @@ -796,7 +796,7 @@ * Cannot wake this guy. Remember his request for later * checks. */ - aheadRequests |= (1 << lockmode); + aheadRequests |= LOCKBIT_ON(lockmode); proc = (PGPROC *) MAKE_PTR(proc->links.next); } } diff -ruN ../base/src/include/storage/lmgr.h src/include/storage/lmgr.h --- ../base/src/include/storage/lmgr.h 2003-08-04 04:40:14.000000000 +0200 +++ src/include/storage/lmgr.h 2003-09-06 18:02:43.000000000 +0200 @@ -40,10 +40,7 @@ * so increase that if you want to add more modes. */ -extern LOCKMETHOD LockTableId; - - -extern LOCKMETHOD InitLockTable(int maxBackends); +extern void InitLockTable(int maxBackends); extern void RelationInitLockInfo(Relation relation); /* Lock a relation */ diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h --- ../base/src/include/storage/lock.h 2003-08-04 04:40:14.000000000 +0200 +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 @@ -42,22 +42,23 @@ typedef int LOCKMASK; - typedef int LOCKMODE; -typedef int LOCKMETHOD; - /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ #define MAX_LOCKMODES 10 +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) + +typedef uint16 LOCKMETHODID; /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ #define MAX_LOCK_METHODS 3 -#define INVALID_TABLEID 0 - -#define INVALID_LOCKMETHOD INVALID_TABLEID +#define INVALID_LOCKMETHOD 0 #define DEFAULT_LOCKMETHOD 1 #define USER_LOCKMETHOD 2 +#define LockMethodIsValid(lockmethodid) ((lockmethodid) != INVALID_LOCKMETHOD) + /* * There is normally only one lock method, the default one. * If user locks are enabled, an additional lock method is present. @@ -83,15 +84,16 @@ * masterLock -- synchronizes access to the table * */ -typedef struct LOCKMETHODTABLE +typedef struct LockMethodData { - HTAB *lockHash; - HTAB *proclockHash; - LOCKMETHOD lockmethod; - int numLockModes; - int conflictTab[MAX_LOCKMODES]; - LWLockId masterLock; -} LOCKMETHODTABLE; + HTAB *lockHash; + HTAB *proclockHash; + LOCKMETHODID lockmethodid; + int numLockModes; + LOCKMASK conflictTab[MAX_LOCKMODES]; + LWLockId masterLock; +} LockMethodData; +typedef LockMethodData *LockMethod; /* @@ -115,7 +117,7 @@ */ OffsetNumber offnum; - uint16 lockmethod; /* needed by userlocks */ + LOCKMETHODID lockmethodid; /* needed by userlocks */ } LOCKTAG; @@ -139,8 +141,8 @@ LOCKTAG tag; /* unique identifier of lockable object */ /* data */ - int grantMask; /* bitmask for lock types already granted */ - int waitMask; /* bitmask for lock types awaited */ + LOCKMASK grantMask; /* bitmask for lock types already granted */ + LOCKMASK waitMask; /* bitmask for lock types awaited */ SHM_QUEUE lockHolders; /* list of PROCLOCK objects assoc. with * lock */ PROC_QUEUE waitProcs; /* list of PGPROC objects waiting on lock */ @@ -151,7 +153,7 @@ int nGranted; /* total of granted[] array */ } LOCK; -#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethod) +#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethodid) /* @@ -204,7 +206,7 @@ } PROCLOCK; #define PROCLOCK_LOCKMETHOD(proclock) \ - (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethod) + (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethodid) /* * This struct holds information passed from lmgr internals to the lock @@ -227,17 +229,17 @@ * function prototypes */ extern void InitLocks(void); -extern LOCKMETHODTABLE *GetLocksMethodTable(LOCK *lock); -extern LOCKMETHOD LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, +extern LockMethod GetLocksMethodTable(LOCK *lock); +extern LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, int numModes, int maxBackends); -extern LOCKMETHOD LockMethodTableRename(LOCKMETHOD lockmethod); -extern bool LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, +extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid); +extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode, bool dontWait); -extern bool LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, +extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, TransactionId xid, LOCKMODE lockmode); -extern bool LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, +extern bool LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, bool allxids, TransactionId xid); -extern int LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, +extern int LockCheckConflicts(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock, PGPROC *proc, int *myHolding); diff -ruN ../base/src/include/storage/proc.h src/include/storage/proc.h --- ../base/src/include/storage/proc.h 2003-08-04 04:40:15.000000000 +0200 +++ src/include/storage/proc.h 2003-09-06 22:45:54.000000000 +0200 @@ -101,10 +101,10 @@ extern void ProcReleaseLocks(bool isCommit); extern void ProcQueueInit(PROC_QUEUE *queue); -extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode, +extern int ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, LOCK *lock, PROCLOCK *proclock); extern PGPROC *ProcWakeup(PGPROC *proc, int errType); -extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock); +extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); extern bool LockWaitCancel(void); extern void ProcWaitForSignal(void);
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > This has been saved for the 7.5 release: > > http:/momjian.postgresql.org/cgi-bin/pgpatches2 > > BTW, that page still calls itself "PostgreSQL Items For 7.4." Thanks. Fixed. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
This has been saved for the 7.5 release: http:/momjian.postgresql.org/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Manfred Koizar wrote: > On Mon, 8 Sep 2003 00:51:45 -0400 (EDT), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > >This has been saved for the 7.5 release: > > > > http:/momjian.postgresql.org/cgi-bin/pgpatches2 > > Here is the combined patch. Prior patches in this thread are hereby > obsolete. > > Servus > Manfred > diff -ruN ../base/src/backend/storage/ipc/ipci.c src/backend/storage/ipc/ipci.c > --- ../base/src/backend/storage/ipc/ipci.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/ipc/ipci.c 2003-09-06 16:53:54.000000000 +0200 > @@ -111,8 +111,7 @@ > * Set up lock manager > */ > InitLocks(); > - if (InitLockTable(maxBackends) == INVALID_TABLEID) > - elog(FATAL, "could not create the lock table"); > + InitLockTable(maxBackends); > > /* > * Set up process table > diff -ruN ../base/src/backend/storage/lmgr/deadlock.c src/backend/storage/lmgr/deadlock.c > --- ../base/src/backend/storage/lmgr/deadlock.c 2003-08-08 23:42:00.000000000 +0200 > +++ src/backend/storage/lmgr/deadlock.c 2003-09-06 22:25:46.000000000 +0200 > @@ -428,7 +428,7 @@ > LOCK *lock; > PROCLOCK *proclock; > SHM_QUEUE *lockHolders; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROC_QUEUE *waitQueue; > int queue_size; > int conflictMask; > diff -ruN ../base/src/backend/storage/lmgr/lmgr.c src/backend/storage/lmgr/lmgr.c > --- ../base/src/backend/storage/lmgr/lmgr.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lmgr.c 2003-09-06 18:10:48.000000000 +0200 > @@ -65,26 +65,24 @@ > > }; > > -LOCKMETHOD LockTableId = (LOCKMETHOD) NULL; > -LOCKMETHOD LongTermTableId = (LOCKMETHOD) NULL; > +static LOCKMETHODID LockTableId = INVALID_LOCKMETHOD; > > /* > * Create the lock table described by LockConflicts > */ > -LOCKMETHOD > +void > InitLockTable(int maxBackends) > { > - int lockmethod; > + LOCKMETHODID LongTermTableId; > > /* number of lock modes is lengthof()-1 because of dummy zero */ > - lockmethod = LockMethodTableInit("LockTable", > - LockConflicts, > - lengthof(LockConflicts) - 1, > - maxBackends); > - LockTableId = lockmethod; > - > - if (!(LockTableId)) > + LockTableId = LockMethodTableInit("LockTable", > + LockConflicts, > + lengthof(LockConflicts) - 1, > + maxBackends); > + if (!LockMethodIsValid(LockTableId)) > elog(ERROR, "could not initialize lock table"); > + Assert(LockTableId == DEFAULT_LOCKMETHOD); > > #ifdef USER_LOCKS > > @@ -92,11 +90,10 @@ > * Allocate another tableId for long-term locks > */ > LongTermTableId = LockMethodTableRename(LockTableId); > - if (!(LongTermTableId)) > + if (!LockMethodIsValid(LongTermTableId)) > elog(ERROR, "could not rename long-term lock table"); > + Assert(LongTermTableId == USER_LOCKMETHOD); > #endif > - > - return LockTableId; > } > > /* > diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c > --- ../base/src/backend/storage/lmgr/lock.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 > @@ -46,7 +46,7 @@ > #define NLOCKENTS(maxBackends) (max_locks_per_xact * (maxBackends)) > > > -static int WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +static int WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > static void LockCountMyLocks(SHMEM_OFFSET lockOffset, PGPROC *proc, > int *myHolding); > @@ -111,7 +111,7 @@ > "req(%d,%d,%d,%d,%d,%d,%d)=%d " > "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", > where, MAKE_OFFSET(lock), > - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, > + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, > lock->tag.objId.blkno, lock->grantMask, > lock->requested[1], lock->requested[2], lock->requested[3], > lock->requested[4], lock->requested[5], lock->requested[6], > @@ -150,19 +150,9 @@ > > > /* > - * These are to simplify/speed up some bit arithmetic. > - * > - * XXX is a fetch from a static array really faster than a shift? > - * Wouldn't bet on it... > + * map from lock method id to the lock table structure > */ > - > -static LOCKMASK BITS_OFF[MAX_LOCKMODES]; > -static LOCKMASK BITS_ON[MAX_LOCKMODES]; > - > -/* > - * map from lockmethod to the lock table structure > - */ > -static LOCKMETHODTABLE *LockMethodTable[MAX_LOCK_METHODS]; > +static LockMethod LockMethods[MAX_LOCK_METHODS]; > > static int NumLockMethods; > > @@ -173,28 +163,20 @@ > void > InitLocks(void) > { > - int i; > - int bit; > - > - bit = 1; > - for (i = 0; i < MAX_LOCKMODES; i++, bit <<= 1) > - { > - BITS_ON[i] = bit; > - BITS_OFF[i] = ~bit; > - } > + /* NOP */ > } > > > /* > * Fetch the lock method table associated with a given lock > */ > -LOCKMETHODTABLE * > +LockMethod > GetLocksMethodTable(LOCK *lock) > { > - LOCKMETHOD lockmethod = LOCK_LOCKMETHOD(*lock); > + LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*lock); > > - Assert(lockmethod > 0 && lockmethod < NumLockMethods); > - return LockMethodTable[lockmethod]; > + Assert(0 < lockmethodid && lockmethodid < NumLockMethods); > + return LockMethods[lockmethodid]; > } > > > @@ -205,7 +187,7 @@ > * Notes: just copying. Should only be called once. > */ > static void > -LockMethodInit(LOCKMETHODTABLE *lockMethodTable, > +LockMethodInit(LockMethod lockMethodTable, > LOCKMASK *conflictsP, > int numModes) > { > @@ -226,13 +208,13 @@ > * by the postmaster are inherited by each backend, so they must be in > * TopMemoryContext. > */ > -LOCKMETHOD > +LOCKMETHODID > LockMethodTableInit(char *tabName, > LOCKMASK *conflictsP, > int numModes, > int maxBackends) > { > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod newLockMethod; > char *shmemName; > HASHCTL info; > int hash_flags; > @@ -254,10 +236,10 @@ > > /* each lock table has a header in shared memory */ > sprintf(shmemName, "%s (lock method table)", tabName); > - lockMethodTable = (LOCKMETHODTABLE *) > - ShmemInitStruct(shmemName, sizeof(LOCKMETHODTABLE), &found); > + newLockMethod = (LockMethod) > + ShmemInitStruct(shmemName, sizeof(LockMethodData), &found); > > - if (!lockMethodTable) > + if (!newLockMethod) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* > @@ -275,15 +257,15 @@ > */ > if (!found) > { > - MemSet(lockMethodTable, 0, sizeof(LOCKMETHODTABLE)); > - lockMethodTable->masterLock = LockMgrLock; > - lockMethodTable->lockmethod = NumLockMethods; > + MemSet(newLockMethod, 0, sizeof(LockMethodData)); > + newLockMethod->masterLock = LockMgrLock; > + newLockMethod->lockmethodid = NumLockMethods; > } > > /* > * other modules refer to the lock table by a lockmethod ID > */ > - LockMethodTable[NumLockMethods] = lockMethodTable; > + LockMethods[NumLockMethods] = newLockMethod; > NumLockMethods++; > Assert(NumLockMethods <= MAX_LOCK_METHODS); > > @@ -297,15 +279,15 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (lock hash)", tabName); > - lockMethodTable->lockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->lockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->lockHash) > + if (!newLockMethod->lockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > - Assert(lockMethodTable->lockHash->hash == tag_hash); > + Assert(newLockMethod->lockHash->hash == tag_hash); > > /* > * allocate a hash table for PROCLOCK structs. This is used to store > @@ -317,23 +299,23 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (proclock hash)", tabName); > - lockMethodTable->proclockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->proclockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->proclockHash) > + if (!newLockMethod->proclockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* init data structures */ > - LockMethodInit(lockMethodTable, conflictsP, numModes); > + LockMethodInit(newLockMethod, conflictsP, numModes); > > LWLockRelease(LockMgrLock); > > pfree(shmemName); > > - return lockMethodTable->lockmethod; > + return newLockMethod->lockmethodid; > } > > /* > @@ -349,22 +331,22 @@ > * short term and long term locks, yet store them all in one hashtable. > */ > > -LOCKMETHOD > -LockMethodTableRename(LOCKMETHOD lockmethod) > +LOCKMETHODID > +LockMethodTableRename(LOCKMETHODID lockmethodid) > { > - LOCKMETHOD newLockMethod; > + LOCKMETHODID newLockMethodId; > > if (NumLockMethods >= MAX_LOCK_METHODS) > return INVALID_LOCKMETHOD; > - if (LockMethodTable[lockmethod] == INVALID_LOCKMETHOD) > + if (LockMethods[lockmethodid] == INVALID_LOCKMETHOD) > return INVALID_LOCKMETHOD; > > /* other modules refer to the lock table by a lockmethod ID */ > - newLockMethod = NumLockMethods; > + newLockMethodId = NumLockMethods; > NumLockMethods++; > > - LockMethodTable[newLockMethod] = LockMethodTable[lockmethod]; > - return newLockMethod; > + LockMethods[newLockMethodId] = LockMethods[lockmethodid]; > + return newLockMethodId; > } > > /* > @@ -412,7 +394,7 @@ > * > * normal lock user lock > * > - * lockmethod 1 2 > + * lockmethodid 1 2 > * tag.dbId database oid database oid > * tag.relId rel oid or 0 0 > * tag.objId block id lock id2 > @@ -429,7 +411,7 @@ > */ > > bool > -LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait) > { > PROCLOCK *proclock; > @@ -438,25 +420,25 @@ > bool found; > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int status; > int myHolding[MAX_LOCKMODES]; > int i; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockAcquire: user lock [%u] %s", > locktag->objId.blkno, lock_mode_names[lockmode]); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock table id: %d", lockmethod); > + elog(WARNING, "bad lock table id: %d", lockmethodid); > return FALSE; > } > > @@ -666,15 +648,12 @@ > * Construct bitmask of locks this process holds on this object. > */ > { > - int heldLocks = 0; > - int tmpMask; > + LOCKMASK heldLocks = 0; > > - for (i = 1, tmpMask = 2; > - i <= lockMethodTable->numLockModes; > - i++, tmpMask <<= 1) > + for (i = 1; i <= lockMethodTable->numLockModes; i++) > { > if (myHolding[i] > 0) > - heldLocks |= tmpMask; > + heldLocks |= LOCKBIT_ON(i); > } > MyProc->heldLocks = heldLocks; > } > @@ -682,7 +661,7 @@ > /* > * Sleep till someone wakes me up. > */ > - status = WaitOnLock(lockmethod, lockmode, lock, proclock); > + status = WaitOnLock(lockmethodid, lockmode, lock, proclock); > > /* > * NOTE: do not do any material change of state between here and > @@ -729,7 +708,7 @@ > * known. If NULL is passed then these values will be computed internally. > */ > int > -LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock, > @@ -737,9 +716,8 @@ > int *myHolding) /* myHolding[] array or NULL */ > { > int numLockModes = lockMethodTable->numLockModes; > - int bitmask; > - int i, > - tmpMask; > + LOCKMASK bitmask; > + int i; > int localHolding[MAX_LOCKMODES]; > > /* > @@ -772,11 +750,10 @@ > > /* Compute mask of lock types held by other processes */ > bitmask = 0; > - tmpMask = 2; > - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) > + for (i = 1; i <= numLockModes; i++) > { > if (lock->granted[i] != myHolding[i]) > - bitmask |= tmpMask; > + bitmask |= LOCKBIT_ON(i); > } > > /* > @@ -842,9 +819,9 @@ > { > lock->nGranted++; > lock->granted[lockmode]++; > - lock->grantMask |= BITS_ON[lockmode]; > + lock->grantMask |= LOCKBIT_ON(lockmode); > if (lock->granted[lockmode] == lock->requested[lockmode]) > - lock->waitMask &= BITS_OFF[lockmode]; > + lock->waitMask &= LOCKBIT_OFF(lockmode); > LOCK_PRINT("GrantLock", lock, lockmode); > Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); > Assert(lock->nGranted <= lock->nRequested); > @@ -862,14 +839,14 @@ > * The locktable's masterLock must be held at entry. > */ > static int > -WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock) > { > - LOCKMETHODTABLE *lockMethodTable = LockMethodTable[lockmethod]; > + LockMethod lockMethodTable = LockMethods[lockmethodid]; > char *new_status, > *old_status; > > - Assert(lockmethod < NumLockMethods); > + Assert(lockmethodid < NumLockMethods); > > LOCK_PRINT("WaitOnLock: sleeping on lock", lock, lockmode); > > @@ -957,7 +934,7 @@ > waitLock->requested[lockmode]--; > /* don't forget to clear waitMask bit if appropriate */ > if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) > - waitLock->waitMask &= BITS_OFF[lockmode]; > + waitLock->waitMask &= LOCKBIT_OFF(lockmode); > > /* Clean up the proc's own state */ > proc->waitLock = NULL; > @@ -968,7 +945,7 @@ > } > > /* > - * LockRelease -- look up 'locktag' in lock table 'lockmethod' and > + * LockRelease -- look up 'locktag' in lock table 'lockmethodid' and > * release one 'lockmode' lock on it. > * > * Side Effects: find any waiting processes that are now wakable, > @@ -978,27 +955,27 @@ > * come along and request the lock.) > */ > bool > -LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode) > { > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROCLOCK *proclock; > PROCLOCKTAG proclocktag; > HTAB *proclockTable; > bool wakeupNeeded = false; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockRelease: user lock tag [%u] %d", locktag->objId.blkno, lockmode); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > elog(WARNING, "lockMethodTable is null in LockRelease"); > @@ -1045,7 +1022,7 @@ > { > LWLockRelease(masterLock); > #ifdef USER_LOCKS > - if (lockmethod == USER_LOCKMETHOD) > + if (lockmethodid == USER_LOCKMETHOD) > elog(WARNING, "no lock with this tag"); > else > #endif > @@ -1083,7 +1060,7 @@ > if (lock->granted[lockmode] == 0) > { > /* change the conflict mask. No more of this lock type. */ > - lock->grantMask &= BITS_OFF[lockmode]; > + lock->grantMask &= LOCKBIT_OFF(lockmode); > } > > LOCK_PRINT("LockRelease: updated", lock, lockmode); > @@ -1173,29 +1150,29 @@ > * specified XID are released. > */ > bool > -LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid) > { > SHM_QUEUE *procHolders = &(proc->procHolders); > PROCLOCK *proclock; > PROCLOCK *nextHolder; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int i, > numLockModes; > LOCK *lock; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll: lockmethod=%d, pid=%d", > - lockmethod, proc->pid); > + lockmethodid, proc->pid); > #endif > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock method: %d", lockmethod); > + elog(WARNING, "bad lock method: %d", lockmethodid); > return FALSE; > } > > @@ -1220,7 +1197,7 @@ > lock = (LOCK *) MAKE_PTR(proclock->tag.lock); > > /* Ignore items that are not of the lockmethod to be removed */ > - if (LOCK_LOCKMETHOD(*lock) != lockmethod) > + if (LOCK_LOCKMETHOD(*lock) != lockmethodid) > goto next_item; > > /* If not allxids, ignore items that are of the wrong xid */ > @@ -1249,7 +1226,7 @@ > lock->granted[i] -= proclock->holding[i]; > Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); > if (lock->granted[i] == 0) > - lock->grantMask &= BITS_OFF[i]; > + lock->grantMask &= LOCKBIT_OFF(i); > > /* > * Read comments in LockRelease > @@ -1331,7 +1308,7 @@ > LWLockRelease(masterLock); > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll done"); > #endif > > @@ -1346,7 +1323,7 @@ > > size += MAXALIGN(sizeof(PROC_HDR)); /* ProcGlobal */ > size += maxBackends * MAXALIGN(sizeof(PGPROC)); /* each MyProc */ > - size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LOCKMETHODTABLE)); /* each lockMethodTable */ > + size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LockMethodData)); /* each lock method */ > > /* lockHash table */ > size += hash_estimate_size(max_table_size, sizeof(LOCK)); > @@ -1390,7 +1367,7 @@ > > LWLockAcquire(LockMgrLock, LW_EXCLUSIVE); > > - proclockTable = LockMethodTable[DEFAULT_LOCKMETHOD]->proclockHash; > + proclockTable = LockMethods[DEFAULT_LOCKMETHOD]->proclockHash; > > data->nelements = i = proclockTable->hctl->nentries; > > @@ -1446,8 +1423,8 @@ > SHM_QUEUE *procHolders; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > > proc = MyProc; > if (proc == NULL) > @@ -1455,8 +1432,8 @@ > > procHolders = &proc->procHolders; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > @@ -1489,8 +1466,8 @@ > PGPROC *proc; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > HTAB *proclockTable; > HASH_SEQ_STATUS status; > > @@ -1498,8 +1475,8 @@ > if (proc == NULL) > return; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c > --- ../base/src/backend/storage/lmgr/proc.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 > @@ -524,14 +524,14 @@ > * semaphore is normally zero, so when we try to acquire it, we sleep. > */ > int > -ProcSleep(LOCKMETHODTABLE *lockMethodTable, > +ProcSleep(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock) > { > LWLockId masterLock = lockMethodTable->masterLock; > PROC_QUEUE *waitQueue = &(lock->waitProcs); > - int myHeldLocks = MyProc->heldLocks; > + LOCKMASK myHeldLocks = MyProc->heldLocks; > bool early_deadlock = false; > PGPROC *proc; > int i; > @@ -556,7 +556,7 @@ > */ > if (myHeldLocks != 0) > { > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); > for (i = 0; i < waitQueue->size; i++) > @@ -596,7 +596,7 @@ > break; > } > /* Nope, so advance to next waiter */ > - aheadRequests |= (1 << proc->waitLockMode); > + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > > @@ -618,7 +618,7 @@ > SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); > waitQueue->size++; > > - lock->waitMask |= (1 << lockmode); > + lock->waitMask |= LOCKBIT_ON(lockmode); > > /* Set up wait information in PGPROC object, too */ > MyProc->waitLock = lock; > @@ -750,12 +750,12 @@ > * for lock, waken any that are no longer blocked. > */ > void > -ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock) > +ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) > { > PROC_QUEUE *waitQueue = &(lock->waitProcs); > int queue_size = waitQueue->size; > PGPROC *proc; > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > Assert(queue_size >= 0); > > @@ -796,7 +796,7 @@ > * Cannot wake this guy. Remember his request for later > * checks. > */ > - aheadRequests |= (1 << lockmode); > + aheadRequests |= LOCKBIT_ON(lockmode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > } > diff -ruN ../base/src/include/storage/lmgr.h src/include/storage/lmgr.h > --- ../base/src/include/storage/lmgr.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lmgr.h 2003-09-06 18:02:43.000000000 +0200 > @@ -40,10 +40,7 @@ > * so increase that if you want to add more modes. > */ > > -extern LOCKMETHOD LockTableId; > - > - > -extern LOCKMETHOD InitLockTable(int maxBackends); > +extern void InitLockTable(int maxBackends); > extern void RelationInitLockInfo(Relation relation); > > /* Lock a relation */ > diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h > --- ../base/src/include/storage/lock.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 > @@ -42,22 +42,23 @@ > > > typedef int LOCKMASK; > - > typedef int LOCKMODE; > -typedef int LOCKMETHOD; > - > /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ > #define MAX_LOCKMODES 10 > > +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) > +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) > + > +typedef uint16 LOCKMETHODID; > /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ > #define MAX_LOCK_METHODS 3 > > -#define INVALID_TABLEID 0 > - > -#define INVALID_LOCKMETHOD INVALID_TABLEID > +#define INVALID_LOCKMETHOD 0 > #define DEFAULT_LOCKMETHOD 1 > #define USER_LOCKMETHOD 2 > > +#define LockMethodIsValid(lockmethodid) ((lockmethodid) != INVALID_LOCKMETHOD) > + > /* > * There is normally only one lock method, the default one. > * If user locks are enabled, an additional lock method is present. > @@ -83,15 +84,16 @@ > * masterLock -- synchronizes access to the table > * > */ > -typedef struct LOCKMETHODTABLE > +typedef struct LockMethodData > { > - HTAB *lockHash; > - HTAB *proclockHash; > - LOCKMETHOD lockmethod; > - int numLockModes; > - int conflictTab[MAX_LOCKMODES]; > - LWLockId masterLock; > -} LOCKMETHODTABLE; > + HTAB *lockHash; > + HTAB *proclockHash; > + LOCKMETHODID lockmethodid; > + int numLockModes; > + LOCKMASK conflictTab[MAX_LOCKMODES]; > + LWLockId masterLock; > +} LockMethodData; > +typedef LockMethodData *LockMethod; > > > /* > @@ -115,7 +117,7 @@ > */ > OffsetNumber offnum; > > - uint16 lockmethod; /* needed by userlocks */ > + LOCKMETHODID lockmethodid; /* needed by userlocks */ > } LOCKTAG; > > > @@ -139,8 +141,8 @@ > LOCKTAG tag; /* unique identifier of lockable object */ > > /* data */ > - int grantMask; /* bitmask for lock types already granted */ > - int waitMask; /* bitmask for lock types awaited */ > + LOCKMASK grantMask; /* bitmask for lock types already granted */ > + LOCKMASK waitMask; /* bitmask for lock types awaited */ > SHM_QUEUE lockHolders; /* list of PROCLOCK objects assoc. with > * lock */ > PROC_QUEUE waitProcs; /* list of PGPROC objects waiting on lock */ > @@ -151,7 +153,7 @@ > int nGranted; /* total of granted[] array */ > } LOCK; > > -#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethod) > +#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethodid) > > > /* > @@ -204,7 +206,7 @@ > } PROCLOCK; > > #define PROCLOCK_LOCKMETHOD(proclock) \ > - (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethod) > + (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethodid) > > /* > * This struct holds information passed from lmgr internals to the lock > @@ -227,17 +229,17 @@ > * function prototypes > */ > extern void InitLocks(void); > -extern LOCKMETHODTABLE *GetLocksMethodTable(LOCK *lock); > -extern LOCKMETHOD LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > +extern LockMethod GetLocksMethodTable(LOCK *lock); > +extern LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > int numModes, int maxBackends); > -extern LOCKMETHOD LockMethodTableRename(LOCKMETHOD lockmethod); > -extern bool LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid); > +extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait); > -extern bool LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode); > -extern bool LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +extern bool LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid); > -extern int LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +extern int LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock, PGPROC *proc, > int *myHolding); > diff -ruN ../base/src/include/storage/proc.h src/include/storage/proc.h > --- ../base/src/include/storage/proc.h 2003-08-04 04:40:15.000000000 +0200 > +++ src/include/storage/proc.h 2003-09-06 22:45:54.000000000 +0200 > @@ -101,10 +101,10 @@ > extern void ProcReleaseLocks(bool isCommit); > > extern void ProcQueueInit(PROC_QUEUE *queue); > -extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode, > +extern int ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > extern PGPROC *ProcWakeup(PGPROC *proc, int errType); > -extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock); > +extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); > extern bool LockWaitCancel(void); > > extern void ProcWaitForSignal(void); > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Manfred, can I get a description for this patch? Thanks. --------------------------------------------------------------------------- Manfred Koizar wrote: > On Mon, 8 Sep 2003 00:51:45 -0400 (EDT), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > >This has been saved for the 7.5 release: > > > > http:/momjian.postgresql.org/cgi-bin/pgpatches2 > > Here is the combined patch. Prior patches in this thread are hereby > obsolete. > > Servus > Manfred > diff -ruN ../base/src/backend/storage/ipc/ipci.c src/backend/storage/ipc/ipci.c > --- ../base/src/backend/storage/ipc/ipci.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/ipc/ipci.c 2003-09-06 16:53:54.000000000 +0200 > @@ -111,8 +111,7 @@ > * Set up lock manager > */ > InitLocks(); > - if (InitLockTable(maxBackends) == INVALID_TABLEID) > - elog(FATAL, "could not create the lock table"); > + InitLockTable(maxBackends); > > /* > * Set up process table > diff -ruN ../base/src/backend/storage/lmgr/deadlock.c src/backend/storage/lmgr/deadlock.c > --- ../base/src/backend/storage/lmgr/deadlock.c 2003-08-08 23:42:00.000000000 +0200 > +++ src/backend/storage/lmgr/deadlock.c 2003-09-06 22:25:46.000000000 +0200 > @@ -428,7 +428,7 @@ > LOCK *lock; > PROCLOCK *proclock; > SHM_QUEUE *lockHolders; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROC_QUEUE *waitQueue; > int queue_size; > int conflictMask; > diff -ruN ../base/src/backend/storage/lmgr/lmgr.c src/backend/storage/lmgr/lmgr.c > --- ../base/src/backend/storage/lmgr/lmgr.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lmgr.c 2003-09-06 18:10:48.000000000 +0200 > @@ -65,26 +65,24 @@ > > }; > > -LOCKMETHOD LockTableId = (LOCKMETHOD) NULL; > -LOCKMETHOD LongTermTableId = (LOCKMETHOD) NULL; > +static LOCKMETHODID LockTableId = INVALID_LOCKMETHOD; > > /* > * Create the lock table described by LockConflicts > */ > -LOCKMETHOD > +void > InitLockTable(int maxBackends) > { > - int lockmethod; > + LOCKMETHODID LongTermTableId; > > /* number of lock modes is lengthof()-1 because of dummy zero */ > - lockmethod = LockMethodTableInit("LockTable", > - LockConflicts, > - lengthof(LockConflicts) - 1, > - maxBackends); > - LockTableId = lockmethod; > - > - if (!(LockTableId)) > + LockTableId = LockMethodTableInit("LockTable", > + LockConflicts, > + lengthof(LockConflicts) - 1, > + maxBackends); > + if (!LockMethodIsValid(LockTableId)) > elog(ERROR, "could not initialize lock table"); > + Assert(LockTableId == DEFAULT_LOCKMETHOD); > > #ifdef USER_LOCKS > > @@ -92,11 +90,10 @@ > * Allocate another tableId for long-term locks > */ > LongTermTableId = LockMethodTableRename(LockTableId); > - if (!(LongTermTableId)) > + if (!LockMethodIsValid(LongTermTableId)) > elog(ERROR, "could not rename long-term lock table"); > + Assert(LongTermTableId == USER_LOCKMETHOD); > #endif > - > - return LockTableId; > } > > /* > diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c > --- ../base/src/backend/storage/lmgr/lock.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 > @@ -46,7 +46,7 @@ > #define NLOCKENTS(maxBackends) (max_locks_per_xact * (maxBackends)) > > > -static int WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +static int WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > static void LockCountMyLocks(SHMEM_OFFSET lockOffset, PGPROC *proc, > int *myHolding); > @@ -111,7 +111,7 @@ > "req(%d,%d,%d,%d,%d,%d,%d)=%d " > "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", > where, MAKE_OFFSET(lock), > - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, > + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, > lock->tag.objId.blkno, lock->grantMask, > lock->requested[1], lock->requested[2], lock->requested[3], > lock->requested[4], lock->requested[5], lock->requested[6], > @@ -150,19 +150,9 @@ > > > /* > - * These are to simplify/speed up some bit arithmetic. > - * > - * XXX is a fetch from a static array really faster than a shift? > - * Wouldn't bet on it... > + * map from lock method id to the lock table structure > */ > - > -static LOCKMASK BITS_OFF[MAX_LOCKMODES]; > -static LOCKMASK BITS_ON[MAX_LOCKMODES]; > - > -/* > - * map from lockmethod to the lock table structure > - */ > -static LOCKMETHODTABLE *LockMethodTable[MAX_LOCK_METHODS]; > +static LockMethod LockMethods[MAX_LOCK_METHODS]; > > static int NumLockMethods; > > @@ -173,28 +163,20 @@ > void > InitLocks(void) > { > - int i; > - int bit; > - > - bit = 1; > - for (i = 0; i < MAX_LOCKMODES; i++, bit <<= 1) > - { > - BITS_ON[i] = bit; > - BITS_OFF[i] = ~bit; > - } > + /* NOP */ > } > > > /* > * Fetch the lock method table associated with a given lock > */ > -LOCKMETHODTABLE * > +LockMethod > GetLocksMethodTable(LOCK *lock) > { > - LOCKMETHOD lockmethod = LOCK_LOCKMETHOD(*lock); > + LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*lock); > > - Assert(lockmethod > 0 && lockmethod < NumLockMethods); > - return LockMethodTable[lockmethod]; > + Assert(0 < lockmethodid && lockmethodid < NumLockMethods); > + return LockMethods[lockmethodid]; > } > > > @@ -205,7 +187,7 @@ > * Notes: just copying. Should only be called once. > */ > static void > -LockMethodInit(LOCKMETHODTABLE *lockMethodTable, > +LockMethodInit(LockMethod lockMethodTable, > LOCKMASK *conflictsP, > int numModes) > { > @@ -226,13 +208,13 @@ > * by the postmaster are inherited by each backend, so they must be in > * TopMemoryContext. > */ > -LOCKMETHOD > +LOCKMETHODID > LockMethodTableInit(char *tabName, > LOCKMASK *conflictsP, > int numModes, > int maxBackends) > { > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod newLockMethod; > char *shmemName; > HASHCTL info; > int hash_flags; > @@ -254,10 +236,10 @@ > > /* each lock table has a header in shared memory */ > sprintf(shmemName, "%s (lock method table)", tabName); > - lockMethodTable = (LOCKMETHODTABLE *) > - ShmemInitStruct(shmemName, sizeof(LOCKMETHODTABLE), &found); > + newLockMethod = (LockMethod) > + ShmemInitStruct(shmemName, sizeof(LockMethodData), &found); > > - if (!lockMethodTable) > + if (!newLockMethod) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* > @@ -275,15 +257,15 @@ > */ > if (!found) > { > - MemSet(lockMethodTable, 0, sizeof(LOCKMETHODTABLE)); > - lockMethodTable->masterLock = LockMgrLock; > - lockMethodTable->lockmethod = NumLockMethods; > + MemSet(newLockMethod, 0, sizeof(LockMethodData)); > + newLockMethod->masterLock = LockMgrLock; > + newLockMethod->lockmethodid = NumLockMethods; > } > > /* > * other modules refer to the lock table by a lockmethod ID > */ > - LockMethodTable[NumLockMethods] = lockMethodTable; > + LockMethods[NumLockMethods] = newLockMethod; > NumLockMethods++; > Assert(NumLockMethods <= MAX_LOCK_METHODS); > > @@ -297,15 +279,15 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (lock hash)", tabName); > - lockMethodTable->lockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->lockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->lockHash) > + if (!newLockMethod->lockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > - Assert(lockMethodTable->lockHash->hash == tag_hash); > + Assert(newLockMethod->lockHash->hash == tag_hash); > > /* > * allocate a hash table for PROCLOCK structs. This is used to store > @@ -317,23 +299,23 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (proclock hash)", tabName); > - lockMethodTable->proclockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->proclockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->proclockHash) > + if (!newLockMethod->proclockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* init data structures */ > - LockMethodInit(lockMethodTable, conflictsP, numModes); > + LockMethodInit(newLockMethod, conflictsP, numModes); > > LWLockRelease(LockMgrLock); > > pfree(shmemName); > > - return lockMethodTable->lockmethod; > + return newLockMethod->lockmethodid; > } > > /* > @@ -349,22 +331,22 @@ > * short term and long term locks, yet store them all in one hashtable. > */ > > -LOCKMETHOD > -LockMethodTableRename(LOCKMETHOD lockmethod) > +LOCKMETHODID > +LockMethodTableRename(LOCKMETHODID lockmethodid) > { > - LOCKMETHOD newLockMethod; > + LOCKMETHODID newLockMethodId; > > if (NumLockMethods >= MAX_LOCK_METHODS) > return INVALID_LOCKMETHOD; > - if (LockMethodTable[lockmethod] == INVALID_LOCKMETHOD) > + if (LockMethods[lockmethodid] == INVALID_LOCKMETHOD) > return INVALID_LOCKMETHOD; > > /* other modules refer to the lock table by a lockmethod ID */ > - newLockMethod = NumLockMethods; > + newLockMethodId = NumLockMethods; > NumLockMethods++; > > - LockMethodTable[newLockMethod] = LockMethodTable[lockmethod]; > - return newLockMethod; > + LockMethods[newLockMethodId] = LockMethods[lockmethodid]; > + return newLockMethodId; > } > > /* > @@ -412,7 +394,7 @@ > * > * normal lock user lock > * > - * lockmethod 1 2 > + * lockmethodid 1 2 > * tag.dbId database oid database oid > * tag.relId rel oid or 0 0 > * tag.objId block id lock id2 > @@ -429,7 +411,7 @@ > */ > > bool > -LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait) > { > PROCLOCK *proclock; > @@ -438,25 +420,25 @@ > bool found; > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int status; > int myHolding[MAX_LOCKMODES]; > int i; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockAcquire: user lock [%u] %s", > locktag->objId.blkno, lock_mode_names[lockmode]); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock table id: %d", lockmethod); > + elog(WARNING, "bad lock table id: %d", lockmethodid); > return FALSE; > } > > @@ -666,15 +648,12 @@ > * Construct bitmask of locks this process holds on this object. > */ > { > - int heldLocks = 0; > - int tmpMask; > + LOCKMASK heldLocks = 0; > > - for (i = 1, tmpMask = 2; > - i <= lockMethodTable->numLockModes; > - i++, tmpMask <<= 1) > + for (i = 1; i <= lockMethodTable->numLockModes; i++) > { > if (myHolding[i] > 0) > - heldLocks |= tmpMask; > + heldLocks |= LOCKBIT_ON(i); > } > MyProc->heldLocks = heldLocks; > } > @@ -682,7 +661,7 @@ > /* > * Sleep till someone wakes me up. > */ > - status = WaitOnLock(lockmethod, lockmode, lock, proclock); > + status = WaitOnLock(lockmethodid, lockmode, lock, proclock); > > /* > * NOTE: do not do any material change of state between here and > @@ -729,7 +708,7 @@ > * known. If NULL is passed then these values will be computed internally. > */ > int > -LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock, > @@ -737,9 +716,8 @@ > int *myHolding) /* myHolding[] array or NULL */ > { > int numLockModes = lockMethodTable->numLockModes; > - int bitmask; > - int i, > - tmpMask; > + LOCKMASK bitmask; > + int i; > int localHolding[MAX_LOCKMODES]; > > /* > @@ -772,11 +750,10 @@ > > /* Compute mask of lock types held by other processes */ > bitmask = 0; > - tmpMask = 2; > - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) > + for (i = 1; i <= numLockModes; i++) > { > if (lock->granted[i] != myHolding[i]) > - bitmask |= tmpMask; > + bitmask |= LOCKBIT_ON(i); > } > > /* > @@ -842,9 +819,9 @@ > { > lock->nGranted++; > lock->granted[lockmode]++; > - lock->grantMask |= BITS_ON[lockmode]; > + lock->grantMask |= LOCKBIT_ON(lockmode); > if (lock->granted[lockmode] == lock->requested[lockmode]) > - lock->waitMask &= BITS_OFF[lockmode]; > + lock->waitMask &= LOCKBIT_OFF(lockmode); > LOCK_PRINT("GrantLock", lock, lockmode); > Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); > Assert(lock->nGranted <= lock->nRequested); > @@ -862,14 +839,14 @@ > * The locktable's masterLock must be held at entry. > */ > static int > -WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock) > { > - LOCKMETHODTABLE *lockMethodTable = LockMethodTable[lockmethod]; > + LockMethod lockMethodTable = LockMethods[lockmethodid]; > char *new_status, > *old_status; > > - Assert(lockmethod < NumLockMethods); > + Assert(lockmethodid < NumLockMethods); > > LOCK_PRINT("WaitOnLock: sleeping on lock", lock, lockmode); > > @@ -957,7 +934,7 @@ > waitLock->requested[lockmode]--; > /* don't forget to clear waitMask bit if appropriate */ > if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) > - waitLock->waitMask &= BITS_OFF[lockmode]; > + waitLock->waitMask &= LOCKBIT_OFF(lockmode); > > /* Clean up the proc's own state */ > proc->waitLock = NULL; > @@ -968,7 +945,7 @@ > } > > /* > - * LockRelease -- look up 'locktag' in lock table 'lockmethod' and > + * LockRelease -- look up 'locktag' in lock table 'lockmethodid' and > * release one 'lockmode' lock on it. > * > * Side Effects: find any waiting processes that are now wakable, > @@ -978,27 +955,27 @@ > * come along and request the lock.) > */ > bool > -LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode) > { > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROCLOCK *proclock; > PROCLOCKTAG proclocktag; > HTAB *proclockTable; > bool wakeupNeeded = false; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockRelease: user lock tag [%u] %d", locktag->objId.blkno, lockmode); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > elog(WARNING, "lockMethodTable is null in LockRelease"); > @@ -1045,7 +1022,7 @@ > { > LWLockRelease(masterLock); > #ifdef USER_LOCKS > - if (lockmethod == USER_LOCKMETHOD) > + if (lockmethodid == USER_LOCKMETHOD) > elog(WARNING, "no lock with this tag"); > else > #endif > @@ -1083,7 +1060,7 @@ > if (lock->granted[lockmode] == 0) > { > /* change the conflict mask. No more of this lock type. */ > - lock->grantMask &= BITS_OFF[lockmode]; > + lock->grantMask &= LOCKBIT_OFF(lockmode); > } > > LOCK_PRINT("LockRelease: updated", lock, lockmode); > @@ -1173,29 +1150,29 @@ > * specified XID are released. > */ > bool > -LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid) > { > SHM_QUEUE *procHolders = &(proc->procHolders); > PROCLOCK *proclock; > PROCLOCK *nextHolder; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int i, > numLockModes; > LOCK *lock; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll: lockmethod=%d, pid=%d", > - lockmethod, proc->pid); > + lockmethodid, proc->pid); > #endif > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock method: %d", lockmethod); > + elog(WARNING, "bad lock method: %d", lockmethodid); > return FALSE; > } > > @@ -1220,7 +1197,7 @@ > lock = (LOCK *) MAKE_PTR(proclock->tag.lock); > > /* Ignore items that are not of the lockmethod to be removed */ > - if (LOCK_LOCKMETHOD(*lock) != lockmethod) > + if (LOCK_LOCKMETHOD(*lock) != lockmethodid) > goto next_item; > > /* If not allxids, ignore items that are of the wrong xid */ > @@ -1249,7 +1226,7 @@ > lock->granted[i] -= proclock->holding[i]; > Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); > if (lock->granted[i] == 0) > - lock->grantMask &= BITS_OFF[i]; > + lock->grantMask &= LOCKBIT_OFF(i); > > /* > * Read comments in LockRelease > @@ -1331,7 +1308,7 @@ > LWLockRelease(masterLock); > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll done"); > #endif > > @@ -1346,7 +1323,7 @@ > > size += MAXALIGN(sizeof(PROC_HDR)); /* ProcGlobal */ > size += maxBackends * MAXALIGN(sizeof(PGPROC)); /* each MyProc */ > - size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LOCKMETHODTABLE)); /* each lockMethodTable */ > + size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LockMethodData)); /* each lock method */ > > /* lockHash table */ > size += hash_estimate_size(max_table_size, sizeof(LOCK)); > @@ -1390,7 +1367,7 @@ > > LWLockAcquire(LockMgrLock, LW_EXCLUSIVE); > > - proclockTable = LockMethodTable[DEFAULT_LOCKMETHOD]->proclockHash; > + proclockTable = LockMethods[DEFAULT_LOCKMETHOD]->proclockHash; > > data->nelements = i = proclockTable->hctl->nentries; > > @@ -1446,8 +1423,8 @@ > SHM_QUEUE *procHolders; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > > proc = MyProc; > if (proc == NULL) > @@ -1455,8 +1432,8 @@ > > procHolders = &proc->procHolders; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > @@ -1489,8 +1466,8 @@ > PGPROC *proc; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > HTAB *proclockTable; > HASH_SEQ_STATUS status; > > @@ -1498,8 +1475,8 @@ > if (proc == NULL) > return; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c > --- ../base/src/backend/storage/lmgr/proc.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 > @@ -524,14 +524,14 @@ > * semaphore is normally zero, so when we try to acquire it, we sleep. > */ > int > -ProcSleep(LOCKMETHODTABLE *lockMethodTable, > +ProcSleep(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock) > { > LWLockId masterLock = lockMethodTable->masterLock; > PROC_QUEUE *waitQueue = &(lock->waitProcs); > - int myHeldLocks = MyProc->heldLocks; > + LOCKMASK myHeldLocks = MyProc->heldLocks; > bool early_deadlock = false; > PGPROC *proc; > int i; > @@ -556,7 +556,7 @@ > */ > if (myHeldLocks != 0) > { > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); > for (i = 0; i < waitQueue->size; i++) > @@ -596,7 +596,7 @@ > break; > } > /* Nope, so advance to next waiter */ > - aheadRequests |= (1 << proc->waitLockMode); > + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > > @@ -618,7 +618,7 @@ > SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); > waitQueue->size++; > > - lock->waitMask |= (1 << lockmode); > + lock->waitMask |= LOCKBIT_ON(lockmode); > > /* Set up wait information in PGPROC object, too */ > MyProc->waitLock = lock; > @@ -750,12 +750,12 @@ > * for lock, waken any that are no longer blocked. > */ > void > -ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock) > +ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) > { > PROC_QUEUE *waitQueue = &(lock->waitProcs); > int queue_size = waitQueue->size; > PGPROC *proc; > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > Assert(queue_size >= 0); > > @@ -796,7 +796,7 @@ > * Cannot wake this guy. Remember his request for later > * checks. > */ > - aheadRequests |= (1 << lockmode); > + aheadRequests |= LOCKBIT_ON(lockmode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > } > diff -ruN ../base/src/include/storage/lmgr.h src/include/storage/lmgr.h > --- ../base/src/include/storage/lmgr.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lmgr.h 2003-09-06 18:02:43.000000000 +0200 > @@ -40,10 +40,7 @@ > * so increase that if you want to add more modes. > */ > > -extern LOCKMETHOD LockTableId; > - > - > -extern LOCKMETHOD InitLockTable(int maxBackends); > +extern void InitLockTable(int maxBackends); > extern void RelationInitLockInfo(Relation relation); > > /* Lock a relation */ > diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h > --- ../base/src/include/storage/lock.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 > @@ -42,22 +42,23 @@ > > > typedef int LOCKMASK; > - > typedef int LOCKMODE; > -typedef int LOCKMETHOD; > - > /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ > #define MAX_LOCKMODES 10 > > +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) > +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) > + > +typedef uint16 LOCKMETHODID; > /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ > #define MAX_LOCK_METHODS 3 > > -#define INVALID_TABLEID 0 > - > -#define INVALID_LOCKMETHOD INVALID_TABLEID > +#define INVALID_LOCKMETHOD 0 > #define DEFAULT_LOCKMETHOD 1 > #define USER_LOCKMETHOD 2 > > +#define LockMethodIsValid(lockmethodid) ((lockmethodid) != INVALID_LOCKMETHOD) > + > /* > * There is normally only one lock method, the default one. > * If user locks are enabled, an additional lock method is present. > @@ -83,15 +84,16 @@ > * masterLock -- synchronizes access to the table > * > */ > -typedef struct LOCKMETHODTABLE > +typedef struct LockMethodData > { > - HTAB *lockHash; > - HTAB *proclockHash; > - LOCKMETHOD lockmethod; > - int numLockModes; > - int conflictTab[MAX_LOCKMODES]; > - LWLockId masterLock; > -} LOCKMETHODTABLE; > + HTAB *lockHash; > + HTAB *proclockHash; > + LOCKMETHODID lockmethodid; > + int numLockModes; > + LOCKMASK conflictTab[MAX_LOCKMODES]; > + LWLockId masterLock; > +} LockMethodData; > +typedef LockMethodData *LockMethod; > > > /* > @@ -115,7 +117,7 @@ > */ > OffsetNumber offnum; > > - uint16 lockmethod; /* needed by userlocks */ > + LOCKMETHODID lockmethodid; /* needed by userlocks */ > } LOCKTAG; > > > @@ -139,8 +141,8 @@ > LOCKTAG tag; /* unique identifier of lockable object */ > > /* data */ > - int grantMask; /* bitmask for lock types already granted */ > - int waitMask; /* bitmask for lock types awaited */ > + LOCKMASK grantMask; /* bitmask for lock types already granted */ > + LOCKMASK waitMask; /* bitmask for lock types awaited */ > SHM_QUEUE lockHolders; /* list of PROCLOCK objects assoc. with > * lock */ > PROC_QUEUE waitProcs; /* list of PGPROC objects waiting on lock */ > @@ -151,7 +153,7 @@ > int nGranted; /* total of granted[] array */ > } LOCK; > > -#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethod) > +#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethodid) > > > /* > @@ -204,7 +206,7 @@ > } PROCLOCK; > > #define PROCLOCK_LOCKMETHOD(proclock) \ > - (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethod) > + (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethodid) > > /* > * This struct holds information passed from lmgr internals to the lock > @@ -227,17 +229,17 @@ > * function prototypes > */ > extern void InitLocks(void); > -extern LOCKMETHODTABLE *GetLocksMethodTable(LOCK *lock); > -extern LOCKMETHOD LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > +extern LockMethod GetLocksMethodTable(LOCK *lock); > +extern LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > int numModes, int maxBackends); > -extern LOCKMETHOD LockMethodTableRename(LOCKMETHOD lockmethod); > -extern bool LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid); > +extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait); > -extern bool LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode); > -extern bool LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +extern bool LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid); > -extern int LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +extern int LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock, PGPROC *proc, > int *myHolding); > diff -ruN ../base/src/include/storage/proc.h src/include/storage/proc.h > --- ../base/src/include/storage/proc.h 2003-08-04 04:40:15.000000000 +0200 > +++ src/include/storage/proc.h 2003-09-06 22:45:54.000000000 +0200 > @@ -101,10 +101,10 @@ > extern void ProcReleaseLocks(bool isCommit); > > extern void ProcQueueInit(PROC_QUEUE *queue); > -extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode, > +extern int ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > extern PGPROC *ProcWakeup(PGPROC *proc, int errType); > -extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock); > +extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); > extern bool LockWaitCancel(void); > > extern void ProcWaitForSignal(void); -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
On Mon, 8 Sep 2003 13:02:11 -0400 (EDT), Bruce Momjian <pgman@candle.pha.pa.us> wrote: > >Manfred, can I get a description for this patch? Thanks. Try to reduce confusion about what is a lock method identifier, a lock method control structure, or a table of control structures. . Use type LOCKMASK where an int is not a counter. . Get rid of INVALID_TABLEID, use INVALID_LOCKMETHOD instead. . Use INVALID_LOCKMETHOD instead of (LOCKMETHOD) NULL, because LOCKMETHOD is not a pointer. . Define and use macro LockMethodIsValid. . Rename LOCKMETHOD to LOCKMETHODID. . Remove global variable LongTermTableId in lmgr.c, because it is never used. . Make LockTableId static in lmgr.c, because it is used nowhere else. Why not remove it and use DEFAULT_LOCKMETHOD? . Rename the lock method control structure from LOCKMETHODTABLE to LockMethodData. Introduce a pointer type named LockMethod. . Remove elog(FATAL) after InitLockTable() call in CreateSharedMemoryAndSemaphores(), because if something goes wrong, there is elog(FATAL) in LockMethodTableInit(), and if this doesn't help, an elog(ERROR) in InitLockTable() is promoted to FATAL. . Make InitLockTable() void, because its only caller does not use its return value any more. . Rename variables in lock.c to avoid statements like LockMethodTable[NumLockMethods] = lockMethodTable; lockMethodTable = LockMethodTable[lockmethod]; . Change LOCKMETHODID type to uint16 to fit into struct LOCKTAG. . Remove static variables BITS_OFF and BITS_ON from lock.c, because I agree to this doubt: * XXX is a fetch from a static array really faster than a shift? . Define and use macros LOCKBIT_ON/OFF. ====================== All 93 tests passed. ====================== Servus Manfred
This has been saved for the 7.5 release: http:/momjian.postgresql.org/cgi-bin/pgpatches2 --------------------------------------------------------------------------- Manfred Koizar wrote: > On Mon, 8 Sep 2003 13:02:11 -0400 (EDT), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > >Manfred, can I get a description for this patch? Thanks. > > Try to reduce confusion about what is a lock method identifier, a lock > method control structure, or a table of control structures. > > . Use type LOCKMASK where an int is not a counter. > > . Get rid of INVALID_TABLEID, use INVALID_LOCKMETHOD instead. > > . Use INVALID_LOCKMETHOD instead of (LOCKMETHOD) NULL, because > LOCKMETHOD is not a pointer. > > . Define and use macro LockMethodIsValid. > > . Rename LOCKMETHOD to LOCKMETHODID. > > . Remove global variable LongTermTableId in lmgr.c, because it is > never used. > > . Make LockTableId static in lmgr.c, because it is used nowhere else. > Why not remove it and use DEFAULT_LOCKMETHOD? > > . Rename the lock method control structure from LOCKMETHODTABLE to > LockMethodData. Introduce a pointer type named LockMethod. > > . Remove elog(FATAL) after InitLockTable() call in > CreateSharedMemoryAndSemaphores(), because if something goes wrong, > there is elog(FATAL) in LockMethodTableInit(), and if this doesn't > help, an elog(ERROR) in InitLockTable() is promoted to FATAL. > > . Make InitLockTable() void, because its only caller does not use its > return value any more. > > . Rename variables in lock.c to avoid statements like > LockMethodTable[NumLockMethods] = lockMethodTable; > lockMethodTable = LockMethodTable[lockmethod]; > > . Change LOCKMETHODID type to uint16 to fit into struct LOCKTAG. > > . Remove static variables BITS_OFF and BITS_ON from lock.c, because > I agree to this doubt: > * XXX is a fetch from a static array really faster than a shift? > > . Define and use macros LOCKBIT_ON/OFF. > > ====================== > All 93 tests passed. > ====================== > > Servus > Manfred > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches I will try to apply it within the next 48 hours. --------------------------------------------------------------------------- Manfred Koizar wrote: > On Mon, 8 Sep 2003 00:51:45 -0400 (EDT), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > >This has been saved for the 7.5 release: > > > > http:/momjian.postgresql.org/cgi-bin/pgpatches2 > > Here is the combined patch. Prior patches in this thread are hereby > obsolete. > > Servus > Manfred > diff -ruN ../base/src/backend/storage/ipc/ipci.c src/backend/storage/ipc/ipci.c > --- ../base/src/backend/storage/ipc/ipci.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/ipc/ipci.c 2003-09-06 16:53:54.000000000 +0200 > @@ -111,8 +111,7 @@ > * Set up lock manager > */ > InitLocks(); > - if (InitLockTable(maxBackends) == INVALID_TABLEID) > - elog(FATAL, "could not create the lock table"); > + InitLockTable(maxBackends); > > /* > * Set up process table > diff -ruN ../base/src/backend/storage/lmgr/deadlock.c src/backend/storage/lmgr/deadlock.c > --- ../base/src/backend/storage/lmgr/deadlock.c 2003-08-08 23:42:00.000000000 +0200 > +++ src/backend/storage/lmgr/deadlock.c 2003-09-06 22:25:46.000000000 +0200 > @@ -428,7 +428,7 @@ > LOCK *lock; > PROCLOCK *proclock; > SHM_QUEUE *lockHolders; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROC_QUEUE *waitQueue; > int queue_size; > int conflictMask; > diff -ruN ../base/src/backend/storage/lmgr/lmgr.c src/backend/storage/lmgr/lmgr.c > --- ../base/src/backend/storage/lmgr/lmgr.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lmgr.c 2003-09-06 18:10:48.000000000 +0200 > @@ -65,26 +65,24 @@ > > }; > > -LOCKMETHOD LockTableId = (LOCKMETHOD) NULL; > -LOCKMETHOD LongTermTableId = (LOCKMETHOD) NULL; > +static LOCKMETHODID LockTableId = INVALID_LOCKMETHOD; > > /* > * Create the lock table described by LockConflicts > */ > -LOCKMETHOD > +void > InitLockTable(int maxBackends) > { > - int lockmethod; > + LOCKMETHODID LongTermTableId; > > /* number of lock modes is lengthof()-1 because of dummy zero */ > - lockmethod = LockMethodTableInit("LockTable", > - LockConflicts, > - lengthof(LockConflicts) - 1, > - maxBackends); > - LockTableId = lockmethod; > - > - if (!(LockTableId)) > + LockTableId = LockMethodTableInit("LockTable", > + LockConflicts, > + lengthof(LockConflicts) - 1, > + maxBackends); > + if (!LockMethodIsValid(LockTableId)) > elog(ERROR, "could not initialize lock table"); > + Assert(LockTableId == DEFAULT_LOCKMETHOD); > > #ifdef USER_LOCKS > > @@ -92,11 +90,10 @@ > * Allocate another tableId for long-term locks > */ > LongTermTableId = LockMethodTableRename(LockTableId); > - if (!(LongTermTableId)) > + if (!LockMethodIsValid(LongTermTableId)) > elog(ERROR, "could not rename long-term lock table"); > + Assert(LongTermTableId == USER_LOCKMETHOD); > #endif > - > - return LockTableId; > } > > /* > diff -ruN ../base/src/backend/storage/lmgr/lock.c src/backend/storage/lmgr/lock.c > --- ../base/src/backend/storage/lmgr/lock.c 2003-08-18 00:41:12.000000000 +0200 > +++ src/backend/storage/lmgr/lock.c 2003-09-07 07:53:16.000000000 +0200 > @@ -46,7 +46,7 @@ > #define NLOCKENTS(maxBackends) (max_locks_per_xact * (maxBackends)) > > > -static int WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +static int WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > static void LockCountMyLocks(SHMEM_OFFSET lockOffset, PGPROC *proc, > int *myHolding); > @@ -111,7 +111,7 @@ > "req(%d,%d,%d,%d,%d,%d,%d)=%d " > "grant(%d,%d,%d,%d,%d,%d,%d)=%d wait(%d) type(%s)", > where, MAKE_OFFSET(lock), > - lock->tag.lockmethod, lock->tag.relId, lock->tag.dbId, > + lock->tag.lockmethodid, lock->tag.relId, lock->tag.dbId, > lock->tag.objId.blkno, lock->grantMask, > lock->requested[1], lock->requested[2], lock->requested[3], > lock->requested[4], lock->requested[5], lock->requested[6], > @@ -150,19 +150,9 @@ > > > /* > - * These are to simplify/speed up some bit arithmetic. > - * > - * XXX is a fetch from a static array really faster than a shift? > - * Wouldn't bet on it... > + * map from lock method id to the lock table structure > */ > - > -static LOCKMASK BITS_OFF[MAX_LOCKMODES]; > -static LOCKMASK BITS_ON[MAX_LOCKMODES]; > - > -/* > - * map from lockmethod to the lock table structure > - */ > -static LOCKMETHODTABLE *LockMethodTable[MAX_LOCK_METHODS]; > +static LockMethod LockMethods[MAX_LOCK_METHODS]; > > static int NumLockMethods; > > @@ -173,28 +163,20 @@ > void > InitLocks(void) > { > - int i; > - int bit; > - > - bit = 1; > - for (i = 0; i < MAX_LOCKMODES; i++, bit <<= 1) > - { > - BITS_ON[i] = bit; > - BITS_OFF[i] = ~bit; > - } > + /* NOP */ > } > > > /* > * Fetch the lock method table associated with a given lock > */ > -LOCKMETHODTABLE * > +LockMethod > GetLocksMethodTable(LOCK *lock) > { > - LOCKMETHOD lockmethod = LOCK_LOCKMETHOD(*lock); > + LOCKMETHODID lockmethodid = LOCK_LOCKMETHOD(*lock); > > - Assert(lockmethod > 0 && lockmethod < NumLockMethods); > - return LockMethodTable[lockmethod]; > + Assert(0 < lockmethodid && lockmethodid < NumLockMethods); > + return LockMethods[lockmethodid]; > } > > > @@ -205,7 +187,7 @@ > * Notes: just copying. Should only be called once. > */ > static void > -LockMethodInit(LOCKMETHODTABLE *lockMethodTable, > +LockMethodInit(LockMethod lockMethodTable, > LOCKMASK *conflictsP, > int numModes) > { > @@ -226,13 +208,13 @@ > * by the postmaster are inherited by each backend, so they must be in > * TopMemoryContext. > */ > -LOCKMETHOD > +LOCKMETHODID > LockMethodTableInit(char *tabName, > LOCKMASK *conflictsP, > int numModes, > int maxBackends) > { > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod newLockMethod; > char *shmemName; > HASHCTL info; > int hash_flags; > @@ -254,10 +236,10 @@ > > /* each lock table has a header in shared memory */ > sprintf(shmemName, "%s (lock method table)", tabName); > - lockMethodTable = (LOCKMETHODTABLE *) > - ShmemInitStruct(shmemName, sizeof(LOCKMETHODTABLE), &found); > + newLockMethod = (LockMethod) > + ShmemInitStruct(shmemName, sizeof(LockMethodData), &found); > > - if (!lockMethodTable) > + if (!newLockMethod) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* > @@ -275,15 +257,15 @@ > */ > if (!found) > { > - MemSet(lockMethodTable, 0, sizeof(LOCKMETHODTABLE)); > - lockMethodTable->masterLock = LockMgrLock; > - lockMethodTable->lockmethod = NumLockMethods; > + MemSet(newLockMethod, 0, sizeof(LockMethodData)); > + newLockMethod->masterLock = LockMgrLock; > + newLockMethod->lockmethodid = NumLockMethods; > } > > /* > * other modules refer to the lock table by a lockmethod ID > */ > - LockMethodTable[NumLockMethods] = lockMethodTable; > + LockMethods[NumLockMethods] = newLockMethod; > NumLockMethods++; > Assert(NumLockMethods <= MAX_LOCK_METHODS); > > @@ -297,15 +279,15 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (lock hash)", tabName); > - lockMethodTable->lockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->lockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->lockHash) > + if (!newLockMethod->lockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > - Assert(lockMethodTable->lockHash->hash == tag_hash); > + Assert(newLockMethod->lockHash->hash == tag_hash); > > /* > * allocate a hash table for PROCLOCK structs. This is used to store > @@ -317,23 +299,23 @@ > hash_flags = (HASH_ELEM | HASH_FUNCTION); > > sprintf(shmemName, "%s (proclock hash)", tabName); > - lockMethodTable->proclockHash = ShmemInitHash(shmemName, > - init_table_size, > - max_table_size, > - &info, > - hash_flags); > + newLockMethod->proclockHash = ShmemInitHash(shmemName, > + init_table_size, > + max_table_size, > + &info, > + hash_flags); > > - if (!lockMethodTable->proclockHash) > + if (!newLockMethod->proclockHash) > elog(FATAL, "could not initialize lock table \"%s\"", tabName); > > /* init data structures */ > - LockMethodInit(lockMethodTable, conflictsP, numModes); > + LockMethodInit(newLockMethod, conflictsP, numModes); > > LWLockRelease(LockMgrLock); > > pfree(shmemName); > > - return lockMethodTable->lockmethod; > + return newLockMethod->lockmethodid; > } > > /* > @@ -349,22 +331,22 @@ > * short term and long term locks, yet store them all in one hashtable. > */ > > -LOCKMETHOD > -LockMethodTableRename(LOCKMETHOD lockmethod) > +LOCKMETHODID > +LockMethodTableRename(LOCKMETHODID lockmethodid) > { > - LOCKMETHOD newLockMethod; > + LOCKMETHODID newLockMethodId; > > if (NumLockMethods >= MAX_LOCK_METHODS) > return INVALID_LOCKMETHOD; > - if (LockMethodTable[lockmethod] == INVALID_LOCKMETHOD) > + if (LockMethods[lockmethodid] == INVALID_LOCKMETHOD) > return INVALID_LOCKMETHOD; > > /* other modules refer to the lock table by a lockmethod ID */ > - newLockMethod = NumLockMethods; > + newLockMethodId = NumLockMethods; > NumLockMethods++; > > - LockMethodTable[newLockMethod] = LockMethodTable[lockmethod]; > - return newLockMethod; > + LockMethods[newLockMethodId] = LockMethods[lockmethodid]; > + return newLockMethodId; > } > > /* > @@ -412,7 +394,7 @@ > * > * normal lock user lock > * > - * lockmethod 1 2 > + * lockmethodid 1 2 > * tag.dbId database oid database oid > * tag.relId rel oid or 0 0 > * tag.objId block id lock id2 > @@ -429,7 +411,7 @@ > */ > > bool > -LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait) > { > PROCLOCK *proclock; > @@ -438,25 +420,25 @@ > bool found; > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int status; > int myHolding[MAX_LOCKMODES]; > int i; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockAcquire: user lock [%u] %s", > locktag->objId.blkno, lock_mode_names[lockmode]); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock table id: %d", lockmethod); > + elog(WARNING, "bad lock table id: %d", lockmethodid); > return FALSE; > } > > @@ -666,15 +648,12 @@ > * Construct bitmask of locks this process holds on this object. > */ > { > - int heldLocks = 0; > - int tmpMask; > + LOCKMASK heldLocks = 0; > > - for (i = 1, tmpMask = 2; > - i <= lockMethodTable->numLockModes; > - i++, tmpMask <<= 1) > + for (i = 1; i <= lockMethodTable->numLockModes; i++) > { > if (myHolding[i] > 0) > - heldLocks |= tmpMask; > + heldLocks |= LOCKBIT_ON(i); > } > MyProc->heldLocks = heldLocks; > } > @@ -682,7 +661,7 @@ > /* > * Sleep till someone wakes me up. > */ > - status = WaitOnLock(lockmethod, lockmode, lock, proclock); > + status = WaitOnLock(lockmethodid, lockmode, lock, proclock); > > /* > * NOTE: do not do any material change of state between here and > @@ -729,7 +708,7 @@ > * known. If NULL is passed then these values will be computed internally. > */ > int > -LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock, > @@ -737,9 +716,8 @@ > int *myHolding) /* myHolding[] array or NULL */ > { > int numLockModes = lockMethodTable->numLockModes; > - int bitmask; > - int i, > - tmpMask; > + LOCKMASK bitmask; > + int i; > int localHolding[MAX_LOCKMODES]; > > /* > @@ -772,11 +750,10 @@ > > /* Compute mask of lock types held by other processes */ > bitmask = 0; > - tmpMask = 2; > - for (i = 1; i <= numLockModes; i++, tmpMask <<= 1) > + for (i = 1; i <= numLockModes; i++) > { > if (lock->granted[i] != myHolding[i]) > - bitmask |= tmpMask; > + bitmask |= LOCKBIT_ON(i); > } > > /* > @@ -842,9 +819,9 @@ > { > lock->nGranted++; > lock->granted[lockmode]++; > - lock->grantMask |= BITS_ON[lockmode]; > + lock->grantMask |= LOCKBIT_ON(lockmode); > if (lock->granted[lockmode] == lock->requested[lockmode]) > - lock->waitMask &= BITS_OFF[lockmode]; > + lock->waitMask &= LOCKBIT_OFF(lockmode); > LOCK_PRINT("GrantLock", lock, lockmode); > Assert((lock->nGranted > 0) && (lock->granted[lockmode] > 0)); > Assert(lock->nGranted <= lock->nRequested); > @@ -862,14 +839,14 @@ > * The locktable's masterLock must be held at entry. > */ > static int > -WaitOnLock(LOCKMETHOD lockmethod, LOCKMODE lockmode, > +WaitOnLock(LOCKMETHODID lockmethodid, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock) > { > - LOCKMETHODTABLE *lockMethodTable = LockMethodTable[lockmethod]; > + LockMethod lockMethodTable = LockMethods[lockmethodid]; > char *new_status, > *old_status; > > - Assert(lockmethod < NumLockMethods); > + Assert(lockmethodid < NumLockMethods); > > LOCK_PRINT("WaitOnLock: sleeping on lock", lock, lockmode); > > @@ -957,7 +934,7 @@ > waitLock->requested[lockmode]--; > /* don't forget to clear waitMask bit if appropriate */ > if (waitLock->granted[lockmode] == waitLock->requested[lockmode]) > - waitLock->waitMask &= BITS_OFF[lockmode]; > + waitLock->waitMask &= LOCKBIT_OFF(lockmode); > > /* Clean up the proc's own state */ > proc->waitLock = NULL; > @@ -968,7 +945,7 @@ > } > > /* > - * LockRelease -- look up 'locktag' in lock table 'lockmethod' and > + * LockRelease -- look up 'locktag' in lock table 'lockmethodid' and > * release one 'lockmode' lock on it. > * > * Side Effects: find any waiting processes that are now wakable, > @@ -978,27 +955,27 @@ > * come along and request the lock.) > */ > bool > -LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode) > { > LOCK *lock; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > PROCLOCK *proclock; > PROCLOCKTAG proclocktag; > HTAB *proclockTable; > bool wakeupNeeded = false; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD && Trace_userlocks) > + if (lockmethodid == USER_LOCKMETHOD && Trace_userlocks) > elog(LOG, "LockRelease: user lock tag [%u] %d", locktag->objId.blkno, lockmode); > #endif > > /* ???????? This must be changed when short term locks will be used */ > - locktag->lockmethod = lockmethod; > + locktag->lockmethodid = lockmethodid; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > elog(WARNING, "lockMethodTable is null in LockRelease"); > @@ -1045,7 +1022,7 @@ > { > LWLockRelease(masterLock); > #ifdef USER_LOCKS > - if (lockmethod == USER_LOCKMETHOD) > + if (lockmethodid == USER_LOCKMETHOD) > elog(WARNING, "no lock with this tag"); > else > #endif > @@ -1083,7 +1060,7 @@ > if (lock->granted[lockmode] == 0) > { > /* change the conflict mask. No more of this lock type. */ > - lock->grantMask &= BITS_OFF[lockmode]; > + lock->grantMask &= LOCKBIT_OFF(lockmode); > } > > LOCK_PRINT("LockRelease: updated", lock, lockmode); > @@ -1173,29 +1150,29 @@ > * specified XID are released. > */ > bool > -LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid) > { > SHM_QUEUE *procHolders = &(proc->procHolders); > PROCLOCK *proclock; > PROCLOCK *nextHolder; > LWLockId masterLock; > - LOCKMETHODTABLE *lockMethodTable; > + LockMethod lockMethodTable; > int i, > numLockModes; > LOCK *lock; > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll: lockmethod=%d, pid=%d", > - lockmethod, proc->pid); > + lockmethodid, proc->pid); > #endif > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > { > - elog(WARNING, "bad lock method: %d", lockmethod); > + elog(WARNING, "bad lock method: %d", lockmethodid); > return FALSE; > } > > @@ -1220,7 +1197,7 @@ > lock = (LOCK *) MAKE_PTR(proclock->tag.lock); > > /* Ignore items that are not of the lockmethod to be removed */ > - if (LOCK_LOCKMETHOD(*lock) != lockmethod) > + if (LOCK_LOCKMETHOD(*lock) != lockmethodid) > goto next_item; > > /* If not allxids, ignore items that are of the wrong xid */ > @@ -1249,7 +1226,7 @@ > lock->granted[i] -= proclock->holding[i]; > Assert(lock->requested[i] >= 0 && lock->granted[i] >= 0); > if (lock->granted[i] == 0) > - lock->grantMask &= BITS_OFF[i]; > + lock->grantMask &= LOCKBIT_OFF(i); > > /* > * Read comments in LockRelease > @@ -1331,7 +1308,7 @@ > LWLockRelease(masterLock); > > #ifdef LOCK_DEBUG > - if (lockmethod == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > + if (lockmethodid == USER_LOCKMETHOD ? Trace_userlocks : Trace_locks) > elog(LOG, "LockReleaseAll done"); > #endif > > @@ -1346,7 +1323,7 @@ > > size += MAXALIGN(sizeof(PROC_HDR)); /* ProcGlobal */ > size += maxBackends * MAXALIGN(sizeof(PGPROC)); /* each MyProc */ > - size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LOCKMETHODTABLE)); /* each lockMethodTable */ > + size += MAX_LOCK_METHODS * MAXALIGN(sizeof(LockMethodData)); /* each lock method */ > > /* lockHash table */ > size += hash_estimate_size(max_table_size, sizeof(LOCK)); > @@ -1390,7 +1367,7 @@ > > LWLockAcquire(LockMgrLock, LW_EXCLUSIVE); > > - proclockTable = LockMethodTable[DEFAULT_LOCKMETHOD]->proclockHash; > + proclockTable = LockMethods[DEFAULT_LOCKMETHOD]->proclockHash; > > data->nelements = i = proclockTable->hctl->nentries; > > @@ -1446,8 +1423,8 @@ > SHM_QUEUE *procHolders; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > > proc = MyProc; > if (proc == NULL) > @@ -1455,8 +1432,8 @@ > > procHolders = &proc->procHolders; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > @@ -1489,8 +1466,8 @@ > PGPROC *proc; > PROCLOCK *proclock; > LOCK *lock; > - int lockmethod = DEFAULT_LOCKMETHOD; > - LOCKMETHODTABLE *lockMethodTable; > + int lockmethodid = DEFAULT_LOCKMETHOD; > + LockMethod lockMethodTable; > HTAB *proclockTable; > HASH_SEQ_STATUS status; > > @@ -1498,8 +1475,8 @@ > if (proc == NULL) > return; > > - Assert(lockmethod < NumLockMethods); > - lockMethodTable = LockMethodTable[lockmethod]; > + Assert(lockmethodid < NumLockMethods); > + lockMethodTable = LockMethods[lockmethodid]; > if (!lockMethodTable) > return; > > diff -ruN ../base/src/backend/storage/lmgr/proc.c src/backend/storage/lmgr/proc.c > --- ../base/src/backend/storage/lmgr/proc.c 2003-08-04 04:40:03.000000000 +0200 > +++ src/backend/storage/lmgr/proc.c 2003-09-07 07:35:06.000000000 +0200 > @@ -524,14 +524,14 @@ > * semaphore is normally zero, so when we try to acquire it, we sleep. > */ > int > -ProcSleep(LOCKMETHODTABLE *lockMethodTable, > +ProcSleep(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, > PROCLOCK *proclock) > { > LWLockId masterLock = lockMethodTable->masterLock; > PROC_QUEUE *waitQueue = &(lock->waitProcs); > - int myHeldLocks = MyProc->heldLocks; > + LOCKMASK myHeldLocks = MyProc->heldLocks; > bool early_deadlock = false; > PGPROC *proc; > int i; > @@ -556,7 +556,7 @@ > */ > if (myHeldLocks != 0) > { > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > proc = (PGPROC *) MAKE_PTR(waitQueue->links.next); > for (i = 0; i < waitQueue->size; i++) > @@ -596,7 +596,7 @@ > break; > } > /* Nope, so advance to next waiter */ > - aheadRequests |= (1 << proc->waitLockMode); > + aheadRequests |= LOCKBIT_ON(proc->waitLockMode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > > @@ -618,7 +618,7 @@ > SHMQueueInsertBefore(&(proc->links), &(MyProc->links)); > waitQueue->size++; > > - lock->waitMask |= (1 << lockmode); > + lock->waitMask |= LOCKBIT_ON(lockmode); > > /* Set up wait information in PGPROC object, too */ > MyProc->waitLock = lock; > @@ -750,12 +750,12 @@ > * for lock, waken any that are no longer blocked. > */ > void > -ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock) > +ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) > { > PROC_QUEUE *waitQueue = &(lock->waitProcs); > int queue_size = waitQueue->size; > PGPROC *proc; > - int aheadRequests = 0; > + LOCKMASK aheadRequests = 0; > > Assert(queue_size >= 0); > > @@ -796,7 +796,7 @@ > * Cannot wake this guy. Remember his request for later > * checks. > */ > - aheadRequests |= (1 << lockmode); > + aheadRequests |= LOCKBIT_ON(lockmode); > proc = (PGPROC *) MAKE_PTR(proc->links.next); > } > } > diff -ruN ../base/src/include/storage/lmgr.h src/include/storage/lmgr.h > --- ../base/src/include/storage/lmgr.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lmgr.h 2003-09-06 18:02:43.000000000 +0200 > @@ -40,10 +40,7 @@ > * so increase that if you want to add more modes. > */ > > -extern LOCKMETHOD LockTableId; > - > - > -extern LOCKMETHOD InitLockTable(int maxBackends); > +extern void InitLockTable(int maxBackends); > extern void RelationInitLockInfo(Relation relation); > > /* Lock a relation */ > diff -ruN ../base/src/include/storage/lock.h src/include/storage/lock.h > --- ../base/src/include/storage/lock.h 2003-08-04 04:40:14.000000000 +0200 > +++ src/include/storage/lock.h 2003-09-07 07:25:13.000000000 +0200 > @@ -42,22 +42,23 @@ > > > typedef int LOCKMASK; > - > typedef int LOCKMODE; > -typedef int LOCKMETHOD; > - > /* MAX_LOCKMODES cannot be larger than the # of bits in LOCKMASK */ > #define MAX_LOCKMODES 10 > > +#define LOCKBIT_ON(lockmode) (1 << (lockmode)) > +#define LOCKBIT_OFF(lockmode) (~(1 << (lockmode))) > + > +typedef uint16 LOCKMETHODID; > /* MAX_LOCK_METHODS is the number of distinct lock control tables allowed */ > #define MAX_LOCK_METHODS 3 > > -#define INVALID_TABLEID 0 > - > -#define INVALID_LOCKMETHOD INVALID_TABLEID > +#define INVALID_LOCKMETHOD 0 > #define DEFAULT_LOCKMETHOD 1 > #define USER_LOCKMETHOD 2 > > +#define LockMethodIsValid(lockmethodid) ((lockmethodid) != INVALID_LOCKMETHOD) > + > /* > * There is normally only one lock method, the default one. > * If user locks are enabled, an additional lock method is present. > @@ -83,15 +84,16 @@ > * masterLock -- synchronizes access to the table > * > */ > -typedef struct LOCKMETHODTABLE > +typedef struct LockMethodData > { > - HTAB *lockHash; > - HTAB *proclockHash; > - LOCKMETHOD lockmethod; > - int numLockModes; > - int conflictTab[MAX_LOCKMODES]; > - LWLockId masterLock; > -} LOCKMETHODTABLE; > + HTAB *lockHash; > + HTAB *proclockHash; > + LOCKMETHODID lockmethodid; > + int numLockModes; > + LOCKMASK conflictTab[MAX_LOCKMODES]; > + LWLockId masterLock; > +} LockMethodData; > +typedef LockMethodData *LockMethod; > > > /* > @@ -115,7 +117,7 @@ > */ > OffsetNumber offnum; > > - uint16 lockmethod; /* needed by userlocks */ > + LOCKMETHODID lockmethodid; /* needed by userlocks */ > } LOCKTAG; > > > @@ -139,8 +141,8 @@ > LOCKTAG tag; /* unique identifier of lockable object */ > > /* data */ > - int grantMask; /* bitmask for lock types already granted */ > - int waitMask; /* bitmask for lock types awaited */ > + LOCKMASK grantMask; /* bitmask for lock types already granted */ > + LOCKMASK waitMask; /* bitmask for lock types awaited */ > SHM_QUEUE lockHolders; /* list of PROCLOCK objects assoc. with > * lock */ > PROC_QUEUE waitProcs; /* list of PGPROC objects waiting on lock */ > @@ -151,7 +153,7 @@ > int nGranted; /* total of granted[] array */ > } LOCK; > > -#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethod) > +#define LOCK_LOCKMETHOD(lock) ((lock).tag.lockmethodid) > > > /* > @@ -204,7 +206,7 @@ > } PROCLOCK; > > #define PROCLOCK_LOCKMETHOD(proclock) \ > - (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethod) > + (((LOCK *) MAKE_PTR((proclock).tag.lock))->tag.lockmethodid) > > /* > * This struct holds information passed from lmgr internals to the lock > @@ -227,17 +229,17 @@ > * function prototypes > */ > extern void InitLocks(void); > -extern LOCKMETHODTABLE *GetLocksMethodTable(LOCK *lock); > -extern LOCKMETHOD LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > +extern LockMethod GetLocksMethodTable(LOCK *lock); > +extern LOCKMETHODID LockMethodTableInit(char *tabName, LOCKMASK *conflictsP, > int numModes, int maxBackends); > -extern LOCKMETHOD LockMethodTableRename(LOCKMETHOD lockmethod); > -extern bool LockAcquire(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern LOCKMETHODID LockMethodTableRename(LOCKMETHODID lockmethodid); > +extern bool LockAcquire(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode, bool dontWait); > -extern bool LockRelease(LOCKMETHOD lockmethod, LOCKTAG *locktag, > +extern bool LockRelease(LOCKMETHODID lockmethodid, LOCKTAG *locktag, > TransactionId xid, LOCKMODE lockmode); > -extern bool LockReleaseAll(LOCKMETHOD lockmethod, PGPROC *proc, > +extern bool LockReleaseAll(LOCKMETHODID lockmethodid, PGPROC *proc, > bool allxids, TransactionId xid); > -extern int LockCheckConflicts(LOCKMETHODTABLE *lockMethodTable, > +extern int LockCheckConflicts(LockMethod lockMethodTable, > LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock, PGPROC *proc, > int *myHolding); > diff -ruN ../base/src/include/storage/proc.h src/include/storage/proc.h > --- ../base/src/include/storage/proc.h 2003-08-04 04:40:15.000000000 +0200 > +++ src/include/storage/proc.h 2003-09-06 22:45:54.000000000 +0200 > @@ -101,10 +101,10 @@ > extern void ProcReleaseLocks(bool isCommit); > > extern void ProcQueueInit(PROC_QUEUE *queue); > -extern int ProcSleep(LOCKMETHODTABLE *lockMethodTable, LOCKMODE lockmode, > +extern int ProcSleep(LockMethod lockMethodTable, LOCKMODE lockmode, > LOCK *lock, PROCLOCK *proclock); > extern PGPROC *ProcWakeup(PGPROC *proc, int errType); > -extern void ProcLockWakeup(LOCKMETHODTABLE *lockMethodTable, LOCK *lock); > +extern void ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock); > extern bool LockWaitCancel(void); > > extern void ProcWaitForSignal(void); > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Don't 'kill -9' the postmaster -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Patch applied. Thanks. That lock manager code really needed the cleanup. --------------------------------------------------------------------------- Manfred Koizar wrote: > On Mon, 8 Sep 2003 13:02:11 -0400 (EDT), Bruce Momjian > <pgman@candle.pha.pa.us> wrote: > > > >Manfred, can I get a description for this patch? Thanks. > > Try to reduce confusion about what is a lock method identifier, a lock > method control structure, or a table of control structures. > > . Use type LOCKMASK where an int is not a counter. > > . Get rid of INVALID_TABLEID, use INVALID_LOCKMETHOD instead. > > . Use INVALID_LOCKMETHOD instead of (LOCKMETHOD) NULL, because > LOCKMETHOD is not a pointer. > > . Define and use macro LockMethodIsValid. > > . Rename LOCKMETHOD to LOCKMETHODID. > > . Remove global variable LongTermTableId in lmgr.c, because it is > never used. > > . Make LockTableId static in lmgr.c, because it is used nowhere else. > Why not remove it and use DEFAULT_LOCKMETHOD? > > . Rename the lock method control structure from LOCKMETHODTABLE to > LockMethodData. Introduce a pointer type named LockMethod. > > . Remove elog(FATAL) after InitLockTable() call in > CreateSharedMemoryAndSemaphores(), because if something goes wrong, > there is elog(FATAL) in LockMethodTableInit(), and if this doesn't > help, an elog(ERROR) in InitLockTable() is promoted to FATAL. > > . Make InitLockTable() void, because its only caller does not use its > return value any more. > > . Rename variables in lock.c to avoid statements like > LockMethodTable[NumLockMethods] = lockMethodTable; > lockMethodTable = LockMethodTable[lockmethod]; > > . Change LOCKMETHODID type to uint16 to fit into struct LOCKTAG. > > . Remove static variables BITS_OFF and BITS_ON from lock.c, because > I agree to this doubt: > * XXX is a fetch from a static array really faster than a shift? > > . Define and use macros LOCKBIT_ON/OFF. > > ====================== > All 93 tests passed. > ====================== > > Servus > Manfred > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073