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 <buildbot@rampaginggeek.com>
Tested-by: Jeffrey Altman <jaltman@secure-endpoints.com>
Reviewed-by: Jeffrey Altman <jaltman@secure-endpoints.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
index c74b5688ddb61b22f225b04d5c3bc8ef5dc6635a..e8d73563282111d15fd4d317757e29336ecc70a4 100644 (file)
--- 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 6d79ea855f76910e967b22de5b664c2c41d5df69..cda97c51ba18e36bb5d0f7f696e9828112878559 100644 (file)
--- 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 57c7909926e0b708b88351aefc9d04f4e7f73fac..4abd76e324458d09889bd4f72d784d740e7af83f 100644 (file)
--- 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 f0376646fb830adb67c58b1bc4448054ef09d7aa..b2a5d233ef51492ef14a74421fad0547abee3390 100644 (file)
--- 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 8bb007bcf1bb53ae35fa294890fa34297b381e99..258e671fa1d419ab768b058ecd78d4e7fb8609ad 100644 (file)
--- 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 &&
@@ -3526,12 +3494,6 @@ AFSParseName( IN PIRP Irp,
             *ParentDirectoryCB = pVolumeCB->DirectoryCB;
         }
 
-        //
-        // 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);
         }
 
@@ -3716,12 +3675,6 @@ AFSCheckCellName( IN GUID *AuthGroup,
             pDirEnumEntry->FileId.Volume != AFSGlobalRoot->ObjectInformation.FileId.Volume)
         {
 
-            //
-            // 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 16c455ed2f5e91e724fe5f473dde0470b96e2f43..c481e38403d90b62b17369c914860145bec5caa7 100644 (file)
--- 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;