From: Jeffrey Altman Date: Sat, 26 Nov 2011 22:26:50 +0000 (-0500) Subject: Windows: osi_mutex / osi_rwlock changes X-Git-Tag: upstream/1.8.0_pre1^2~3012 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=a07338f4ccc5062e224405ccda0c9ed41e666f5e;p=packages%2Fo%2Fopenafs.git Windows: osi_mutex / osi_rwlock changes Reorganize the osi_mutex and osi_rwlock structure so that all counters are 32-bit and pointers are aligned. This requires adding padding fields. Move lock validation checks within the critical section. Include additional assertions checking the ownership state and protecting against under/overflows. Increase the size of the rwlock tid array to support a larger number of simultaneous readers. Change-Id: Ia46684c601a1a589a210a36862ae6ad6448a435e Reviewed-on: http://gerrit.openafs.org/6130 Tested-by: BuildBot Tested-by: Jeffrey Altman Reviewed-by: Jeffrey Altman --- diff --git a/src/WINNT/client_osi/osibasel.c b/src/WINNT/client_osi/osibasel.c index d6916e3a6..351a6f78f 100644 --- a/src/WINNT/client_osi/osibasel.c +++ b/src/WINNT/client_osi/osibasel.c @@ -154,6 +154,7 @@ void lock_ObtainWrite(osi_rwlock_t *lockp) CRITICAL_SECTION *csp; osi_queue_t * lockRefH, *lockRefT; osi_lock_ref_t *lockRefp; + DWORD tid = thrd_Current(); if ((i=lockp->type) != 0) { if (i >= 0 && i < OSI_NLOCKTYPES) @@ -173,18 +174,29 @@ void lock_ObtainWrite(osi_rwlock_t *lockp) csp = &osi_baseAtomicCS[lockp->atomicIndex]; EnterCriticalSection(csp); + if (lockp->flags & OSI_LOCKFLAG_EXCL) { + osi_assertx(lockp->tid[0] != tid, "OSI_RWLOCK_WRITEHELD"); + } else { + for ( i=0; i < lockp->readers && i < OSI_RWLOCK_THREADS; i++ ) { + osi_assertx(lockp->tid[i] != tid, "OSI_RWLOCK_READHELD"); + } + } + /* here we have the fast lock, so see if we can obtain the real lock */ if (lockp->waiters > 0 || (lockp->flags & OSI_LOCKFLAG_EXCL) || (lockp->readers > 0)) { lockp->waiters++; osi_TWait(&lockp->d.turn, OSI_SLEEPINFO_W4WRITE, &lockp->flags, lockp->tid, csp); lockp->waiters--; + osi_assertx(lockp->waiters >= 0, "waiters underflow"); osi_assert(lockp->readers == 0 && (lockp->flags & OSI_LOCKFLAG_EXCL)); } else { /* if we're here, all clear to set the lock */ lockp->flags |= OSI_LOCKFLAG_EXCL; - lockp->tid[0] = thrd_Current(); + lockp->tid[0] = tid; } + osi_assertx(lockp->readers == 0, "write lock readers present"); + LeaveCriticalSection(csp); if (lockOrderValidation) { @@ -221,8 +233,12 @@ void lock_ObtainRead(osi_rwlock_t *lockp) csp = &osi_baseAtomicCS[lockp->atomicIndex]; EnterCriticalSection(csp); - for ( i=0; i < lockp->readers; i++ ) { - osi_assertx(lockp->tid[i] != tid, "OSI_RWLOCK_READHELD"); + if (lockp->flags & OSI_LOCKFLAG_EXCL) { + osi_assertx(lockp->tid[0] != tid, "OSI_RWLOCK_WRITEHELD"); + } else { + for ( i=0; i < lockp->readers && i < OSI_RWLOCK_THREADS; i++ ) { + osi_assertx(lockp->tid[i] != tid, "OSI_RWLOCK_READHELD"); + } } /* here we have the fast lock, so see if we can obtain the real lock */ @@ -230,6 +246,7 @@ void lock_ObtainRead(osi_rwlock_t *lockp) lockp->waiters++; osi_TWait(&lockp->d.turn, OSI_SLEEPINFO_W4READ, &lockp->readers, lockp->tid, csp); lockp->waiters--; + osi_assertx(lockp->waiters >= 0, "waiters underflow"); osi_assert(!(lockp->flags & OSI_LOCKFLAG_EXCL) && lockp->readers > 0); } else { /* if we're here, all clear to set the lock */ @@ -260,6 +277,10 @@ void lock_ReleaseRead(osi_rwlock_t *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { int found = 0; lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); @@ -279,10 +300,6 @@ void lock_ReleaseRead(osi_rwlock_t *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->readers > 0, "read lock not held"); for ( i=0; i < lockp->readers; i++) { @@ -295,10 +312,12 @@ void lock_ReleaseRead(osi_rwlock_t *lockp) } /* releasing a read lock can allow readers or writers */ - if (--lockp->readers == 0 && !osi_TEmpty(&lockp->d.turn)) { + if (--(lockp->readers) == 0 && !osi_TEmpty(&lockp->d.turn)) { osi_TSignalForMLs(&lockp->d.turn, 0, csp); } else { + osi_assertx(lockp->readers >= 0, "read lock underflow"); + /* and finally release the big lock */ LeaveCriticalSection(csp); } @@ -317,6 +336,10 @@ void lock_ReleaseWrite(osi_rwlock_t *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { int found = 0; lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); @@ -336,10 +359,6 @@ void lock_ReleaseWrite(osi_rwlock_t *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->flags & OSI_LOCKFLAG_EXCL, "write lock not held"); osi_assertx(lockp->tid[0] == thrd_Current(), "write lock not held by current thread"); @@ -377,6 +396,8 @@ void lock_ConvertWToR(osi_rwlock_t *lockp) lockp->flags &= ~OSI_LOCKFLAG_EXCL; lockp->readers++; + osi_assertx(lockp->readers == 1, "read lock not one"); + if (!osi_TEmpty(&lockp->d.turn)) { osi_TSignalForMLs(&lockp->d.turn, /* still have readers */ 1, csp); } @@ -414,14 +435,17 @@ void lock_ConvertRToW(osi_rwlock_t *lockp) } } - if (--lockp->readers == 0) { + if (--(lockp->readers) == 0) { /* convert read lock to write lock */ lockp->flags |= OSI_LOCKFLAG_EXCL; lockp->tid[0] = tid; } else { + osi_assertx(lockp->readers > 0, "read lock underflow"); + lockp->waiters++; osi_TWait(&lockp->d.turn, OSI_SLEEPINFO_W4WRITE, &lockp->flags, lockp->tid, csp); lockp->waiters--; + osi_assertx(lockp->waiters >= 0, "waiters underflow"); osi_assert(lockp->readers == 0 && (lockp->flags & OSI_LOCKFLAG_EXCL)); } @@ -441,6 +465,10 @@ void lock_ObtainMutex(struct osi_mutex *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -449,21 +477,19 @@ void lock_ObtainMutex(struct osi_mutex *lockp) lock_VerifyOrderMX(lockRefH, lockRefT, lockp); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - /* here we have the fast lock, so see if we can obtain the real lock */ if (lockp->waiters > 0 || (lockp->flags & OSI_LOCKFLAG_EXCL)) { lockp->waiters++; osi_TWait(&lockp->d.turn, OSI_SLEEPINFO_W4WRITE, &lockp->flags, &lockp->tid, csp); lockp->waiters--; + osi_assertx(lockp->waiters >= 0, "waiters underflow"); osi_assert(lockp->flags & OSI_LOCKFLAG_EXCL); } else { /* if we're here, all clear to set the lock */ lockp->flags |= OSI_LOCKFLAG_EXCL; lockp->tid = thrd_Current(); } + LeaveCriticalSection(csp); if (lockOrderValidation) { @@ -487,6 +513,10 @@ void lock_ReleaseMutex(struct osi_mutex *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { int found = 0; lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); @@ -506,10 +536,6 @@ void lock_ReleaseMutex(struct osi_mutex *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->flags & OSI_LOCKFLAG_EXCL, "mutex not held"); osi_assertx(lockp->tid == thrd_Current(), "mutex not held by current thread"); @@ -535,6 +561,10 @@ int lock_TryRead(struct osi_rwlock *lockp) if (i >= 0 && i < OSI_NLOCKTYPES) return (osi_lockOps[i]->TryReadProc)(lockp); + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -548,17 +578,13 @@ int lock_TryRead(struct osi_rwlock *lockp) } } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - /* here we have the fast lock, so see if we can obtain the real lock */ if (lockp->waiters > 0 || (lockp->flags & OSI_LOCKFLAG_EXCL)) { i = 0; } else { /* if we're here, all clear to set the lock */ - if (++lockp->readers < OSI_RWLOCK_THREADS) + if (++(lockp->readers) < OSI_RWLOCK_THREADS) lockp->tid[lockp->readers-1] = thrd_Current(); i = 1; } @@ -587,6 +613,10 @@ int lock_TryWrite(struct osi_rwlock *lockp) if (i >= 0 && i < OSI_NLOCKTYPES) return (osi_lockOps[i]->TryWriteProc)(lockp); + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -600,10 +630,6 @@ int lock_TryWrite(struct osi_rwlock *lockp) } } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - /* here we have the fast lock, so see if we can obtain the real lock */ if (lockp->waiters > 0 || (lockp->flags & OSI_LOCKFLAG_EXCL) || (lockp->readers > 0)) { @@ -639,6 +665,10 @@ int lock_TryMutex(struct osi_mutex *lockp) { if (i >= 0 && i < OSI_NLOCKTYPES) return (osi_lockOps[i]->TryMutexProc)(lockp); + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -652,10 +682,6 @@ int lock_TryMutex(struct osi_mutex *lockp) { } } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - /* here we have the fast lock, so see if we can obtain the real lock */ if (lockp->waiters > 0 || (lockp->flags & OSI_LOCKFLAG_EXCL)) { i = 0; @@ -675,6 +701,7 @@ int lock_TryMutex(struct osi_mutex *lockp) { TlsSetValue(tls_LockRefH, lockRefH); TlsSetValue(tls_LockRefT, lockRefT); } + return i; } @@ -692,6 +719,10 @@ void osi_SleepR(LONG_PTR sleepVal, struct osi_rwlock *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -708,10 +739,6 @@ void osi_SleepR(LONG_PTR sleepVal, struct osi_rwlock *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->readers > 0, "osi_SleepR: not held"); for ( i=0; i < lockp->readers; i++) { @@ -726,7 +753,7 @@ void osi_SleepR(LONG_PTR sleepVal, struct osi_rwlock *lockp) /* XXX better to get the list of things to wakeup from TSignalForMLs, and * then do the wakeup after SleepSpin releases the low-level mutex. */ - if (--lockp->readers == 0 && !osi_TEmpty(&lockp->d.turn)) { + if (--(lockp->readers) == 0 && !osi_TEmpty(&lockp->d.turn)) { osi_TSignalForMLs(&lockp->d.turn, 0, NULL); } @@ -748,6 +775,10 @@ void osi_SleepW(LONG_PTR sleepVal, struct osi_rwlock *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -764,10 +795,6 @@ void osi_SleepW(LONG_PTR sleepVal, struct osi_rwlock *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->flags & OSI_LOCKFLAG_EXCL, "osi_SleepW: not held"); lockp->flags &= ~OSI_LOCKFLAG_EXCL; @@ -793,6 +820,10 @@ void osi_SleepM(LONG_PTR sleepVal, struct osi_mutex *lockp) return; } + /* otherwise we're the fast base type */ + csp = &osi_baseAtomicCS[lockp->atomicIndex]; + EnterCriticalSection(csp); + if (lockOrderValidation && lockp->level != 0) { lockRefH = (osi_queue_t *)TlsGetValue(tls_LockRefH); lockRefT = (osi_queue_t *)TlsGetValue(tls_LockRefT); @@ -809,10 +840,6 @@ void osi_SleepM(LONG_PTR sleepVal, struct osi_mutex *lockp) TlsSetValue(tls_LockRefT, lockRefT); } - /* otherwise we're the fast base type */ - csp = &osi_baseAtomicCS[lockp->atomicIndex]; - EnterCriticalSection(csp); - osi_assertx(lockp->flags & OSI_LOCKFLAG_EXCL, "osi_SleepM not held"); lockp->flags &= ~OSI_LOCKFLAG_EXCL; @@ -853,12 +880,11 @@ void lock_InitializeMutex(osi_mutex_t *mp, char *namep, unsigned short level) return; } - /* otherwise we have the base case, which requires no special + /* + * otherwise we have the base case, which requires no special * initialization. */ - mp->type = 0; - mp->flags = 0; - mp->tid = 0; + memset(mp, 0, sizeof(osi_mutex_t)); mp->atomicIndex = (unsigned short)(InterlockedIncrement(&atomicIndexCounter) % OSI_MUTEXHASHSIZE); mp->level = level; osi_TInit(&mp->d.turn); diff --git a/src/WINNT/client_osi/osibasel.h b/src/WINNT/client_osi/osibasel.h index 9ef6b9051..410668d3c 100644 --- a/src/WINNT/client_osi/osibasel.h +++ b/src/WINNT/client_osi/osibasel.h @@ -28,17 +28,18 @@ * lock using an atomic increment operation. */ typedef struct osi_mutex { - char type; /* for all types; type 0 uses atomic count */ - char flags; /* flags for base type */ + short type; /* for all types; type 0 uses atomic count */ unsigned short atomicIndex; /* index of lock for low-level sync */ + int flags; /* flags for base type */ DWORD tid; /* tid of thread that owns the lock */ - unsigned short waiters; /* waiters */ - unsigned short pad; + int waiters; /* waiters */ + unsigned short level; /* locking hierarchy level */ + short pad1; + int pad2; union { void *privateDatap; /* data pointer for non-zero types */ osi_turnstile_t turn; /* turnstile */ } d; - unsigned short level; /* locking hierarchy level */ } osi_mutex_t; /* a read/write lock. This structure has two forms. In the @@ -54,20 +55,21 @@ typedef struct osi_mutex { * This type of lock has N readers or one writer. */ -#define OSI_RWLOCK_THREADS 32 +#define OSI_RWLOCK_THREADS 64 typedef struct osi_rwlock { - char type; /* for all types; type 0 uses atomic count */ - char flags; /* flags for base type */ + short type; /* for all types; type 0 uses atomic count */ unsigned short atomicIndex; /* index into hash table for low-level sync */ - DWORD tid[OSI_RWLOCK_THREADS]; /* writer's tid */ - unsigned short waiters; /* waiters */ - unsigned short readers; /* readers */ + int flags; /* flags */ + int waiters; /* waiters */ + int readers; /* readers */ + DWORD tid[OSI_RWLOCK_THREADS]; /* writer's tid */ + short pad2; + unsigned short level; /* locking hierarchy level */ union { void *privateDatap; /* data pointer for non-zero types */ osi_turnstile_t turn; /* turnstile */ } d; - unsigned short level; /* locking hierarchy level */ } osi_rwlock_t;