diff --git a/reactos/drivers/fs/vfat/blockdev.c b/reactos/drivers/fs/vfat/blockdev.c index 221142c4b59..529f85734ba 100644 --- a/reactos/drivers/fs/vfat/blockdev.c +++ b/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); } diff --git a/reactos/include/ddk/mmfuncs.h b/reactos/include/ddk/mmfuncs.h index 21c2a33073d..2940bdc9be8 100644 --- a/reactos/include/ddk/mmfuncs.h +++ b/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)->ByteOffset = (ULONG)((BaseVa) - PAGE_ROUND_DOWN((BaseVa))); \ + (MemoryDescriptorList)->StartVa = (PVOID)PAGE_BASE(BaseVa); \ + (MemoryDescriptorList)->ByteOffset = (ULONG)PAGE_OFFSET(BaseVa); \ (MemoryDescriptorList)->ByteCount = (Length); \ - (MemoryDescriptorList)->Process = PsGetCurrentProcess(); \ } /* diff --git a/reactos/ntoskrnl/io/cleanup.c b/reactos/ntoskrnl/io/cleanup.c index bf7858dda0a..f9bb8995346 100644 --- a/reactos/ntoskrnl/io/cleanup.c +++ b/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) - { - DPRINT("Copying buffered io back to user\n"); - memcpy(Irp->UserBuffer,Irp->AssociatedIrp.SystemBuffer, + { + if (IoStack->MajorFunction == IRP_MJ_READ) + { + DPRINT("Copying buffered io back to user\n"); + 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. */ - DPRINT("Tearing down MDL\n"); - if (Irp->MdlAddress->MappedSystemVa != NULL) - { - MmUnmapLockedPages(Irp->MdlAddress->MappedSystemVa, - Irp->MdlAddress); - } - MmUnlockPages(Irp->MdlAddress); - ExFreePool(Irp->MdlAddress); - } + { + IoFreeMdl(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) diff --git a/reactos/ntoskrnl/io/irp.c b/reactos/ntoskrnl/io/irp.c index d720f007cce..08eee855758 100644 --- a/reactos/ntoskrnl/io/irp.c +++ b/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)) - { - if (Irp->Flags & IRP_PAGING_IO) - { - /* 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; + /* + * Were done calling completion routines. Now do any cleanup that can be + * done in an arbitrarily context. + */ - 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"); - KeInitializeApc( &Irp->Tail.Apc, + /* io manager frees the irp for close operations */ + 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) - { - DPRINT1("Error queueing APC for thread. Thread has probably exited.\n"); - } + if (bStatus == FALSE) + { + DPRINT1("Error queueing APC for thread. Thread has probably exited.\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"); - } + 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"); + } + } diff --git a/reactos/ntoskrnl/io/mdl.c b/reactos/ntoskrnl/io/mdl.c index f476511461f..6ff5aeb1310 100644 --- a/reactos/ntoskrnl/io/mdl.c +++ b/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) - { - Irp->MdlAddress = Mdl; - } + + if (Irp) + { + 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); }