From 779c27bf1daaf01c79c0bb7c687781151abb4383 Mon Sep 17 00:00:00 2001 From: Jeffrey Altman Date: Mon, 9 Apr 2012 18:41:13 -0400 Subject: [PATCH] Windows: Refactor AFSValidateEntry Refactor AFSValidateEntry to avoid obtaining the ObjectInformation->Fcb->Resource when it isn't necessary. This will avoid contention and improve performance. The only time that the Fcb->Resource is required is when the object requires verification, the object is a FILE, and the object was successfully evaluated. Even with this reorganization there is a small window of opportunity for a deadlock to occur if a CcPurgeCacheSection() which is called with the Fcb->Resource held triggers a filter driver to issue a CreateFile and in between the two operations an invalidate object is received. Change-Id: Iddf64f030c6a608ac29a10b7a9a7e130ae6c4249 Reviewed-on: http://gerrit.openafs.org/7143 Reviewed-by: Derrick Brashear Tested-by: Derrick Brashear Tested-by: Jeffrey Altman Reviewed-by: Jeffrey Altman --- src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp | 2 - src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp | 171 ++++++++++----------- 2 files changed, 80 insertions(+), 93 deletions(-) diff --git a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp index 586762551..6e0e94c5f 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSCreate.cpp @@ -3247,8 +3247,6 @@ AFSProcessOverwriteSupersede( IN PDEVICE_OBJECT DeviceObject, KeQuerySystemTime( &pObjectInfo->LastAccessTime); - //KeQuerySystemTime( &pObjectInfo->CreationTime); - KeQuerySystemTime( &pObjectInfo->LastWriteTime); ntStatus = AFSUpdateFileInformation( &pParentObjectInfo->FileId, diff --git a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp index add53f144..8c3ea261a 100644 --- a/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp +++ b/src/WINNT/afsrdr/kernel/lib/AFSGeneric.cpp @@ -3756,28 +3756,6 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, try_return( ntStatus); } - if( PurgeContent && - pObjectInfo->Fcb != NULL) - { - - pCurrentFcb = pObjectInfo->Fcb; - - if( !ExIsResourceAcquiredLite( &pCurrentFcb->NPFcb->Resource)) - { - - AFSDbgLogMsg( AFS_SUBSYSTEM_LOCK_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSValidateEntry Acquiring Fcb lock %08lX EXCL %08lX\n", - &pCurrentFcb->NPFcb->Resource, - PsGetCurrentThread()); - - AFSAcquireExcl( &pCurrentFcb->NPFcb->Resource, - TRUE); - - bReleaseFcb = TRUE; - } - } - // // This routine ensures that the current entry is valid by: // @@ -3905,109 +3883,120 @@ AFSValidateEntry( IN AFSDirectoryCB *DirEntry, // Can't hold the Fcb resource while doing this // - if( pCurrentFcb != NULL && + if( PurgeContent && + pObjectInfo->Fcb != NULL && (pObjectInfo->DataVersion.QuadPart != pDirEnumEntry->DataVersion.QuadPart || BooleanFlagOn( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA))) { - IO_STATUS_BLOCK stIoStatus; - BOOLEAN bPurgeExtents = FALSE; + pCurrentFcb = pObjectInfo->Fcb; - AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE_2, - "AFSValidateEntry Flush/purge entry %wZ FID %08lX-%08lX-%08lX-%08lX\n", - &DirEntry->NameInformation.FileName, - pObjectInfo->FileId.Cell, - pObjectInfo->FileId.Volume, - pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique); + if( !ExIsResourceAcquiredLite( &pCurrentFcb->NPFcb->Resource)) + { + + AFSDbgLogMsg( AFS_SUBSYSTEM_LOCK_PROCESSING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSValidateEntry Acquiring Fcb lock %08lX EXCL %08lX\n", + &pCurrentFcb->NPFcb->Resource, + PsGetCurrentThread()); + + AFSAcquireExcl( &pCurrentFcb->NPFcb->Resource, + TRUE); - if ( BooleanFlagOn( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA)) + bReleaseFcb = TRUE; + } + + if( pCurrentFcb != NULL) { - bPurgeExtents = TRUE; + + IO_STATUS_BLOCK stIoStatus; + BOOLEAN bPurgeExtents = FALSE; AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSVerifyEntry Clearing VERIFY_DATA flag %wZ FID %08lX-%08lX-%08lX-%08lX\n", + AFS_TRACE_LEVEL_VERBOSE_2, + "AFSValidateEntry Flush/purge entry %wZ FID %08lX-%08lX-%08lX-%08lX\n", &DirEntry->NameInformation.FileName, pObjectInfo->FileId.Cell, pObjectInfo->FileId.Volume, pObjectInfo->FileId.Vnode, pObjectInfo->FileId.Unique); - ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA); - } + if ( BooleanFlagOn( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA)) + { + bPurgeExtents = TRUE; - __try - { + AFSDbgLogMsg( AFS_SUBSYSTEM_FILE_PROCESSING, + AFS_TRACE_LEVEL_VERBOSE, + "AFSVerifyEntry Clearing VERIFY_DATA flag %wZ FID %08lX-%08lX-%08lX-%08lX\n", + &DirEntry->NameInformation.FileName, + pObjectInfo->FileId.Cell, + pObjectInfo->FileId.Volume, + pObjectInfo->FileId.Vnode, + pObjectInfo->FileId.Unique); - CcFlushCache( &pCurrentFcb->NPFcb->SectionObjectPointers, - NULL, - 0, - &stIoStatus); + ClearFlag( pObjectInfo->Flags, AFS_OBJECT_FLAGS_VERIFY_DATA); + } - if( !NT_SUCCESS( stIoStatus.Status)) + __try { + CcFlushCache( &pCurrentFcb->NPFcb->SectionObjectPointers, + NULL, + 0, + &stIoStatus); + + if( !NT_SUCCESS( stIoStatus.Status)) + { + + AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, + AFS_TRACE_LEVEL_ERROR, + "AFSValidateEntry CcFlushCache failure %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX Bytes 0x%08lX\n", + &DirEntry->NameInformation.FileName, + pObjectInfo->FileId.Cell, + pObjectInfo->FileId.Volume, + pObjectInfo->FileId.Vnode, + pObjectInfo->FileId.Unique, + stIoStatus.Status, + stIoStatus.Information); + + ntStatus = stIoStatus.Status; + } + + if ( bPurgeExtents) + { + + CcPurgeCacheSection( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers, + NULL, + 0, + FALSE); + } + } + __except( EXCEPTION_EXECUTE_HANDLER) + { + ntStatus = GetExceptionCode(); + AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, AFS_TRACE_LEVEL_ERROR, - "AFSValidateEntry CcFlushCache failure %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX Bytes 0x%08lX\n", + "AFSValidateEntry CcFlushCache or CcPurgeCacheSection exception %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", &DirEntry->NameInformation.FileName, pObjectInfo->FileId.Cell, pObjectInfo->FileId.Volume, pObjectInfo->FileId.Vnode, pObjectInfo->FileId.Unique, - stIoStatus.Status, - stIoStatus.Information); + ntStatus); - ntStatus = stIoStatus.Status; } + AFSReleaseResource( &pCurrentFcb->NPFcb->Resource); + + bReleaseFcb = FALSE; + if ( bPurgeExtents) { - - CcPurgeCacheSection( &pObjectInfo->Fcb->NPFcb->SectionObjectPointers, - NULL, - 0, - FALSE); + AFSFlushExtents( pCurrentFcb, + AuthGroup); } } - __except( EXCEPTION_EXECUTE_HANDLER) - { - ntStatus = GetExceptionCode(); - - AFSDbgLogMsg( AFS_SUBSYSTEM_IO_PROCESSING, - AFS_TRACE_LEVEL_ERROR, - "AFSValidateEntry CcFlushCache or CcPurgeCacheSection exception %wZ FID %08lX-%08lX-%08lX-%08lX Status 0x%08lX\n", - &DirEntry->NameInformation.FileName, - pObjectInfo->FileId.Cell, - pObjectInfo->FileId.Volume, - pObjectInfo->FileId.Vnode, - pObjectInfo->FileId.Unique, - ntStatus); - - } - - AFSReleaseResource( &pCurrentFcb->NPFcb->Resource); - - if ( bPurgeExtents) - { - AFSFlushExtents( pCurrentFcb, - AuthGroup); - } - - // - // Reacquire the Fcb to purge the cache - // - - AFSDbgLogMsg( AFS_SUBSYSTEM_LOCK_PROCESSING, - AFS_TRACE_LEVEL_VERBOSE, - "AFSValidateEntry Acquiring Fcb lock %08lX EXCL %08lX\n", - &pCurrentFcb->NPFcb->Resource, - PsGetCurrentThread()); - - AFSAcquireExcl( &pCurrentFcb->NPFcb->Resource, - TRUE); } // -- 2.39.5