From: Jeffrey Altman Date: Tue, 26 Nov 2013 15:52:45 +0000 (-0500) Subject: Windows: Rationalize Freelance vs "fs flush*" X-Git-Tag: openafs-stable-1_8_0pre1~882 X-Git-Url: https://git.openafs.org/?p=openafs.git;a=commitdiff_plain;h=06fe2957348cfb2c571f2a0b099e09ef7e9fb3b0 Windows: Rationalize Freelance vs "fs flush*" Background: cm_scache_t objects representing Freelance volume (cell=-1, volume=-1) are special because they are populated from the Freelance mountpoint and symlink tables. These tables are in turn generated from the registry. The tables are regenerated on-demand after the execution of cm_noteLocalMountPointChange() which increments cm_data.fakeDirVersion which becomes the new data version value for the (-1.-1.1.1) directory object. The next time that cm_GetSCache() is called for a Freelance object the fake root directory is rebuilt by cm_InitFakeRootDir(). Since the vnode values are not persistent with regards to directory entry names the FileId unique is used to distinguish the various versions. cm_data.fakeUnique is incremented with each call to cm_InitFakeRootDir(). Each time cm_noteLocalMountPointChange() is executed the afs redirector is notified of the data version change which will force the redirector to rebuild its view of the directory the next time a path evaluation requires evaluation of the root (\afs). In other words, on the next request. If cm_noteLocalMountPointChange() is executed multiple times there is the possibility of a race between the redirector and the service. When the race is lost the redirector receives an invalidation event for -1.-1.1.1 as it is in the process of rebuilding the directory contents. The redirector ends up believing it has the most recent data version when it doesn't but the service no longer has Freelance mountpoint and symlink tables representing the requested data version. Hence, the mountpoints and symlinks end up as CM_SCACHETYPE_INVALID. fs flushfile and fs flushvolume both had explicit checks to prevent flushing Freelance objects because each call to cm_FlushFile() on a Freelance object would execute cm_noteLocalMountPointChange() triggering the race. The Problem: fs flushall is not executed on a specific object (volume or file). Therefore there was no explicit check to prevent execution against Freelance objects. For each cm_scache_t in the cache cm_FlushFile() is processed. If there are N Freelance mountpoints and symlinks, there will be N+1 calls to cm_noteLocalMountPointChange() in quick succession. Not only does this risk losing the race described above but it is extremely wasteful as the Freelance tables may be repeatedly regenerated. This Patchset: This patchset re-organizes the Freelance processing in the flush code paths. cm_FlushFile() and cm_FlushVolume() can simply no longer be successfully executed against a Freelance object. Both will return CM_ERROR_NOACCESS. "fs flush " is not permitted against Freelance objects. "fs flushvolume " will execute cm_noteLocalMountPointChange() once if the path is a Freelance object. "fs flushall" continues to execute cm_FlushFile() on all cm_scache_t objects. The calls on Freelance object will fail. After all cm_scache_t objects are flushed then cm_noteLocalMountPointChange() will be executed once to force the Freelance directory to be rebuilt. This patchset does not address the race but significantly reduces the likelihood the race will be lost. Change-Id: I298dad453432001b7b2e6f4533ddee17e041b02e Reviewed-on: http://gerrit.openafs.org/10521 Tested-by: BuildBot Reviewed-by: Jeffrey Altman --- diff --git a/src/WINNT/afsd/cm_ioctl.c b/src/WINNT/afsd/cm_ioctl.c index ae83382..87ce2b1 100644 --- a/src/WINNT/afsd/cm_ioctl.c +++ b/src/WINNT/afsd/cm_ioctl.c @@ -104,8 +104,7 @@ cm_FlushFile(cm_scache_t *scp, cm_user_t *userp, cm_req_t *reqp) #ifdef AFS_FREELANCE_CLIENT if ( scp->fid.cell == AFS_FAKE_ROOT_CELL_ID && scp->fid.volume == AFS_FAKE_ROOT_VOL_ID ) { - cm_noteLocalMountPointChange(FALSE); - return 0; + return CM_ERROR_NOACCESS; } #endif @@ -171,8 +170,7 @@ cm_FlushVolume(cm_user_t *userp, cm_req_t *reqp, afs_uint32 cell, afs_uint32 vol #ifdef AFS_FREELANCE_CLIENT if ( cell == AFS_FAKE_ROOT_CELL_ID && volume == AFS_FAKE_ROOT_VOL_ID ) { - cm_noteLocalMountPointChange(FALSE); - return 0; + return CM_ERROR_NOACCESS; } #endif @@ -192,7 +190,7 @@ cm_FlushVolume(cm_user_t *userp, cm_req_t *reqp, afs_uint32 cell, afs_uint32 vol } lock_ReleaseWrite(&cm_scacheLock); - return code; + return 0; } /* @@ -584,7 +582,11 @@ cm_IoctlFlushAllVolumes(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_req_t } lock_ReleaseWrite(&cm_scacheLock); - return code; +#ifdef AFS_FREELANCE_CLIENT + cm_noteLocalMountPointChange(FALSE); +#endif + + return 0; } /* @@ -596,21 +598,15 @@ cm_IoctlFlushAllVolumes(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_req_t afs_int32 cm_IoctlFlushVolume(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scache_t *scp, cm_req_t *reqp) { - afs_int32 code; - afs_uint32 volume; - afs_uint32 cell; #ifdef AFS_FREELANCE_CLIENT if ( scp->fid.cell == AFS_FAKE_ROOT_CELL_ID && scp->fid.volume == AFS_FAKE_ROOT_VOL_ID ) { - code = CM_ERROR_NOACCESS; - } else -#endif - { - volume = scp->fid.volume; - cell = scp->fid.cell; - code = cm_FlushVolume(userp, reqp, cell, volume); + cm_noteLocalMountPointChange(FALSE); + return 0; } - return code; +#endif + + return cm_FlushVolume(userp, reqp, scp->fid.cell, scp->fid.volume); } /* @@ -626,13 +622,11 @@ cm_IoctlFlushFile(struct cm_ioctl *ioctlp, struct cm_user *userp, cm_scache_t *s #ifdef AFS_FREELANCE_CLIENT if ( scp->fid.cell == AFS_FAKE_ROOT_CELL_ID && scp->fid.volume == AFS_FAKE_ROOT_VOL_ID ) { - code = CM_ERROR_NOACCESS; - } else -#endif - { - cm_FlushFile(scp, userp, reqp); + return CM_ERROR_NOACCESS; } - return 0; +#endif + + return cm_FlushFile(scp, userp, reqp); }