From: dan Date: Wed, 14 Apr 2010 15:49:40 +0000 (+0000) Subject: Improve the logLockRegion() function in log.c. X-Git-Tag: version-3.7.2~455^2~83 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=02bb59664928c74552cbbcb3390ccbb61bbaf961;p=thirdparty%2Fsqlite.git Improve the logLockRegion() function in log.c. FossilOrigin-Name: 5e9dd3bd8e829376408925fb4cfcd5bb1eb1105f --- diff --git a/manifest b/manifest index 1f3fc5b551..611f989a4f 100644 --- a/manifest +++ b/manifest @@ -1,5 +1,5 @@ -C Fixes\sfor\slocking\sissues\sin\sWAL\smode. -D 2010-04-14T11:23:30 +C Improve\sthe\slogLockRegion()\sfunction\sin\slog.c. +D 2010-04-14T15:49:40 F Makefile.arm-wince-mingw32ce-gcc fcd5e9cd67fe88836360bb4f9ef4cb7f8e2fb5a0 F Makefile.in 4f2f967b7e58a35bb74fb7ec8ae90e0f4ca7868b F Makefile.linux-gcc d53183f4aa6a9192d249731c90dbdffbd2c68654 @@ -131,7 +131,7 @@ F src/journal.c b0ea6b70b532961118ab70301c00a33089f9315c F src/legacy.c a199d7683d60cef73089e892409113e69c23a99f F src/lempar.c 7f026423f4d71d989e719a743f98a1cbd4e6d99e F src/loadext.c 1c7a61ce1281041f437333f366a96aa0d29bb581 -F src/log.c 78575e5bed43578580a0ee86fd3c2e924abe4a88 +F src/log.c e01488f4e515d17ecb21d6cebc5a37bec5b0e56c F src/log.h a2654af46ce7b5732f4d5a731abfdd180f0a06d9 F src/main.c c0e7192bad5b90544508b241eb2487ac661de890 F src/malloc.c a08f16d134f0bfab6b20c3cd142ebf3e58235a6a @@ -803,7 +803,7 @@ F tool/speedtest2.tcl ee2149167303ba8e95af97873c575c3e0fab58ff F tool/speedtest8.c 2902c46588c40b55661e471d7a86e4dd71a18224 F tool/speedtest8inst1.c 293327bc76823f473684d589a8160bde1f52c14e F tool/vdbe-compress.tcl d70ea6d8a19e3571d7ab8c9b75cba86d1173ff0f -P 3f958e87c33d667d299b03ffdef58db5dc6363f4 -R 91ba1bf1aecd8bbf6dbeda941d0dbe58 +P a9617eff39177250e2f118f25fdd4b3acb8b0478 +R 7008c4f8854b7fbb33a10340b4b449d8 U dan -Z d3b26d31320c12902d5354af18b08451 +Z d0e2fbdd9e82a46dc4aa039b5fc8e3ce diff --git a/manifest.uuid b/manifest.uuid index 8cd6641af0..8fdc50e3bd 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a9617eff39177250e2f118f25fdd4b3acb8b0478 \ No newline at end of file +5e9dd3bd8e829376408925fb4cfcd5bb1eb1105f \ No newline at end of file diff --git a/src/log.c b/src/log.c index c7fb8738d2..9b25216df4 100644 --- a/src/log.c +++ b/src/log.c @@ -989,84 +989,129 @@ static void logLeaveMutex(Log *pLog){ static int logLockRegion(Log *pLog, u32 mRegion, int op){ LogSummary *pSummary = pLog->pSummary; LogLock *p; /* Used to iterate through in-process locks */ - u32 mNew; /* New locks on file */ - u32 mOld; /* Old locks on file */ - u32 mNewLock; /* New locks held by pLog */ + u32 mOther; /* Locks held by other connections */ + u32 mNew; /* New mask for pLog */ assert( /* Writer lock operations */ (op==LOG_WRLOCK && mRegion==(LOG_REGION_C|LOG_REGION_D)) || (op==LOG_UNLOCK && mRegion==(LOG_REGION_C|LOG_REGION_D)) - /* Reader lock operations */ + /* Normal reader lock operations */ || (op==LOG_RDLOCK && mRegion==(LOG_REGION_A|LOG_REGION_B)) - || (op==LOG_RDLOCK && mRegion==(LOG_REGION_D)) || (op==LOG_UNLOCK && mRegion==(LOG_REGION_A)) || (op==LOG_UNLOCK && mRegion==(LOG_REGION_B)) + + /* Region D reader lock operations */ + || (op==LOG_RDLOCK && mRegion==(LOG_REGION_D)) || (op==LOG_UNLOCK && mRegion==(LOG_REGION_D)) /* Checkpointer lock operations */ || (op==LOG_WRLOCK && mRegion==(LOG_REGION_B|LOG_REGION_C)) || (op==LOG_WRLOCK && mRegion==(LOG_REGION_A)) - || (op==LOG_UNLOCK && mRegion==(LOG_REGION_A|LOG_REGION_B|LOG_REGION_C)) || (op==LOG_UNLOCK && mRegion==(LOG_REGION_B|LOG_REGION_C)) + || (op==LOG_UNLOCK && mRegion==(LOG_REGION_A|LOG_REGION_B|LOG_REGION_C)) ); + /* Assert that a connection never tries to go from an EXCLUSIVE to a + ** SHARED lock on a region. Moving from SHARED to EXCLUSIVE sometimes + ** happens though (when a region D reader upgrades to a writer). + */ + assert( op!=LOG_RDLOCK || 0==(pLog->lock.mLock & (mRegion<<8)) ); + sqlite3_mutex_enter(pSummary->mutex); - /* If obtaining (not releasing) a lock, check if there exist any - ** conflicting locks in process. Return SQLITE_BUSY in this case. + /* Calculate a mask of logs held by all connections in this process apart + ** from this one. The least significant byte of the mask contains a mask + ** of the SHARED logs held. The next least significant byte of the mask + ** indicates the EXCLUSIVE locks held. For example, to test if some other + ** connection is holding a SHARED lock on region A, or an EXCLUSIVE lock + ** on region C, do: + ** + ** hasSharedOnA = (mOther & (LOG_REGION_A<<0)); + ** hasExclusiveOnC = (mOther & (LOG_REGION_C<<8)); + ** + ** In all masks, if the bit in the EXCLUSIVE byte mask is set, so is the + ** corresponding bit in the SHARED mask. */ - if( op ){ - u32 mConflict = (mRegion<<8) | ((op==LOG_WRLOCK) ? mRegion : 0); - for(p=pSummary->pLock; p; p=p->pNext){ - if( p!=&pLog->lock && (p->mLock & mConflict) ){ - sqlite3_mutex_leave(pSummary->mutex); - return SQLITE_BUSY; - } + mOther = 0; + for(p=pSummary->pLock; p; p=p->pNext){ + assert( (p->mLock & (p->mLock<<8))==(p->mLock&0x0000FF00) ); + if( p!=&pLog->lock ){ + mOther |= p->mLock; } } - /* Determine the new lock mask for this log connection */ + /* If this call is to lock a region (not to unlock one), test if locks held + ** by any other connection in this process prevent the new locks from + ** begin granted. If so, exit the summary mutex and return SQLITE_BUSY. + */ + if( op && (mOther & (mRegion << (op==LOG_RDLOCK ? 8 : 0))) ){ + sqlite3_mutex_leave(pSummary->mutex); + return SQLITE_BUSY; + } + + /* Figure out the new log mask for this connection. */ switch( op ){ case LOG_UNLOCK: - mNewLock = (pLog->lock.mLock & ~(mRegion|(mRegion<<8))); + mNew = (pLog->lock.mLock & ~(mRegion|(mRegion<<8))); break; case LOG_RDLOCK: - mNewLock = ((pLog->lock.mLock & ~(mRegion<<8)) | mRegion); + mNew = (pLog->lock.mLock | mRegion); break; default: assert( op==LOG_WRLOCK ); - mNewLock = (pLog->lock.mLock | (mRegion<<8) | mRegion); + mNew = (pLog->lock.mLock | (mRegion<<8) | mRegion); break; } - /* Determine the current and desired sets of locks at the file level. */ - mNew = 0; - for(p=pSummary->pLock; p; p=p->pNext){ - assert( (p->mLock & (p->mLock<<8))==(p->mLock & 0x00000F00) ); - if( p!=&pLog->lock ) mNew |= p->mLock; + /* Now modify the locks held on the log-summary file descriptor. This + ** file descriptor is shared by all log connections in this process. + ** Therefore: + ** + ** + If one or more log connections in this process hold a SHARED lock + ** on a region, the file-descriptor should hold a SHARED lock on + ** the file region. + ** + ** + If a log connection in this process holds an EXCLUSIVE lock on a + ** region, the file-descriptor should also hold an EXCLUSIVE lock on + ** the region in question. + ** + ** If this is an LOG_UNLOCK operation, only regions for which no other + ** connection holds a lock should actually be unlocked. And if this + ** is a LOG_RDLOCK operation and other connections already hold all + ** the required SHARED locks, then no system call is required. + */ + if( op==LOG_UNLOCK ){ + mRegion = (mRegion & ~mOther); } - mOld = mNew | pLog->lock.mLock; - mNew = mNew | mNewLock; + if( (op==LOG_WRLOCK) + || (op==LOG_UNLOCK && mRegion) + || (op==LOG_RDLOCK && (mOther&mRegion)!=mRegion) + ){ + struct LockMap { + int iStart; /* Byte offset to start locking operation */ + int iLen; /* Length field for locking operation */ + } aMap[] = { + /* 0000 */ {0, 0}, /* 0001 */ {4, 1}, + /* 0010 */ {3, 1}, /* 0011 */ {3, 2}, + /* 0100 */ {2, 1}, /* 0101 */ {0, 0}, + /* 0110 */ {2, 2}, /* 0111 */ {2, 3}, + /* 1000 */ {1, 1}, /* 1001 */ {0, 0}, + /* 1010 */ {0, 0}, /* 1011 */ {0, 0}, + /* 1100 */ {1, 2}, /* 1101 */ {0, 0}, + /* 1110 */ {1, 3}, /* 1111 */ {0, 0} + }; + int rc; /* Return code of fcntl() */ + struct flock f; /* Locking operation */ + + assert( mRegion>8); - struct flock f; memset(&f, 0, sizeof(f)); f.l_type = (op==LOG_WRLOCK?F_WRLCK:(op==LOG_RDLOCK?F_RDLCK:F_UNLCK)); f.l_whence = SEEK_SET; - - if( mChange & LOG_REGION_A ) f.l_start = 12; - else if( mChange & LOG_REGION_B ) f.l_start = 13; - else if( mChange & LOG_REGION_C ) f.l_start = 14; - else if( mChange & LOG_REGION_D ) f.l_start = 15; - - if( mChange & LOG_REGION_D ) f.l_len = 16 - f.l_start; - else if( mChange & LOG_REGION_C ) f.l_len = 15 - f.l_start; - else if( mChange & LOG_REGION_B ) f.l_len = 14 - f.l_start; - else if( mChange & LOG_REGION_A ) f.l_len = 13 - f.l_start; + f.l_start = 32 + aMap[mRegion].iStart; + f.l_len = aMap[mRegion].iLen; rc = fcntl(pSummary->fd, F_SETLK, &f); if( rc!=0 ){ @@ -1075,7 +1120,7 @@ static int logLockRegion(Log *pLog, u32 mRegion, int op){ } } - pLog->lock.mLock = mNewLock; + pLog->lock.mLock = mNew; sqlite3_mutex_leave(pSummary->mutex); return SQLITE_OK; } @@ -1306,6 +1351,13 @@ int sqlite3LogWriteLock(Log *pLog, int op){ return rc; } + /* TODO: What if this is a region D reader? And after writing this + ** transaction it continues to hold a read-lock on the db? Maybe we + ** need to switch it to a region A reader here so that unlocking C|D + ** does not leave the connection with no lock at all. + */ + assert( pLog->isLocked!=LOG_REGION_D ); + if( memcmp(&pLog->hdr, pLog->pSummary->aData, sizeof(pLog->hdr)) ){ return SQLITE_BUSY; }