From 51fa590e704c77c0e9ba873ecb854448885030a5 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Mon, 27 Jun 2011 09:31:54 -0400 Subject: [PATCH] Windows: MergeStatus before SyncOpDone cm_SyncOp/cm_SyncOpDone is used to synchronize the RPC processing to ensure that calls which are in conflict cannot occur at the same time but also to ensure that the ordering of operations is consistent. cm_MergeStatus() was in many cases executed after cm_SyncOpDone() removed the synchronization barrier which in turn permitted status information to be applied out of order. Side effects could have included data loss due to client side file truncation. More commonly two StoreData RPCs would have their status information applied out of order forcing the cache manager to invalidate all of the cached data for the file. Change-Id: I8073da701cf11cd0df2b901e81180de7c193ae23 Reviewed-on: http://gerrit.openafs.org/4891 Tested-by: BuildBot Reviewed-by: Jeffrey Altman Tested-by: Jeffrey Altman --- src/WINNT/afsd/cm_dcache.c | 6 +++--- src/WINNT/afsd/cm_vnodeops.c | 27 ++++++++++++--------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c index 9259fff..de3dea1 100644 --- a/src/WINNT/afsd/cm_dcache.c +++ b/src/WINNT/afsd/cm_dcache.c @@ -349,7 +349,6 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags, lock_ObtainWrite(&scp->rw); cm_ReleaseBIOD(&biod, 1, code, 1); - cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL); if (code == 0) { osi_hyper_t t; @@ -389,6 +388,8 @@ long cm_BufWrite(void *vscp, osi_hyper_t *offsetp, long length, long flags, else if (code == CM_ERROR_QUOTA) scp->flags |= CM_SCACHEFLAG_OVERQUOTA; } + cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL); + if (!scp_locked) lock_ReleaseWrite(&scp->rw); @@ -491,8 +492,6 @@ long cm_StoreMini(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) /* now, clean up our state */ lock_ObtainWrite(&scp->rw); - cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL); - if (code == 0) { osi_hyper_t t; /* @@ -510,6 +509,7 @@ long cm_StoreMini(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) scp->mask &= ~CM_SCACHEMASK_LENGTH; cm_MergeStatus(NULL, scp, &outStatus, &volSync, userp, reqp, CM_MERGEFLAG_STOREDATA); } + cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STOREDATA_EXCL); return code; } diff --git a/src/WINNT/afsd/cm_vnodeops.c b/src/WINNT/afsd/cm_vnodeops.c index e12b0ab..0f4dd05 100644 --- a/src/WINNT/afsd/cm_vnodeops.c +++ b/src/WINNT/afsd/cm_vnodeops.c @@ -1667,7 +1667,6 @@ long cm_Unlink(cm_scache_t *dscp, fschar_t *fnamep, clientchar_t * cnamep, } lock_ObtainWrite(&dscp->rw); cm_dnlcRemove(dscp, cnamep); - cm_SyncOpDone(dscp, NULL, sflags); if (code == 0) { cm_MergeStatus(NULL, dscp, &newDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); } else if (code == CM_ERROR_NOSUCHFILE) { @@ -1677,6 +1676,7 @@ long cm_Unlink(cm_scache_t *dscp, fschar_t *fnamep, clientchar_t * cnamep, */ dscp->cbServerp = NULL; } + cm_SyncOpDone(dscp, NULL, sflags); lock_ReleaseWrite(&dscp->rw); if (code == 0 && cm_CheckDirOpForSingleChange(&dirop) && cnamep) { @@ -2760,10 +2760,10 @@ long cm_SetAttr(cm_scache_t *scp, cm_attr_t *attrp, cm_user_t *userp, osi_Log0(afsd_logp, "CALL StoreStatus SUCCESS"); lock_ObtainWrite(&scp->rw); - cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STORESTATUS); if (code == 0) cm_MergeStatus(NULL, scp, &afsOutStatus, &volSync, userp, reqp, CM_MERGEFLAG_FORCE|CM_MERGEFLAG_STOREDATA); + cm_SyncOpDone(scp, NULL, CM_SCACHESYNC_STORESTATUS); /* if we're changing the mode bits, discard the ACL cache, * since we changed the mode bits. @@ -2872,10 +2872,9 @@ long cm_Create(cm_scache_t *dscp, clientchar_t *cnamep, long flags, cm_attr_t *a dirop.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&dscp->rw); - cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); - if (code == 0) { + if (code == 0) cm_MergeStatus(NULL, dscp, &updatedDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); - } + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&dscp->rw); /* now try to create the file's entry, too, but be careful to @@ -3053,10 +3052,9 @@ long cm_MakeDir(cm_scache_t *dscp, clientchar_t *cnamep, long flags, cm_attr_t * dirop.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&dscp->rw); - cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); - if (code == 0) { + if (code == 0) cm_MergeStatus(NULL, dscp, &updatedDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); - } + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&dscp->rw); /* now try to create the new dir's entry, too, but be careful to @@ -3178,10 +3176,10 @@ long cm_Link(cm_scache_t *dscp, clientchar_t *cnamep, cm_scache_t *sscp, long fl dirop.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&dscp->rw); - cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); if (code == 0) { cm_MergeStatus(NULL, dscp, &updatedDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); } + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&dscp->rw); if (code == 0) { @@ -3279,10 +3277,9 @@ long cm_SymLink(cm_scache_t *dscp, clientchar_t *cnamep, fschar_t *contentsp, lo dirop.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&dscp->rw); - cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); - if (code == 0) { + if (code == 0) cm_MergeStatus(NULL, dscp, &updatedDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); - } + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&dscp->rw); if (code == 0) { @@ -3427,11 +3424,11 @@ long cm_RemoveDir(cm_scache_t *dscp, fschar_t *fnamep, clientchar_t *cnamep, cm_ dirop.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&dscp->rw); - cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); if (code == 0) { cm_dnlcRemove(dscp, cnamep); cm_MergeStatus(NULL, dscp, &updatedDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); } + cm_SyncOpDone(dscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&dscp->rw); if (code == 0) { @@ -3746,11 +3743,11 @@ long cm_Rename(cm_scache_t *oldDscp, fschar_t *oldNamep, clientchar_t *cOldNamep oldDirOp.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&oldDscp->rw); - cm_SyncOpDone(oldDscp, NULL, CM_SCACHESYNC_STOREDATA); if (code == 0) cm_MergeStatus(NULL, oldDscp, &updatedOldDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); + cm_SyncOpDone(oldDscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&oldDscp->rw); if (code == 0 && cm_CheckDirOpForSingleChange(&oldDirOp)) { @@ -3787,10 +3784,10 @@ long cm_Rename(cm_scache_t *oldDscp, fschar_t *oldNamep, clientchar_t *cOldNamep newDirOp.lockType = CM_DIRLOCK_WRITE; } lock_ObtainWrite(&newDscp->rw); - cm_SyncOpDone(newDscp, NULL, CM_SCACHESYNC_STOREDATA); if (code == 0) cm_MergeStatus(NULL, newDscp, &updatedNewDirStatus, &volSync, userp, reqp, CM_MERGEFLAG_DIROP); + cm_SyncOpDone(newDscp, NULL, CM_SCACHESYNC_STOREDATA); lock_ReleaseWrite(&newDscp->rw); #if 0 -- 1.7.1