make mdl usage/impl. more windows compliant

svn path=/trunk/; revision=9189

reactos/drivers/fs/vfat/blockdev.c

@@ -32,6 +32,8 @@ VfatReadWriteCompletion (IN PDEVICE_OBJECT DeviceObject,
    while ((Mdl = Irp->MdlAddress))
      {
        Irp->MdlAddress = Mdl->Next;
+       
+       MmUnlockPages(Mdl);
        IoFreeMdl(Mdl);
      }
 

reactos/include/ddk/mmfuncs.h

@@ -1,6 +1,6 @@
 #ifndef _INCLUDE_DDK_MMFUNCS_H
 #define _INCLUDE_DDK_MMFUNCS_H
-/* $Id: mmfuncs.h,v 1.20 2004/02/10 16:22:56 navaraf Exp $ */
+/* $Id: mmfuncs.h,v 1.21 2004/04/20 19:04:11 gdalsnes Exp $ */
 /* MEMORY MANAGMENT ******************************************************/
 
 
@@ -51,6 +51,8 @@ extern inline unsigned int ADDRESS_AND_SIZE_TO_SPAN_PAGES(PVOID Va,
  * FUNCTION: Returns the byte offset of the address within its page
  */
 #define BYTE_OFFSET(va) (((ULONG)va)%PAGE_SIZE)
+#define PAGE_OFFSET(va) BYTE_OFFSET(va)
+
 
 /*
  * FUNCTION: Takes a count in bytes and returns the number of pages
@@ -58,6 +60,11 @@ extern inline unsigned int ADDRESS_AND_SIZE_TO_SPAN_PAGES(PVOID Va,
  */
 #define BYTES_TO_PAGES(Size) \
   ((ULONG) ((ULONG_PTR) (Size) >> PAGE_SHIFT) + (((ULONG) (Size) & (PAGE_SIZE - 1)) != 0))
+  
+  
+#define PAGE_ALIGN(va) ( (PVOID) (((ULONG)(va)) & (~(PAGE_SIZE-1))) )
+#define PAGE_BASE(va) PAGE_ALIGN(va)
+
 
 DWORD
 STDCALL
@@ -271,10 +278,9 @@ MmGrowKernelStack (
 	(MemoryDescriptorList)->Size = (CSHORT)(sizeof(MDL) + \
 		(ADDRESS_AND_SIZE_TO_SPAN_PAGES((BaseVa),(Length)) * sizeof(ULONG))); \
 	(MemoryDescriptorList)->MdlFlags = 0; \
-	(MemoryDescriptorList)->StartVa = (PVOID)PAGE_ROUND_DOWN((BaseVa)); \
+	(MemoryDescriptorList)->StartVa = (PVOID)PAGE_BASE(BaseVa); \
-	(MemoryDescriptorList)->ByteOffset = (ULONG)((BaseVa) - PAGE_ROUND_DOWN((BaseVa))); \
+	(MemoryDescriptorList)->ByteOffset = (ULONG)PAGE_OFFSET(BaseVa); \
 	(MemoryDescriptorList)->ByteCount = (Length); \
-	(MemoryDescriptorList)->Process = PsGetCurrentProcess(); \
 }
 
 /*

reactos/ntoskrnl/io/cleanup.c

@@ -119,27 +119,20 @@ VOID IoReadWriteCompletion(PDEVICE_OBJECT DeviceObject,
    FileObject = IoStack->FileObject;
    
    if (DeviceObject->Flags & DO_BUFFERED_IO)
-     {
+   {
-	if (IoStack->MajorFunction == IRP_MJ_READ)
+      if (IoStack->MajorFunction == IRP_MJ_READ)
-	  {
+      {
-	     DPRINT("Copying buffered io back to user\n");
+         DPRINT("Copying buffered io back to user\n");
-	     memcpy(Irp->UserBuffer,Irp->AssociatedIrp.SystemBuffer,
+         memcpy(Irp->UserBuffer,Irp->AssociatedIrp.SystemBuffer,
 		    IoStack->Parameters.Read.Length);
-	  }
+      }
-	ExFreePool(Irp->AssociatedIrp.SystemBuffer);
+      ExFreePool(Irp->AssociatedIrp.SystemBuffer);
-     }
+   }
+
    if (DeviceObject->Flags & DO_DIRECT_IO)
-     {
+   {
-	/* FIXME: Is the MDL destroyed on a paging i/o, check all cases. */
+      IoFreeMdl(Irp->MdlAddress);
-	DPRINT("Tearing down MDL\n");
+   }
-	if (Irp->MdlAddress->MappedSystemVa != NULL)
-	  {	     
-	     MmUnmapLockedPages(Irp->MdlAddress->MappedSystemVa,
-				Irp->MdlAddress);
-	  }
-	MmUnlockPages(Irp->MdlAddress);
-	ExFreePool(Irp->MdlAddress);
-     }
 }
 
 VOID IoVolumeInformationCompletion(PDEVICE_OBJECT DeviceObject,
@@ -201,6 +194,13 @@ IoSecondStageCompletion(
    Irp = (PIRP)(*SystemArgument1);
    PriorityBoost = (CCHAR)(LONG)(*SystemArgument2);
    
+   /*
+    * Note that we'll never see irp's flagged IRP_PAGING_IO (IRP_MOUNT_OPERATION)
+    * or IRP_CLOSE_OPERATION (IRP_MJ_CLOSE and IRP_MJ_CLEANUP) here since their
+    * cleanup/completion is fully taken care of in IoCompleteRequest.
+    * -Gunnar
+    */
+    
    /* 
    Remove synchronous irp's from the threads cleanup list.
    To synchronize with the code inserting the entry, this code must run 
@@ -278,7 +278,7 @@ IoSecondStageCompletion(
    }
 
    //Windows NT File System Internals, page 154
-   if (!(Irp->Flags & IRP_PAGING_IO) && OriginalFileObject)
+   if (OriginalFileObject)   
    {
       // if the event is not the one in the file object, it needs dereferenced
       if (Irp->UserEvent && Irp->UserEvent != &OriginalFileObject->Event)
@@ -286,10 +286,7 @@ IoSecondStageCompletion(
          ObDereferenceObject(Irp->UserEvent);
       }
   
-      if (IoStack->MajorFunction != IRP_MJ_CLOSE)
+      ObDereferenceObject(OriginalFileObject);
-      {
-         ObDereferenceObject(OriginalFileObject);
-      }
    }
 
    if (Irp->Overlay.AsynchronousParameters.UserApcRoutine != NULL)

reactos/ntoskrnl/io/irp.c

@@ -1,4 +1,4 @@
-/* $Id: irp.c,v 1.58 2004/03/04 00:07:00 navaraf Exp $
+/* $Id: irp.c,v 1.59 2004/04/20 19:01:47 gdalsnes Exp $
  *
  * COPYRIGHT:       See COPYING in the top level directory
  * PROJECT:         ReactOS kernel
@@ -231,6 +231,7 @@ IofCompleteRequest(PIRP Irp,
    PDEVICE_OBJECT    DeviceObject;
    PFILE_OBJECT      OriginalFileObject;
    KIRQL             oldIrql;
+   PMDL              Mdl;
 
    DPRINT("IoCompleteRequest(Irp %x, PriorityBoost %d) Event %x THread %x\n",
       Irp,PriorityBoost, Irp->UserEvent, PsGetCurrentThread());
@@ -285,42 +286,83 @@ IofCompleteRequest(PIRP Irp,
       }
    }
 
-   if (Irp->Flags & (IRP_PAGING_IO|IRP_CLOSE_OPERATION))
+   /*
-     {
+    * Were done calling completion routines. Now do any cleanup that can be 
-       if (Irp->Flags & IRP_PAGING_IO)
+    * done in an arbitrarily context.
-         {
+    */
-           /* FIXME:
-            *   The mdl must be freed by the caller! 
-	    */
-           if (Irp->MdlAddress->MappedSystemVa != NULL)
-             {	     
-	       MmUnmapLockedPages(Irp->MdlAddress->MappedSystemVa,
-			          Irp->MdlAddress);
-	     }
-           MmUnlockPages(Irp->MdlAddress);
-           ExFreePool(Irp->MdlAddress);
-	 }
-       if (Irp->UserIosb)
-         {
-           *Irp->UserIosb = Irp->IoStatus;
-	 }
-       if (Irp->UserEvent)
-         {
-           KeSetEvent(Irp->UserEvent, PriorityBoost, FALSE);
-	 }
-       IoFreeIrp(Irp);
-     }
-   else
-     {
-       //Windows NT File System Internals, page 154
-       OriginalFileObject = Irp->Tail.Overlay.OriginalFileObject;
 
-       if (Irp->PendingReturned || KeGetCurrentIrql() == DISPATCH_LEVEL)
+   /* Windows NT File System Internals, page 165 */
+   if (Irp->Flags & (IRP_PAGING_IO|IRP_CLOSE_OPERATION))
+   {
+      if (Irp->Flags & IRP_PAGING_IO)
+      {
+         /* FIXME:
+          *   The mdl should be freed by the caller! 
+	       */
+         if (Irp->MdlAddress->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA)
          {
-           BOOLEAN bStatus;
+            MmUnmapLockedPages(Irp->MdlAddress->MappedSystemVa, Irp->MdlAddress);            
+         }
+         
+         ExFreePool(Irp->MdlAddress);
+      }
+
+      if (Irp->UserIosb)
+      {
+         *Irp->UserIosb = Irp->IoStatus;
+      }
+
+      if (Irp->UserEvent)
+      {
+         KeSetEvent(Irp->UserEvent, PriorityBoost, FALSE);
+      }
       
-           DPRINT("Dispatching APC\n");
+      /* io manager frees the irp for close operations */
-           KeInitializeApc(  &Irp->Tail.Apc,
+      if (Irp->Flags & IRP_PAGING_IO)
+      {
+         IoFreeIrp(Irp);
+      }
+      
+      return;
+   }
+   
+   
+   /*
+   Hi Dave,
+   
+    I went through my old notes. You are correct and in most cases
+   IoCompleteRequest() will issue an MmUnlockPages() for each MDL in the IRP
+   chain. There are however few exceptions: one is MDLs for associated IRPs,
+   it's expected that those MDLs have been initialized with
+   IoBuildPartialMdl(). Another exception is PAGING_IO irps, the i/o completion
+   code doesn't do anything to MDLs of those IRPs.
+   
+   sara
+   
+*/
+   
+   //Windows NT File System Internals, page 166/167
+   if (!(Irp->Flags & IRP_ASSOCIATED_IRP))
+   {
+      for (Mdl = Irp->MdlAddress; Mdl; Mdl = Mdl->Next)
+      {
+         /* 
+          * Undo the MmProbeAndLockPages. If MmGetSystemAddressForMdl was called
+          * on this mdl, this mapping (if any) is also undone by MmUnlockPages.
+          */
+         MmUnlockPages(Irp->MdlAddress);
+      }
+   }
+    
+   //Windows NT File System Internals, page 154
+   OriginalFileObject = Irp->Tail.Overlay.OriginalFileObject;
+
+   if (Irp->PendingReturned || KeGetCurrentIrql() == DISPATCH_LEVEL)
+   {
+      BOOLEAN bStatus;
+      
+      DPRINT("Dispatching APC\n");
+      KeInitializeApc(  &Irp->Tail.Apc,
                              &Irp->Tail.Overlay.Thread->Tcb,
                              OriginalApcEnvironment,
                              IoSecondStageCompletion,//kernel routine
@@ -329,27 +371,27 @@ IofCompleteRequest(PIRP Irp,
                              KernelMode,
                              OriginalFileObject);
       
-           bStatus = KeInsertQueueApc(&Irp->Tail.Apc,
+      bStatus = KeInsertQueueApc(&Irp->Tail.Apc,
                                       (PVOID)Irp,
                                       (PVOID)(ULONG)PriorityBoost,
                                       PriorityBoost);
 
-           if (bStatus == FALSE)
+      if (bStatus == FALSE)
-             {
+      {
-               DPRINT1("Error queueing APC for thread. Thread has probably exited.\n");
+         DPRINT1("Error queueing APC for thread. Thread has probably exited.\n");
-             }
+      }
 
-           DPRINT("Finished dispatching APC\n");
+      DPRINT("Finished dispatching APC\n");
-	 }
-       else
-         {
-           DPRINT("Calling IoSecondStageCompletion routine directly\n");
-           KeRaiseIrql(APC_LEVEL, &oldIrql);
-           IoSecondStageCompletion(NULL,NULL,(PVOID)&OriginalFileObject,(PVOID) &Irp,(PVOID) &PriorityBoost);
-           KeLowerIrql(oldIrql);
-           DPRINT("Finished completition routine\n");
-	 }
    }
+   else
+   {
+      DPRINT("Calling IoSecondStageCompletion routine directly\n");
+      KeRaiseIrql(APC_LEVEL, &oldIrql);
+      IoSecondStageCompletion(NULL,NULL,(PVOID)&OriginalFileObject,(PVOID) &Irp,(PVOID) &PriorityBoost);
+      KeLowerIrql(oldIrql);
+      DPRINT("Finished completition routine\n");
+   }
+
 
 }
 

reactos/ntoskrnl/io/mdl.c

@@ -1,4 +1,4 @@
-/* $Id: mdl.c,v 1.13 2003/12/30 18:52:04 fireball Exp $
+/* $Id: mdl.c,v 1.14 2004/04/20 19:01:47 gdalsnes Exp $
  *
  * COPYRIGHT:       See COPYING in the top level directory
  * PROJECT:         ReactOS kernel
@@ -50,15 +50,38 @@ IoAllocateMdl(PVOID VirtualAddress,
 				    TAG_MDL);
      }
    MmInitializeMdl(Mdl, (char*)VirtualAddress, Length);
-   if (Irp!=NULL && !SecondaryBuffer)
+   
-     {
+   if (Irp)
-	Irp->MdlAddress = Mdl;
+   {
-     }
+      if (SecondaryBuffer)
+      {
+         assert(Irp->MdlAddress);
+         
+         /* FIXME: add to end of list maybe?? */
+         Mdl->Next = Irp->MdlAddress->Next;
+         Irp->MdlAddress->Next = Mdl;
+      }
+      else
+      {
+         /* 
+          * What if there's allready an mdl at Irp->MdlAddress?
+          * Is that bad and should we do something about it?
+          */
+         Irp->MdlAddress = Mdl;
+      }
+   }
+   
    return(Mdl);
 }
 
 /*
  * @implemented
+ *
+ * You must IoFreeMdl the slave before freeing the master.
+ *
+ * IoBuildPartialMdl is more similar to MmBuildMdlForNonPagedPool, the difference
+ * is that the former takes the physical addresses from the master MDL, while the
+ * latter - from the known location of the NPP.
  */
 VOID
 STDCALL
@@ -74,9 +97,11 @@ IoBuildPartialMdl(PMDL SourceMdl,
                  PAGE_SIZE;
 
    for (Va = 0; Va < (PAGE_ROUND_UP(Length)/PAGE_SIZE); Va++)
-     {
+   {
-	TargetPages[Va] = SourcePages[Va+Delta];
+      TargetPages[Va] = SourcePages[Va+Delta];
-     }
+   }
+   
+   TargetMdl->MdlFlags |= MDL_PARTIAL;
 }
 
 /*
@@ -85,8 +110,12 @@ IoBuildPartialMdl(PMDL SourceMdl,
 VOID STDCALL
 IoFreeMdl(PMDL Mdl)
 {   
-   MmUnmapLockedPages(MmGetSystemAddressForMdl(Mdl), Mdl);
+   /* 
-   MmUnlockPages(Mdl);
+    * This unmaps partial mdl's from kernel space but also asserts that non-partial
+    * mdl's isn't still mapped into kernel space.
+    */
+   MmPrepareMdlForReuse(Mdl);
+   
    ExFreePool(Mdl);
 }