Windows: MergeStatus before SyncOpDone
authorJeffrey Altman <jaltman@your-file-system.com>
Mon, 27 Jun 2011 13:31:54 +0000 (09:31 -0400)
committerJeffrey Altman <jaltman@openafs.org>
Mon, 27 Jun 2011 15:18:55 +0000 (08:18 -0700)
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 <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>
Tested-by: Jeffrey Altman <jaltman@openafs.org>

src/WINNT/afsd/cm_dcache.c
src/WINNT/afsd/cm_vnodeops.c

index 9259fff..de3dea1 100644 (file)
@@ -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;
 }
index e12b0ab..0f4dd05 100644 (file)
@@ -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