Windows: hold locks during non-cached writes

Writes can alter both the EndOfFile (Fcb FileSize) and the ValidDataLength
which must remain synchronized with the data known to the service.
Dropping the Fcb.Resource and the SectionObjectResource prior to
performing non-cached writes opens the possibility of a race in which
data changes and length updates can be altered independently.

Efforts are made to avoid holding locks across calls to the service
because they can result in deadlocks with object invalidation or extent
management.  However, object invalidation for data version changes are
now handled in a worker thread.  It should be safe to hold onto the
Fcb Resource and SectionObjectResource across non-cached write processing.

The locks are not held in the paging IO path so paging non-cached
writes (which cannot be extending) will not prevent cached writes from
taking place in parallel.

The reason it is critical for the ValidDataLength and the FileSize to
remain in sync with the data for non-paging non-cached writes is that
these values are used to determine whether the Windows cache manager
should trigger a page fault to read data from the service upon receiving
an extending cached write that doesn't fill the page.

Change-Id: If3edb2a7412623dbec10a6efd2ee8d3b92ac992d
Reviewed-on: http://gerrit.openafs.org/9745
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Rod Widdowson <rdw@steadingsoftware.com>
Reviewed-by: Peter Scott <pscott@kerneldrivers.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>

diff --git a/src/WINNT/afsrdr/kernel/lib/AFSWrite.cpp b/src/WINNT/afsrdr/kernel/lib/AFSWrite.cpp
index 97b55a1..d58e534 100644 (file)
--- a/src/WINNT/afsrdr/kernel/lib/AFSWrite.cpp
+++ b/src/WINNT/afsrdr/kernel/lib/AFSWrite.cpp
@@ -669,35 +669,6 @@ AFSCommonWrite( IN PDEVICE_OBJECT DeviceObject,
         else
         {
 
-            //
-            // if bPagingIo, Paging Resource held Shared
-            // else Main and SectionObject resources held Shared
-            //
-
-            if( bReleaseSectionObject)
-            {
-
-                AFSReleaseResource( &pNPFcb->SectionObjectResource);
-
-                bReleaseSectionObject = FALSE;
-            }
-
-            if( bReleasePaging)
-            {
-
-                AFSReleaseResource( &pNPFcb->PagingResource);
-
-                bReleasePaging = FALSE;
-            }
-
-            if( bReleaseMain)
-            {
-
-                AFSReleaseResource( &pNPFcb->Resource);
-
-                bReleaseMain = FALSE;
-            }
-
             AFSDbgTrace(( AFS_SUBSYSTEM_IO_PROCESSING,
                           AFS_TRACE_LEVEL_VERBOSE,
                           "AFSCommonWrite (%p) Processing NON-CACHED request Offset %0I64X Len %08lX%s\n",