[ros-diffs] [arty] 35972: Fix some of my late-night brain damage regarding starting caching. Factor out some common pinning and cache starting code. Use IoPageRead. We run the install to 100%, then fail. We leak pretty bad in here... I think pages aren't being freed when we unmap our cache sections.

Author: arty
Date: Fri Sep  5 21:20:54 2008
New Revision: 35972
URL: http://svn.reactos.org/svn/reactos?rev=35972&view=rev
Log:
Fix some of my late-night brain damage regarding starting caching.
Factor out some common pinning and cache starting code.
Use IoPageRead.
We run the install to 100%, then fail.
We leak pretty bad in here...  I think pages aren't being freed when we
unmap our cache sections.
Modified:
    branches/arty-newcc/ntoskrnl/cache/cachesub.c
    branches/arty-newcc/ntoskrnl/cache/fssup.c
    branches/arty-newcc/ntoskrnl/cache/lazyrite.c
    branches/arty-newcc/ntoskrnl/cache/pinsup.c
    branches/arty-newcc/ntoskrnl/mm/section.c
Modified: branches/arty-newcc/ntoskrnl/cache/cachesub.c
URL:
http://svn.reactos.org/svn/reactos/branches/arty-newcc/ntoskrnl/cache/cache…
==============================================================================
--- branches/arty-newcc/ntoskrnl/cache/cachesub.c [iso-8859-1] (original)
+++ branches/arty-newcc/ntoskrnl/cache/cachesub.c [iso-8859-1] Fri Sep  5 21:20:54 2008
@@ -167,12 +167,13 @@
        PNOCC_BCB Bcb = (PNOCC_BCB)BcbVoid;
        Bcb->Dirty = TRUE;
        for (Buffer = Bcb->BaseAddress;
-                Buffer < ((PCHAR)Bcb->BaseAddress) + Bcb->Length;
-                Buffer += PAGE_SIZE)
-       {
-               /* Do a fake write on the buffer pages to mark them for mm */
-               *Buffer ^= 0;
-       }
+            Buffer < ((PCHAR)Bcb->BaseAddress) + Bcb->Length;
+            Buffer += PAGE_SIZE)
+       {
+           /* Do a fake write on the buffer pages to mark them for mm */
+           *Buffer ^= 0;
+       }
+       Bcb->Dirty = TRUE;
 }
 LARGE_INTEGER
