Windows: Reduce complexity of Freelance Callback Logic

Over the years the processing of the Freelance callbacks have
added functionality that behaves much more like FetchStatus checks
to a file server.  If the data version of the object has changed,
get the new data.  Given that is the case, we can remove much of
the original refresh logic that is rather race prone.   Say goodbye
to cm_fakeGettingCallback and cm_fakeDirCallback.

LICENSE MIT

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

diff --git a/src/WINNT/afsd/cm_callback.c b/src/WINNT/afsd/cm_callback.c
index f8f587c..07bd3fe 100644 (file)
--- a/src/WINNT/afsd/cm_callback.c
+++ b/src/WINNT/afsd/cm_callback.c
@@ -1538,42 +1538,16 @@ void cm_InitCallback(void)
 int cm_HaveCallback(cm_scache_t *scp)
 {
 #ifdef AFS_FREELANCE_CLIENT
-    // yj: we handle callbacks specially for callbacks on the root directory
-    // Since it's local, we almost always say that we have callback on it
-    // The only time we send back a 0 is if we're need to initialize or
-    // reinitialize the fake directory
-
-    // There are 2 state variables cm_fakeGettingCallback and cm_fakeDirCallback
-    // cm_fakeGettingCallback is 1 if we're in the process of initialization and
-    // hence should return false. it's 0 otherwise
-    // cm_fakeDirCallback is 0 if we haven't loaded the fake directory, it's 1
-    // if the fake directory is loaded and this is the first time cm_HaveCallback
-    // is called since then. We return false in this case to allow cm_GetCallback
-    // to be called because cm_GetCallback has some initialization work to do.
-    // If cm_fakeDirCallback is 2, then it means that the fake directory is in
-    // good shape and we simply return true, provided no change is detected.
-    int fdc, fgc;
-
     if (cm_freelanceEnabled && 
-         scp->fid.cell==AFS_FAKE_ROOT_CELL_ID && scp->fid.volume==AFS_FAKE_ROOT_VOL_ID) {
-        lock_ObtainMutex(&cm_Freelance_Lock);
-        fdc = cm_fakeDirCallback;
-        fgc = cm_fakeGettingCallback;
-        lock_ReleaseMutex(&cm_Freelance_Lock);
-           
-        if (fdc==1) {  // first call since init
-            return 0;
-        } else if (fdc==2 && !fgc) {   // we're in good shape
-            if (cm_getLocalMountPointChange()) {       // check for changes
-                cm_clearLocalMountPointChange(); // clear the changefile
-                lock_ReleaseWrite(&scp->rw);      // this is re-locked in reInitLocalMountPoints
-                cm_reInitLocalMountPoints();   // start reinit
-                lock_ObtainWrite(&scp->rw);      // now get the lock back 
-                return 0;
-            }
-            return (cm_data.fakeDirVersion == scp->dataVersion);
+        scp->fid.cell==AFS_FAKE_ROOT_CELL_ID &&
+        scp->fid.volume==AFS_FAKE_ROOT_VOL_ID) {
+        if (cm_getLocalMountPointChange()) {
+            cm_clearLocalMountPointChange();
+            lock_ReleaseWrite(&scp->rw);
+            cm_reInitLocalMountPoints();
+            lock_ObtainWrite(&scp->rw);
         }
-        return 0;
+        return (cm_data.fakeDirVersion == scp->dataVersion);
     }
 #endif
     if (cm_readonlyVolumeVersioning &&
@@ -1787,7 +1761,7 @@ long cm_GetCallback(cm_scache_t *scp, struct cm_user *userp,
 #ifdef AFS_FREELANCE_CLIENT
     // The case where a callback is needed on /afs is handled
     // specially. We need to fetch the status by calling
-    // cm_MergeStatus and mark that cm_fakeDirCallback is 2
+    // cm_MergeStatus
     if (cm_freelanceEnabled &&
         (scp->fid.cell==AFS_FAKE_ROOT_CELL_ID &&
          scp->fid.volume==AFS_FAKE_ROOT_VOL_ID)) {
@@ -1798,28 +1772,12 @@ long cm_GetCallback(cm_scache_t *scp, struct cm_user *userp,
             goto done;
         syncop_done = 1;
 
-        if (cm_fakeDirCallback != 2) {
-            // Start by indicating that we're in the process
-            // of fetching the callback
-            lock_ObtainMutex(&cm_Freelance_Lock);
-            osi_Log0(afsd_logp,"GetCallback Freelance fakeGettingCallback=1");
-            cm_fakeGettingCallback = 1;
-            lock_ReleaseMutex(&cm_Freelance_Lock);
-
+        if (scp->dataVersion != cm_data.fakeDirVersion) {
             memset(&afsStatus, 0, sizeof(afsStatus));
+            memset(&volSync, 0, sizeof(volSync));
 
             // Fetch the status info 
             cm_MergeStatus(NULL, scp, &afsStatus, &volSync, userp, reqp, 0);
-
-            // Indicate that the callback is not done
-            lock_ObtainMutex(&cm_Freelance_Lock);
-            osi_Log0(afsd_logp,"GetCallback Freelance fakeDirCallback=2");
-            cm_fakeDirCallback = 2;
-
-            // Indicate that we're no longer fetching the callback
-            osi_Log0(afsd_logp,"GetCallback Freelance fakeGettingCallback=0");
-            cm_fakeGettingCallback = 0;
-            lock_ReleaseMutex(&cm_Freelance_Lock);
         }
         goto done;
     }

diff --git a/src/WINNT/afsd/cm_freelance.c b/src/WINNT/afsd/cm_freelance.c
index 8cbb594..4a4a84a 100644 (file)
--- a/src/WINNT/afsd/cm_freelance.c
+++ b/src/WINNT/afsd/cm_freelance.c
@@ -24,8 +24,6 @@ extern void afsi_log(char *pattern, ...);
 static unsigned int cm_noLocalMountPoints = 0;
 char * cm_FakeRootDir = NULL;
 int cm_fakeDirSize = 0;
-int cm_fakeDirCallback=0;
-int cm_fakeGettingCallback=0;
 static cm_localMountPoint_t* cm_localMountPoints;
 osi_mutex_t cm_Freelance_Lock;
 static int cm_localMountPointChangeFlag = 0;
@@ -176,6 +174,10 @@ void cm_InitFreelance() {
     // then we make a call to InitFakeRootDir to create
     // a fake root directory based on the local mount points
     cm_InitFakeRootDir();
+
+    // increment the fakeDirVersion to force status updates for
+    // all cached Freelance objects
+    cm_data.fakeDirVersion++;
     // --- end of yj code
     lock_ReleaseMutex(&cm_Freelance_Lock);
 
@@ -368,8 +370,7 @@ void cm_InitFakeRootDir() {
     }
 
     // we know the fakeDir is setup properly, so we claim that we have callback
-    osi_Log0(afsd_logp,"cm_InitFakeRootDir fakeDirCallback=1");
-    cm_fakeDirCallback=1;
+    osi_Log0(afsd_logp,"cm_InitFakeRootDir completed!");
 
     // when we get here, we've set up everything! done!
 }

diff --git a/src/WINNT/afsd/cm_scache.c b/src/WINNT/afsd/cm_scache.c
index 9a68fdb..75ba858 100644 (file)
--- a/src/WINNT/afsd/cm_scache.c
+++ b/src/WINNT/afsd/cm_scache.c
@@ -766,9 +766,9 @@ long cm_GetSCache(cm_fid_t *fidp, cm_scache_t **outScpp, cm_user_t *userp,
         lock_ReleaseWrite(&cm_scacheLock);
         osi_Log0(afsd_logp,"cm_GetSCache Freelance and special");
 
-        if (cm_getLocalMountPointChange()) {   // check for changes
-            cm_clearLocalMountPointChange();    // clear the changefile
-            cm_reInitLocalMountPoints();       // start reinit
+        if (cm_getLocalMountPointChange()) {
+            cm_clearLocalMountPointChange();
+            cm_reInitLocalMountPoints();
         }
 
         lock_ObtainWrite(&cm_scacheLock);
@@ -1248,18 +1248,7 @@ long cm_SyncOp(cm_scache_t *scp, cm_buf_t *bufp, cm_user_t *userp, cm_req_t *req
             }
         }
 
-        // yj: modified this so that callback only checked if we're
-        // not checking something on /afs
-        /* fix the conditional to match the one in cm_HaveCallback */
-        if ((flags & CM_SCACHESYNC_NEEDCALLBACK)
-#ifdef AFS_FREELANCE_CLIENT
-             && (!cm_freelanceEnabled || 
-                  !(scp->fid.vnode==0x1 && scp->fid.unique==0x1) ||
-                  scp->fid.cell!=AFS_FAKE_ROOT_CELL_ID ||
-                  scp->fid.volume!=AFS_FAKE_ROOT_VOL_ID ||
-                  cm_fakeDirCallback < 2)
-#endif /* AFS_FREELANCE_CLIENT */
-             ) {
+        if ((flags & CM_SCACHESYNC_NEEDCALLBACK)) {
             if ((flags & CM_SCACHESYNC_FORCECB) || !cm_HaveCallback(scp)) {
                 osi_Log1(afsd_logp, "CM SyncOp getting callback on scp 0x%p",
                           scp);