From: Jeffrey Altman Date: Fri, 8 Aug 2008 17:46:35 +0000 (+0000) Subject: DEVEL15-windows-lock-corrections-20080808 X-Git-Tag: openafs-devel-1_5_52~31 X-Git-Url: https://git.michaelhowe.org/gitweb/?a=commitdiff_plain;h=8785d788c6bdbf0f731985b5956697d718b55b3e;p=packages%2Fo%2Fopenafs.git DEVEL15-windows-lock-corrections-20080808 LICENSE MIT Derrick helped identify a few locations where rw or mx locks where not properly being tracked. As a result there were some locations in which an assertion could be thrown due to releasing the wrong type of lock. Also added lock_AssertXXX calls to some locations to ensure that the correct lock type is being held when the calls are made. volume location updates, cm_SyncOp, cm_SyncOpDone. (cherry picked from commit 423cdb708f21dcbc28f6563c7c49069f6a6ec155) --- diff --git a/src/WINNT/afsd/cm_daemon.c b/src/WINNT/afsd/cm_daemon.c index a49c2891c..b930a945e 100644 --- a/src/WINNT/afsd/cm_daemon.c +++ b/src/WINNT/afsd/cm_daemon.c @@ -132,22 +132,14 @@ void cm_BkgDaemon(void * parm) #ifdef DEBUG_REFCOUNT osi_Log2(afsd_logp,"cm_BkgDaemon (after) scp 0x%x ref %d",rp->scp, rp->scp->refCount); #endif - if (code == 0) { - cm_ReleaseUser(rp->userp); - cm_ReleaseSCache(rp->scp); - free(rp); - } - - lock_ObtainWrite(&cm_daemonLock); /* - * Keep the following list synchronized with the - * error code list in cm_BkgStore - */ + * Keep the following list synchronized with the + * error code list in cm_BkgStore. + * cm_SyncOpDone(CM_SCACHESYNC_ASYNCSTORE) will be called there unless + * one of these errors has occurred. + */ switch ( code ) { - case 0: /* success */ - osi_Log1(afsd_logp,"cm_BkgDaemon SUCCESS: request 0x%p", rp); - break; case CM_ERROR_TIMEDOUT: /* or server restarting */ case CM_ERROR_RETRY: case CM_ERROR_WOULDBLOCK: @@ -157,18 +149,26 @@ void cm_BkgDaemon(void * parm) case CM_ERROR_PARTIALWRITE: osi_Log2(afsd_logp,"cm_BkgDaemon re-queueing failed request 0x%p code 0x%x", rp, code); + lock_ObtainWrite(&cm_daemonLock); cm_bkgQueueCount++; osi_QAddT((osi_queue_t **) &cm_bkgListp, (osi_queue_t **)&cm_bkgListEndp, &rp->q); break; - default: - osi_Log2(afsd_logp,"cm_BkgDaemon FAILED: request dropped 0x%p code 0x%x", + case 0: /* success */ + default: /* other error */ + if (code == 0) + osi_Log1(afsd_logp,"cm_BkgDaemon SUCCESS: request 0x%p", rp); + else + osi_Log2(afsd_logp,"cm_BkgDaemon FAILED: request dropped 0x%p code 0x%x", rp, code); + cm_ReleaseUser(rp->userp); + cm_ReleaseSCache(rp->scp); + free(rp); + lock_ObtainWrite(&cm_daemonLock); } } lock_ReleaseWrite(&cm_daemonLock); thrd_SetEvent(cm_BkgDaemon_ShutdownEvent[daemonID]); - } void cm_QueueBKGRequest(cm_scache_t *scp, cm_bkgProc_t *procp, afs_uint32 p1, afs_uint32 p2, afs_uint32 p3, afs_uint32 p4, diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c index 8b582c73f..86598e1fb 100644 --- a/src/WINNT/afsd/cm_scache.c +++ b/src/WINNT/afsd/cm_scache.c @@ -1047,6 +1047,8 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req int wakeupCycle; int getAccessRights = 1; + lock_AssertWrite(&scp->rw); + /* lookup this first */ bufLocked = flags & CM_SCACHESYNC_BUFLOCKED; diff --git a/src/WINNT/afsd/cm_volume.c b/src/WINNT/afsd/cm_volume.c index 9cf6eb04e..c9d3eb2a5 100644 --- a/src/WINNT/afsd/cm_volume.c +++ b/src/WINNT/afsd/cm_volume.c @@ -185,6 +185,8 @@ long cm_UpdateVolumeLocation(struct cm_cell *cellp, cm_user_t *userp, cm_req_t * #endif afs_uint32 volType; + lock_AssertWrite(&volp->rw); + #ifdef AFS_FREELANCE_CLIENT if ( cellp->cellID == AFS_FAKE_ROOT_CELL_ID && volp->vol[RWVOL].ID == AFS_FAKE_ROOT_VOL_ID ) { diff --git a/src/WINNT/afsd/rawops.c b/src/WINNT/afsd/rawops.c index 0848621be..1debe4c15 100644 --- a/src/WINNT/afsd/rawops.c +++ b/src/WINNT/afsd/rawops.c @@ -342,10 +342,11 @@ long WriteData(cm_scache_t *scp, osi_hyper_t offset, long count, char *op, if (code == 0 && doWriteBack) { lock_ObtainWrite(&scp->rw); - cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_ASYNCSTORE); + code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_ASYNCSTORE); lock_ReleaseWrite(&scp->rw); - cm_QueueBKGRequest(scp, cm_BkgStore, writeBackOffset.LowPart, - writeBackOffset.HighPart, cm_chunkSize, 0, userp); + if (code == 0) + cm_QueueBKGRequest(scp, cm_BkgStore, writeBackOffset.LowPart, + writeBackOffset.HighPart, cm_chunkSize, 0, userp); } /* cm_SyncOpDone is called when cm_BkgStore completes */ diff --git a/src/WINNT/afsd/smb3.c b/src/WINNT/afsd/smb3.c index 26b165032..37bd818fc 100644 --- a/src/WINNT/afsd/smb3.c +++ b/src/WINNT/afsd/smb3.c @@ -2848,7 +2848,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t cm_user_t *userp; cm_space_t *spacep; cm_scache_t *scp, *dscp; - int scp_mx_held = 0; + int scp_rw_held = 0; int delonclose = 0; long code = 0; clientchar_t *pathp; @@ -3012,14 +3012,16 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t #endif /* DFS_SUPPORT */ lock_ObtainWrite(&scp->rw); - scp_mx_held = 1; + scp_rw_held = 2; code = cm_SyncOp(scp, NULL, userp, &req, 0, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); - if (code) goto done; + if (code) + goto done; cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); lock_ConvertWToR(&scp->rw); + scp_rw_held = 1; len = 0; @@ -3081,7 +3083,7 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t if (fidp) { lock_ReleaseRead(&scp->rw); - scp_mx_held = 0; + scp_rw_held = 0; lock_ObtainMutex(&fidp->mx); delonclose = fidp->flags & SMB_FID_DELONCLOSE; lock_ReleaseMutex(&fidp->mx); @@ -3125,8 +3127,15 @@ long smb_ReceiveTran2QPathInfo(smb_vc_t *vcp, smb_tran2Packet_t *p, smb_packet_t /* send and free the packets */ done: - if (scp_mx_held) + switch (scp_rw_held) { + case 1: lock_ReleaseRead(&scp->rw); + break; + case 2: + lock_ReleaseWrite(&scp->rw); + break; + } + scp_rw_held = 0; cm_ReleaseSCache(scp); cm_ReleaseUser(userp); if (code == 0) { @@ -4011,6 +4020,8 @@ smb_ApplyV3DirListPatches(cm_scache_t *dscp, smb_dirListPatch_t **dirPatchespp, lock_ObtainWrite(&dscp->rw); code = cm_SyncOp(dscp, NULL, userp, reqp, PRSFS_READ, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); + if (code == 0) + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); lock_ReleaseWrite(&dscp->rw); if (code == CM_ERROR_NOACCESS) { mustFake = 1; @@ -6237,6 +6248,7 @@ long smb_ReceiveV3GetAttributes(smb_vc_t *vcp, smb_packet_t *inp, smb_packet_t * cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_NEEDCALLBACK | CM_SCACHESYNC_GETSTATUS); lock_ConvertWToR(&scp->rw); + readlock = 1; /* decode times. We need a search time, but the response to this * call provides the date first, not the time, as returned in the