Modified: branches/arty-newcc/ntoskrnl/cache/fssup.c
URL:
http://svn.reactos.org/svn/reactos/branches/arty-newcc/ntoskrnl/cache/fssup…
==============================================================================
--- branches/arty-newcc/ntoskrnl/cache/fssup.c [iso-8859-1] (original)
+++ branches/arty-newcc/ntoskrnl/cache/fssup.c [iso-8859-1] Fri Sep  5 21:20:54 2008
@@ -101,7 +101,6 @@
                      IN PCACHE_MANAGER_CALLBACKS Callbacks,
                      IN PVOID LazyWriteContext)
 {
-    DPRINT("Initializing file object for %wZ\n", &FileObject->FileName);
     CcpLock();
     if (FileObject->SectionObjectPointer->SharedCacheMap)
     {
@@ -111,6 +110,8 @@
     else
     {
        PNOCC_CACHE_MAP Map = ExAllocatePool(NonPagedPool, sizeof(NOCC_CACHE_MAP));
+       DPRINT("Initializing file object for (%p) %wZ\n", FileObject,
&FileObject->FileName);
+       ASSERT(FileObject);
        FileObject->SectionObjectPointer->SharedCacheMap = Map;
        Map->RefCount = 1;
        ObReferenceObject(FileObject);
Modified: branches/arty-newcc/ntoskrnl/cache/lazyrite.c
URL:
http://svn.reactos.org/svn/reactos/branches/arty-newcc/ntoskrnl/cache/lazyr…
==============================================================================
--- branches/arty-newcc/ntoskrnl/cache/lazyrite.c [iso-8859-1] (original)
+++ branches/arty-newcc/ntoskrnl/cache/lazyrite.c [iso-8859-1] Fri Sep  5 21:20:54 2008
@@ -9,7 +9,7 @@
 /* INCLUDES *******************************************************************/
 #include <ntoskrnl.h>
-//#define NDEBUG
+#define NDEBUG
 #include <debug.h>
 /* GLOBALS ********************************************************************/
Modified: branches/arty-newcc/ntoskrnl/cache/pinsup.c
URL:
http://svn.reactos.org/svn/reactos/branches/arty-newcc/ntoskrnl/cache/pinsu…
==============================================================================
--- branches/arty-newcc/ntoskrnl/cache/pinsup.c [iso-8859-1] (original)
+++ branches/arty-newcc/ntoskrnl/cache/pinsup.c [iso-8859-1] Fri Sep  5 21:20:54 2008
@@ -383,6 +383,64 @@
 BOOLEAN
 NTAPI
+CcpStartCaching(PFILE_OBJECT FileObject)
+{
+    FILE_STANDARD_INFORMATION FileInfo;
+    ULONG Information;
+    NTSTATUS Status;
+    CC_FILE_SIZES FileSizes;
+
+    if (!FileObject->SectionObjectPointer->SharedCacheMap)
+    {
+       DPRINT("CcpStartCaching %p\n", FileObject);
+       Status = IoQueryFileInformation
+           (FileObject,
+            FileStandardInformation,
+            sizeof(FILE_STANDARD_INFORMATION),
+            &FileInfo,
+            &Information);
+
+       if (!NT_SUCCESS(Status))
+       {
+           return FALSE;
+       }
+
+       Status = IoQueryFileInformation
+           (FileObject,
+            FileAllocationInformation,
+            sizeof(LARGE_INTEGER),
+            &FileSizes.AllocationSize,
+            &Information);
+
+       if (!NT_SUCCESS(Status))
+       {
+           return FALSE;
+       }
+
+       FileSizes.ValidDataLength = FileInfo.EndOfFile;
+       FileSizes.FileSize = FileInfo.EndOfFile;
+
+       DPRINT("Initializing -> File Sizes: VALID %08x%08x FILESIZE %08x%08x ALLOC
%08x%08x\n",
+              FileSizes.ValidDataLength.HighPart,
+              FileSizes.ValidDataLength.LowPart,
+              FileSizes.FileSize.HighPart,
+              FileSizes.FileSize.LowPart,
+              FileSizes.AllocationSize.HighPart,
+              FileSizes.AllocationSize.LowPart);
+
+       CcInitializeCacheMap
+           (FileObject,
+            &FileSizes,
+            FALSE,
+            NULL,
+            NULL);
+    }
+
+    return TRUE;
+}
+
+BOOLEAN
+NTAPI
 CcpMapData
 (IN PNOCC_CACHE_MAP Map,
  IN PLARGE_INTEGER FileOffset,
@@ -418,7 +476,6 @@
        /* Find an accomodating section */
        //DPRINT("Selected BCB #%x\n", BcbHead);
        Bcb = &CcCacheSections[BcbHead];
-       SectionObject = Bcb->SectionObject;
        BcbHead = CcpFindMatchingMap(Bcb, FileOffset, Length);
        if (BcbHead == INVALID_CACHE)
@@ -433,6 +490,7 @@
        else
        {
            Bcb = &CcCacheSections[BcbHead];
+           SectionObject = Bcb->SectionObject;
            Success = TRUE;
            *BcbResult = Bcb;
            *Buffer = ((PCHAR)Bcb->BaseAddress) + (int)(FileOffset->QuadPart -
Bcb->FileOffset.QuadPart);
@@ -457,7 +515,7 @@
        PNOCC_CACHE_MAP Map =
(PNOCC_CACHE_MAP)FileObject->SectionObjectPointer->SharedCacheMap;
        ULONG SectionSize;
-       DPRINT("%08x: File size %x%x\n", FileObject->SectionObjectPointer,
Map->FileSizes.ValidDataLength.HighPart, Map->FileSizes.ValidDataLength.LowPart);
+       DPRINT("%08x: File size %08x%08x\n",
FileObject->SectionObjectPointer, Map->FileSizes.ValidDataLength.HighPart,
Map->FileSizes.ValidDataLength.LowPart);
        if (Map->FileSizes.ValidDataLength.QuadPart)
        {
@@ -508,7 +566,7 @@
        goto cleanup;
     }
-    //DPRINT("Selected BCB #%x\n", BcbHead);
+    DPRINT("Selected BCB #%x\n", BcbHead);
     Bcb = &CcCacheSections[BcbHead];
     Status = CcpMapSegment(BcbHead);
@@ -545,14 +603,7 @@
  OUT PVOID *BcbResult,
  OUT PVOID *Buffer)
 {
-    if (!(PNOCC_CACHE_MAP)FileObject->SectionObjectPointer->SharedCacheMap)
-    {
-       PNOCC_CACHE_MAP Map = ExAllocatePool(NonPagedPool, sizeof(NOCC_CACHE_MAP));
-       FileObject->SectionObjectPointer->SharedCacheMap = Map;
-       Map->FileObject = FileObject;
-       InitializeListHead(&Map->AssociatedBcb);
-    }
-
+    if (!CcpStartCaching(FileObject)) return FALSE;
     return CcpMapData
        ((PNOCC_CACHE_MAP)FileObject->SectionObjectPointer->SharedCacheMap,
         FileOffset,
@@ -570,18 +621,19 @@
                 IN ULONG Flags,
                 IN OUT PVOID *Bcb)
 {
-    BOOLEAN Wait = Flags & PIN_WAIT;
+    BOOLEAN Wait = Flags == TRUE ? PIN_WAIT : Flags & PIN_WAIT;
     BOOLEAN Exclusive = Flags & PIN_EXCLUSIVE;
     BOOLEAN Result;
     ULONG BcbHead;
     PNOCC_BCB TheBcb;
     PVOID Buffer;
+    if (!CcpStartCaching(FileObject)) return FALSE;
     Result = CcMapData(FileObject, FileOffset, Length, Wait ? MAP_WAIT : 0, Bcb,
&Buffer);
     if (!Result) return FALSE;
-    TheBcb = (PNOCC_BCB)Bcb;
+    TheBcb = (PNOCC_BCB)*Bcb;
     BcbHead = TheBcb - CcCacheSections;
     CcpLock();
@@ -591,7 +643,10 @@
        CcpMarkForExclusive(BcbHead);
     }
     else
+    {
+       DPRINT("Reference #%x\n", BcbHead);
        CcpReferenceCache(BcbHead);
+    }
     CcpUnlock();
     if (Exclusive)
@@ -630,31 +685,25 @@
           OUT PVOID *Bcb,
           OUT PVOID *Buffer)
 {
-    BOOLEAN Result = CcMapData(FileObject, FileOffset, Length, Flags, Bcb, Buffer);
+    PNOCC_BCB RealBcb;
+    BOOLEAN Result;
+
+    if (!CcpStartCaching(FileObject)) return FALSE;
+
+    Result = CcPinMappedData
+       (FileObject,
+        FileOffset,
+        Length,
+        Flags,
+        Bcb);
     if (Result)
     {
-       PNOCC_BCB TheBcb = (PNOCC_BCB)*Bcb;
-       if (!TheBcb->Pinned)
-       {
-           TheBcb->Pinned = IoAllocateMdl
-               (TheBcb->BaseAddress,
-                TheBcb->Length,
-                FALSE,
-                FALSE,
-                NULL);
-           _SEH_TRY
-           {
-               MmProbeAndLockPages(TheBcb->Pinned, KernelMode, IoReadAccess);
-           }
-           _SEH_HANDLE
-           {
-               IoFreeMdl(TheBcb->Pinned);
-               TheBcb->Pinned = NULL;
-               Result = FALSE;
-           }
-           _SEH_END;
-       }
+       CcpLock();
+       /* Find out if any range is a superset of what we want */
+       RealBcb = *Bcb;
+       *Buffer = ((PCHAR)RealBcb->BaseAddress) + (int)(FileOffset->QuadPart -
RealBcb->FileOffset.QuadPart);
+       CcpUnlock();
     }
     return Result;
@@ -670,32 +719,25 @@
                   OUT PVOID *Bcb,
                   OUT PVOID *Buffer)
 {
-    BOOLEAN Result = CcMapData(FileObject, FileOffset, Length, Flags, Bcb, Buffer);
+    BOOLEAN Result;
+    PNOCC_BCB RealBcb;
+
+    ASSERT(!Zero);
+
+    Result = CcPinRead
+       (FileObject,
+        FileOffset,
+        Length,
+        Flags,
+        Bcb,
+        Buffer);
     if (Result)
     {
-       PNOCC_BCB TheBcb = (PNOCC_BCB)*Bcb;
-       if (!TheBcb->Pinned)
-       {
-           TheBcb->Pinned = IoAllocateMdl
-               (TheBcb->BaseAddress,
-                TheBcb->Length,
-                FALSE,
-                FALSE,
-                NULL);
-           _SEH_TRY
-           {
-               MmProbeAndLockPages(TheBcb->Pinned, KernelMode, IoReadAccess);
-           }
-           _SEH_HANDLE
-           {
-               IoFreeMdl(TheBcb->Pinned);
-               TheBcb->Pinned = NULL;
-               TheBcb->Dirty = TRUE;
-               Result = FALSE;
-           }
-           _SEH_END;
-       }
+       CcpLock();
+       RealBcb = *Bcb;
+       RealBcb->Dirty = TRUE;
+       CcpUnlock();
     }
     return Result;
@@ -707,7 +749,9 @@
 {
     PNOCC_BCB RealBcb = (PNOCC_BCB)Bcb;
     ULONG Selected = RealBcb - CcCacheSections;
+
     DPRINT("CcUnpinData Bcb #%x (RefCount %d)\n", Selected,
RealBcb->RefCount);
+
     CcpLock();
     if (RealBcb->RefCount <= 2)
     {
Modified: branches/arty-newcc/ntoskrnl/mm/section.c
URL:
http://svn.reactos.org/svn/reactos/branches/arty-newcc/ntoskrnl/mm/section.…
==============================================================================
--- branches/arty-newcc/ntoskrnl/mm/section.c [iso-8859-1] (original)
+++ branches/arty-newcc/ntoskrnl/mm/section.c [iso-8859-1] Fri Sep  5 21:20:54 2008
@@ -147,45 +147,36 @@
  ULONG Length,
  PIO_STATUS_BLOCK ReadStatus)
 {
-    NTSTATUS Status;
-    PIRP Irp = NULL;
+    PMDL Mdl;
     KEVENT ReadWait;
-    PDEVICE_OBJECT DeviceObject = MmGetDeviceObjectForFile(FileObject);
-    PIO_STACK_LOCATION IrpSp;
-
-    DPRINT1
-       ("PAGING READ File %wZ Offset %x Length %d\n",
-        &FileObject->FileName,
-        FileOffset->u.LowPart,
-        Length);
-
-    KeInitializeEvent(&ReadWait, NotificationEvent, FALSE);
-
-    Irp = IoBuildAsynchronousFsdRequest
-       (IRP_MJ_READ,
-        DeviceObject,
-        Buffer,
-        Length,
-        FileOffset,
-        ReadStatus);
-
-    if (!Irp)
+    NTSTATUS Status = STATUS_SUCCESS;
+
+    KeInitializeEvent(&ReadWait, SynchronizationEvent, FALSE);
+
+    Mdl = IoAllocateMdl(Buffer, Length, FALSE, TRUE, NULL);
+
+    if (!Mdl) return STATUS_NO_MEMORY;
+
+    _SEH_TRY
     {
-       return STATUS_NO_MEMORY;
+       /* Allocate an MDL */
+       Mdl = IoAllocateMdl(Buffer, Length, FALSE, TRUE, NULL);
+       MmProbeAndLockPages(Mdl, KernelMode, IoWriteAccess);
+    }
+    _SEH_HANDLE
+    {
+       /* Allocating failed, clean up */
+       Status = _SEH_GetExceptionCode();
+    }
+    _SEH_END;
+
+    if (!NT_SUCCESS(Status))
+    {
+       IoFreeMdl(Mdl);
+       return Status;
     }
-    Irp->Flags |= IRP_PAGING_IO | IRP_SYNCHRONOUS_PAGING_IO | IRP_NOCACHE;
-
-    ObReferenceObject(FileObject);
-
-    Irp->UserEvent = &ReadWait;
-    Irp->Tail.Overlay.OriginalFileObject = FileObject;
-    Irp->Tail.Overlay.Thread = PsGetCurrentThread();
-    IrpSp = IoGetNextIrpStackLocation(Irp);
-    IrpSp->FileObject = FileObject;
-    IrpSp->CompletionRoutine = MiSimpleReadComplete;
-
-    Status = IoCallDriver(DeviceObject, Irp);
+    Status = IoPageRead(FileObject, Mdl, FileOffset, &ReadWait, ReadStatus);
     if (Status == STATUS_PENDING)
     {
        if (!NT_SUCCESS
@@ -197,14 +188,16 @@
              NULL)))
        {
            DPRINT1("Warning: Failed to wait for synchronous IRP\n");
-           ASSERT(FALSE);
-           ObDereferenceObject(FileObject);
+           MmUnlockPages(Mdl);
+           IoFreeMdl(Mdl);
            return Status;
        }
     }
-
-    ObDereferenceObject(FileObject);
-
+
+    DPRINT("MmUnlockPages(%p)\n", Mdl);
+    MmUnlockPages(Mdl);
+    IoFreeMdl(Mdl);
+
     DPRINT("Paging IO Done: %08x [%02x %02x %02x %02x ...]\n",
ReadStatus->Status,
           ((PCHAR)Buffer)[0] & 0xff,
           ((PCHAR)Buffer)[1] & 0xff,