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.

Reviewed-on: http://gerrit.openafs.org/4891
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
(cherry-picked from 51fa590e704c77c0e9ba873ecb854448885030a5)

Change-Id: Ic50241081e52b70bf7b6bd7d92df205f7184f7c8
Reviewed-on: http://gerrit.openafs.org/4895
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>

diff --git a/src/WINNT/afsd/cm_dcache.c b/src/WINNT/afsd/cm_dcache.c
index 038ab11e4c64bffb69909f129c631b677ebffbb2..d5e138a39f23d3c34807de9a049af16fa994b641 100644 (file)
--- a/src/WINNT/afsd/cm_dcache.c
+++ b/src/WINNT/afsd/cm_dcache.c
@@ -345,7 +345,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;
@@ -385,6 +384,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);
 
@@ -487,8 +488,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;
         /*
@@ -506,6 +505,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 371f441fdf11fae90f005cb35b8702d7a91e3ca5..99054709810e262bf141b2d94ce5cb6a0ee3e291 100644 (file)
--- a/src/WINNT/afsd/cm_vnodeops.c
+++ b/src/WINNT/afsd/cm_vnodeops.c
@@ -1664,7 +1664,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) {
@@ -1674,6 +1673,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) {
@@ -2757,10 +2757,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.
@@ -2869,10 +2869,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
@@ -3050,10 +3049,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
@@ -3175,10 +3173,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) {
@@ -3276,10 +3274,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) {
@@ -3424,11 +3421,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) {
@@ -3743,11 +3740,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)) {
@@ -3784,10 +3781,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