[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,