On Sun, 07 Sep 2003 10:19:07 -0400, Tom Lane <[EMAIL PROTECTED]>
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 [EMAIL PROTECTED]
