Minor lmgr code cleanup

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

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

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