From 3ca42749357edc97922918d04558deb268fe78f0 Mon Sep 17 00:00:00 2001 From: Peter Scott Date: Fri, 23 Dec 2011 17:00:57 -0700 Subject: [PATCH] Windows: Avoid bottleneck on VolumeLock The VolumeLock resource was obtained during each AFSParseName() and held across a wide range of operations including volume info queries, renames, and extent requests. These operations can take a long time to complete and as long as the VolumeLock was held exclusively there could only be one operation in flight at a time on a given volume. This significantly reduced the parallelism of operations. The VolumeLock was not required in almost all cases. This patchset adjusts the use of the VolumeLock and avoids the bottleneck. Change-Id: I9d60fe41d157b9e315aeaa15feee8d1e0d4ded4c Reviewed-on: http://gerrit.openafs.org/6420 Tested-by: BuildBot Tested-by: Jeffrey Altman Reviewed-by: Jeffrey Altman --- src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp | 4 +- src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp | 1 - src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp | 20 ++----- src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp | 24 -------- src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp | 79 ++++---------------------- src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp | 4 +- 6 files changed, 20 insertions(+), 112 deletions(-) diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp index c74b568..e8d7356 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp @@ -379,7 +379,7 @@ AFSCommonCreate( IN PDEVICE_OBJECT DeviceObject, } // - // We have our root node shared + // We have a reference on the root volume // bReleaseVolume = TRUE; @@ -1241,8 +1241,6 @@ try_exit: "AFSCommonCreate Decrement count on Volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } // diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp index 6d79ea8..cda97c5 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp @@ -803,7 +803,6 @@ AFSRemoveVolume( IN AFSVolumeCB *VolumeCB) if( ExIsResourceAcquiredLite( VolumeCB->VolumeLock)) { - AFSReleaseResource( VolumeCB->VolumeLock); } diff --git a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp index 57c7909..4abd76e 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp @@ -482,7 +482,6 @@ AFSSetFileInfo( IN PDEVICE_OBJECT LibDeviceObject, PFILE_OBJECT pFileObject = NULL; BOOLEAN bReleaseMain = FALSE; BOOLEAN bUpdateFileInfo = FALSE; - BOOLEAN bReleaseVolumeLock = FALSE; AFSFileID stParentFileId; __try @@ -507,15 +506,6 @@ AFSSetFileInfo( IN PDEVICE_OBJECT LibDeviceObject, bCanQueueRequest = !(IoIsOperationSynchronous( Irp) | (KeGetCurrentIrql() != PASSIVE_LEVEL)); FileInformationClass = pIrpSp->Parameters.SetFile.FileInformationClass; - if( FileInformationClass == FileRenameInformation) - { - - AFSAcquireExcl( pFcb->ObjectInformation->VolumeCB->VolumeLock, - TRUE); - - bReleaseVolumeLock = TRUE; - } - // // Grab the Fcb EXCL // @@ -676,11 +666,6 @@ try_exit: AFSReleaseResource( &pFcb->NPFcb->Resource); } - if( bReleaseVolumeLock) - { - AFSReleaseResource( pFcb->ObjectInformation->VolumeCB->VolumeLock); - } - if( NT_SUCCESS( ntStatus) && bUpdateFileInfo) { @@ -2440,6 +2425,9 @@ AFSSetRenameInfo( IN PIRP Irp) &stNewFid)) { + AFSAcquireExcl( pSrcObject->VolumeCB->ObjectInfoTree.TreeLock, + TRUE); + // // Remove the old information entry // @@ -2468,6 +2456,8 @@ AFSSetRenameInfo( IN PIRP Irp) AFSInsertHashEntry( pSrcObject->VolumeCB->ObjectInfoTree.TreeHead, &pSrcObject->TreeEntry); } + + AFSReleaseResource( pSrcObject->VolumeCB->ObjectInfoTree.TreeLock); } // diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp index f037664..b2a5d23 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp @@ -5780,9 +5780,6 @@ AFSRetrieveFileAttributes( IN AFSDirectoryCB *ParentDirectoryCB, pVolumeCB = ParentDirectoryCB->ObjectInformation->VolumeCB; - AFSAcquireShared( pVolumeCB->VolumeLock, - TRUE); - pParentDirEntry = ParentDirectoryCB; } else @@ -5860,9 +5857,6 @@ AFSRetrieveFileAttributes( IN AFSDirectoryCB *ParentDirectoryCB, pVolumeCB = AFSGlobalRoot; - AFSAcquireShared( pVolumeCB->VolumeLock, - TRUE); - pParentDirEntry = AFSGlobalRoot->DirectoryCB; } @@ -5920,8 +5914,6 @@ AFSRetrieveFileAttributes( IN AFSDirectoryCB *ParentDirectoryCB, "AFSRetrieveFileAttributes Decrement count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } if( pDirectoryEntry != NULL) @@ -6032,8 +6024,6 @@ try_exit: "AFSRetrieveFileAttributes Decrement2 count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } if( pNameArray != NULL) @@ -6441,9 +6431,6 @@ AFSEvaluateRootEntry( IN AFSDirectoryCB *DirectoryCB, pVolumeCB = AFSGlobalRoot; - AFSAcquireShared( pVolumeCB->VolumeLock, - TRUE); - pParentDirEntry = AFSGlobalRoot->DirectoryCB; InterlockedIncrement( &pVolumeCB->VolumeReferenceCount); @@ -6496,8 +6483,6 @@ AFSEvaluateRootEntry( IN AFSDirectoryCB *DirectoryCB, "AFSEvaluateRootEntry Decrement count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } if( pDirectoryEntry != NULL) @@ -6557,8 +6542,6 @@ try_exit: "AFSEvaluateRootEntry2 Decrement count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } if( pNameArray != NULL) @@ -7742,9 +7725,6 @@ AFSGetObjectStatus( IN AFSGetStatusInfoCB *GetStatusInfo, pVolumeCB = AFSGlobalRoot; - AFSAcquireShared( pVolumeCB->VolumeLock, - TRUE); - pParentDirEntry = AFSGlobalRoot->DirectoryCB; // @@ -7802,8 +7782,6 @@ AFSGetObjectStatus( IN AFSGetStatusInfoCB *GetStatusInfo, "AFSGetObjectStatus Decrement count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } if( pDirectoryEntry != NULL) @@ -7859,8 +7837,6 @@ AFSGetObjectStatus( IN AFSGetStatusInfoCB *GetStatusInfo, "AFSRetrieveFileAttributes Decrement2 count on volume %08lX Cnt %d\n", pVolumeCB, pVolumeCB->VolumeReferenceCount); - - AFSReleaseResource( pVolumeCB->VolumeLock); } } diff --git a/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp b/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp index 8bb007b..258e671 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp @@ -663,13 +663,8 @@ AFSLocateNameEntry( IN GUID *AuthGroup, pCurrentVolume, pCurrentVolume->VolumeReferenceCount); - AFSReleaseResource( pCurrentVolume->VolumeLock); - pCurrentVolume = AFSGlobalRoot; - AFSAcquireShared( pCurrentVolume->VolumeLock, - TRUE); - InterlockedIncrement( &pCurrentVolume->VolumeReferenceCount); AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING, @@ -826,8 +821,6 @@ AFSLocateNameEntry( IN GUID *AuthGroup, pCurrentVolume, pCurrentVolume->VolumeReferenceCount); - AFSReleaseResource( pCurrentVolume->VolumeLock); - ntStatus = AFSBuildMountPointTarget( AuthGroup, pDirEntry, &pCurrentVolume); @@ -857,8 +850,6 @@ AFSLocateNameEntry( IN GUID *AuthGroup, ASSERT( pCurrentVolume->VolumeReferenceCount > 1); - ASSERT( ExIsResourceAcquiredLite( pCurrentVolume->VolumeLock)); - // // Replace the current name for the mp with the volume root of the target // @@ -1875,8 +1866,6 @@ try_exit: ASSERT( pCurrentVolume->VolumeReferenceCount > 1); - ASSERT( ExIsResourceAcquiredLite( pCurrentVolume->VolumeLock)); - InterlockedDecrement( &pCurrentVolume->VolumeReferenceCount); AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING, @@ -1884,8 +1873,6 @@ try_exit: "AFSLocateNameEntry Decrement3 count on volume %08lX Cnt %d\n", pCurrentVolume, pCurrentVolume->VolumeReferenceCount); - - AFSReleaseResource( pCurrentVolume->VolumeLock); } if( RootPathName->Buffer != uniFullPathName.Buffer) @@ -2625,11 +2612,11 @@ AFSParseName( IN PIRP Irp, *FileName = pIrpSp->FileObject->FileName; // - // Grab the root node exclusive before returning + // Grab the root node while checking state // - AFSAcquireExcl( pVolumeCB->VolumeLock, - TRUE); + AFSAcquireShared( pVolumeCB->VolumeLock, + TRUE); if( BooleanFlagOn( pVolumeCB->ObjectInformation.Flags, AFS_OBJECT_FLAGS_OBJECT_INVALID) || BooleanFlagOn( pVolumeCB->Flags, AFS_VOLUME_FLAGS_OFFLINE)) @@ -2679,7 +2666,7 @@ AFSParseName( IN PIRP Irp, } } - AFSConvertToShared( pVolumeCB->VolumeLock); + AFSReleaseResource( pVolumeCB->VolumeLock); if( BooleanFlagOn( pDirEntry->ObjectInformation->Flags, AFS_OBJECT_FLAGS_VERIFY)) { @@ -2711,8 +2698,6 @@ AFSParseName( IN PIRP Irp, pDirEntry->ObjectInformation->FileId.Unique, ntStatus); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus); } } @@ -2740,8 +2725,6 @@ AFSParseName( IN PIRP Irp, "AFSParseName (%08lX) Failed to allocate full name buffer\n", Irp); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus = STATUS_INSUFFICIENT_RESOURCES); } @@ -2807,8 +2790,6 @@ AFSParseName( IN PIRP Irp, AFSExFreePool( uniFullName.Buffer); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus = STATUS_INSUFFICIENT_RESOURCES); } @@ -2836,8 +2817,6 @@ AFSParseName( IN PIRP Irp, AFSExFreePool( uniFullName.Buffer); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus = STATUS_INSUFFICIENT_RESOURCES); } @@ -2856,8 +2835,6 @@ AFSParseName( IN PIRP Irp, AFSExFreePool( uniFullName.Buffer); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus); } @@ -3036,8 +3013,8 @@ AFSParseName( IN PIRP Irp, // Be sure we are online and ready to go // - AFSAcquireExcl( AFSGlobalRoot->VolumeLock, - TRUE); + AFSAcquireShared( AFSGlobalRoot->VolumeLock, + TRUE); if( BooleanFlagOn( AFSGlobalRoot->ObjectInformation.Flags, AFS_OBJECT_FLAGS_OBJECT_INVALID) || BooleanFlagOn( AFSGlobalRoot->Flags, AFS_VOLUME_FLAGS_OFFLINE)) @@ -3087,6 +3064,8 @@ AFSParseName( IN PIRP Irp, } } + AFSReleaseResource( AFSGlobalRoot->VolumeLock); + if( !BooleanFlagOn( AFSGlobalRoot->ObjectInformation.Flags, AFS_OBJECT_FLAGS_DIRECTORY_ENUMERATED)) { @@ -3108,8 +3087,6 @@ AFSParseName( IN PIRP Irp, Irp, ntStatus); - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - try_return( ntStatus); } } @@ -3138,8 +3115,6 @@ AFSParseName( IN PIRP Irp, NULL, AFSGlobalRoot->DirectoryCB->OpenReferenceCount); - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - *VolumeCB = NULL; FileName->Length = 0; @@ -3206,8 +3181,6 @@ AFSParseName( IN PIRP Irp, NULL, AFSGlobalRoot->DirectoryCB->OpenReferenceCount); - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - *VolumeCB = NULL; FileName->Length = 0; @@ -3251,8 +3224,6 @@ AFSParseName( IN PIRP Irp, NULL, AFSGlobalRoot->DirectoryCB->OpenReferenceCount); - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - ClearFlag( *ParseFlags, AFS_PARSE_FLAG_ROOT_ACCESS); *VolumeCB = NULL; @@ -3272,8 +3243,6 @@ AFSParseName( IN PIRP Irp, Irp, &uniComponentName); - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - // // Add in the full share name to pass back // @@ -3382,7 +3351,6 @@ AFSParseName( IN PIRP Irp, if( !NT_SUCCESS( ntStatus)) { - AFSReleaseResource( AFSGlobalRoot->VolumeLock); if ( bIsAllShare && uniRemainingPath.Length == 0 && @@ -3527,12 +3495,6 @@ AFSParseName( IN PIRP Irp, } // - // We only need the volume shared at this point - // - - AFSConvertToShared( pVolumeCB->VolumeLock); - - // // Init our name array // @@ -3547,8 +3509,6 @@ AFSParseName( IN PIRP Irp, "AFSParseName (%08lX) Failed to initialize name array\n", Irp); - AFSReleaseResource( pVolumeCB->VolumeLock); - try_return( ntStatus = STATUS_INSUFFICIENT_RESOURCES); } @@ -3596,7 +3556,6 @@ try_exit: if( *VolumeCB != NULL) { - ASSERT( (*VolumeCB)->VolumeReferenceCount > 1); } @@ -3717,12 +3676,6 @@ AFSCheckCellName( IN GUID *AuthGroup, { // - // We have the global root on entry so drop it now - // - - AFSReleaseResource( AFSGlobalRoot->VolumeLock); - - // // Build the root volume entry // @@ -3732,14 +3685,6 @@ AFSCheckCellName( IN GUID *AuthGroup, if( !NT_SUCCESS( ntStatus)) { - - // - // On failure this routine is expecting to hold the global root - // - - AFSAcquireShared( AFSGlobalRoot->VolumeLock, - TRUE); - try_return( ntStatus); } @@ -4116,8 +4061,6 @@ AFSBuildMountPointTarget( IN GUID *AuthGroup, AFSReleaseResource( &pVolumeCB->RootFcb->NPFcb->Resource); } - AFSConvertToShared( pVolumeCB->VolumeLock); - AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, AFS_TRACE_LEVEL_VERBOSE_2, "AFSBuildMountPointTarget Evaluated target of %wZ FID %08lX-%08lX-%08lX-%08lX as root volume\n", @@ -4129,6 +4072,8 @@ AFSBuildMountPointTarget( IN GUID *AuthGroup, InterlockedIncrement( &pVolumeCB->VolumeReferenceCount); + AFSReleaseResource( pVolumeCB->VolumeLock); + AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING, AFS_TRACE_LEVEL_VERBOSE, "AFSBuildMountPointTarget Increment count on volume %08lX Cnt %d\n", @@ -4273,8 +4218,6 @@ AFSBuildRootVolume( IN GUID *AuthGroup, AFSReleaseResource( &pVolumeCB->RootFcb->NPFcb->Resource); } - AFSConvertToShared( pVolumeCB->VolumeLock); - AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, AFS_TRACE_LEVEL_VERBOSE_2, "AFSBuildRootVolume Evaluated target of %wZ FID %08lX-%08lX-%08lX-%08lX as root volume\n", @@ -4286,6 +4229,8 @@ AFSBuildRootVolume( IN GUID *AuthGroup, InterlockedIncrement( &pVolumeCB->VolumeReferenceCount); + AFSReleaseResource( pVolumeCB->VolumeLock); + AFSDbgLogMsg( AFS_SUBSYSTEM_VOLUME_REF_COUNTING, AFS_TRACE_LEVEL_VERBOSE, "AFSBuildRootVolume Increment count on volume %08lX Cnt %d\n", diff --git a/src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp b/src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp index 16c455e..c481e38 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp @@ -79,8 +79,8 @@ AFSQueryVolumeInfo( IN PDEVICE_OBJECT LibDeviceObject, FsInformationClass = pIrpSp->Parameters.QueryVolume.FsInformationClass; pBuffer = Irp->AssociatedIrp.SystemBuffer; - AFSAcquireExcl( pVolumeCB->VolumeLock, - TRUE); + AFSAcquireShared( pVolumeCB->VolumeLock, + TRUE); bReleaseResource = TRUE; -- 1.9.4