Windows: Avoid bottleneck on VolumeLock
authorPeter Scott <pscott@kerneldrivers.com>
Sat, 24 Dec 2011 00:00:57 +0000 (17:00 -0700)
committerJeffrey Altman <jaltman@secure-endpoints.com>
Sun, 25 Dec 2011 05:46:58 +0000 (21:46 -0800)
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>

src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp
src/WINNT/afsrdr/kernel/lib/AFSFcbSupport.cpp
src/WINNT/afsrdr/kernel/lib/AFSFileInfo.cpp
src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp
src/WINNT/afsrdr/kernel/lib/AFSNameSupport.cpp
src/WINNT/afsrdr/kernel/lib/AFSVolumeInfo.cpp

index c74b568..e8d7356 100644 (file)
@@ -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);
         }
 
         //
index 6d79ea8..cda97c5 100644 (file)
@@ -803,7 +803,6 @@ AFSRemoveVolume( IN AFSVolumeCB *VolumeCB)
 
             if( ExIsResourceAcquiredLite( VolumeCB->VolumeLock))
             {
-
                 AFSReleaseResource( VolumeCB->VolumeLock);
             }
 
index 57c7909..4abd76e 100644 (file)
@@ -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);
         }
 
         //
index f037664..b2a5d23 100644 (file)
@@ -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);
             }
         }
 
index 8bb007b..258e671 100644 (file)
@@ -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",
index 16c455e..c481e38 100644 (file)
@@ -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;