Author: ion Date: Mon Mar 5 03:47:19 2007 New Revision: 25993
URL: http://svn.reactos.org/svn/reactos?rev=25993&view=rev Log: - Detect if reparsing is being used during IRP completion and complain. - Free MDLs in a safer way by not actually using the Irp->MdlAddress as we're looping through them. - Don't leak an event for each Asynchronous API anymore. - Handle IRP_OB_QUERY_NAME completion properly. - handle IRP_CREATE_OPERATION with a file object present. - Use deferred delete for File Object dereferences, to speed up I/O completion. - Clear the I/O Stack Location when parsing completion stacks. - Support SL_ERROR_RETURNED during completion routines.
Modified: trunk/reactos/include/ddk/winddk.h trunk/reactos/ntoskrnl/io/iomgr/irp.c
Modified: trunk/reactos/include/ddk/winddk.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ddk/winddk.h?rev=25... ============================================================================== --- trunk/reactos/include/ddk/winddk.h (original) +++ trunk/reactos/include/ddk/winddk.h Mon Mar 5 03:47:19 2007 @@ -3813,6 +3813,7 @@ /* IO_STACK_LOCATION.Control */
#define SL_PENDING_RETURNED 0x01 +#define SL_ERROR_RETURNED 0x02 #define SL_INVOKE_ON_CANCEL 0x20 #define SL_INVOKE_ON_SUCCESS 0x40 #define SL_INVOKE_ON_ERROR 0x80
Modified: trunk/reactos/ntoskrnl/io/iomgr/irp.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/irp.c?rev... ============================================================================== --- trunk/reactos/ntoskrnl/io/iomgr/irp.c (original) +++ trunk/reactos/ntoskrnl/io/iomgr/irp.c Mon Mar 5 03:47:19 2007 @@ -239,7 +239,7 @@ { PFILE_OBJECT FileObject; PIRP Irp; - PMDL Mdl; + PMDL Mdl, NextMdl; PVOID Port = NULL, Key = NULL; BOOLEAN SignaledCreateRequest = FALSE;
@@ -252,10 +252,26 @@ Irp, FileObject);
+ /* Sanity check */ + ASSERT(Irp->IoStatus.Status != 0xFFFFFFFF); + + /* Check if we have a file object */ + if (*SystemArgument2) + { + /* Check if we're reparsing */ + if ((Irp->IoStatus.Status == STATUS_REPARSE) && + (Irp->IoStatus.Information == IO_REPARSE_TAG_MOUNT_POINT)) + { + /* We should never get this yet */ + DPRINT1("Reparse support not yet present!\n"); + while (TRUE); + } + } + /* Handle Buffered case first */ if (Irp->Flags & IRP_BUFFERED_IO) { - /* Check if we have an input buffer and if we suceeded */ + /* Check if we have an input buffer and if we succeeded */ if ((Irp->Flags & IRP_INPUT_OPERATION) && (Irp->IoStatus.Status != STATUS_VERIFY_REQUIRED) && !(NT_ERROR(Irp->IoStatus.Status))) @@ -278,22 +294,25 @@ Irp->Flags &= ~(IRP_BUFFERED_IO | IRP_DEALLOCATE_BUFFER);
/* Check if there's an MDL */ - while ((Mdl = Irp->MdlAddress)) - { - /* Clear all of them */ - Irp->MdlAddress = Mdl->Next; + for (Mdl = Irp->MdlAddress; Mdl; Mdl = NextMdl) + { + /* Free it */ + NextMdl = Mdl->Next; IoFreeMdl(Mdl); } + + /* No MDLs left */ + Irp->MdlAddress = NULL;
/* * Check if either the request was completed without any errors * (but warnings are OK!), or if it was completed with an error, but * did return from a pending I/O Operation and is not synchronous. */ - if ((!NT_ERROR(Irp->IoStatus.Status)) || - (NT_ERROR(Irp->IoStatus.Status) && - (Irp->PendingReturned) && - !(IsIrpSynchronous(Irp, FileObject)))) + if (!(NT_ERROR(Irp->IoStatus.Status)) || + (NT_ERROR(Irp->IoStatus.Status) && + (Irp->PendingReturned) && + !(IsIrpSynchronous(Irp, FileObject)))) { /* Get any information we need from the FO before we kill it */ if ((FileObject) && (FileObject->CompletionContext)) @@ -324,12 +343,20 @@ /* Check if we also have a File Object */ if (FileObject) { + /* Check if this is an Asynch API */ + if (!(Irp->Flags & IRP_SYNCHRONOUS_API)) + { + /* Dereference the event */ + ObDereferenceObject(Irp->UserEvent); + } + /* * Now, if this is a Synch I/O File Object, then this event is * NOT an actual Executive Event, so we won't dereference it, * and instead, we will signal the File Object */ - if (FileObject->Flags & FO_SYNCHRONOUS_IO) + if ((FileObject->Flags & FO_SYNCHRONOUS_IO) && + !(Irp->Flags & IRP_OB_QUERY_NAME)) { /* Signal the file object and set the status */ KeSetEvent(&FileObject->Event, 0, FALSE); @@ -342,7 +369,7 @@ */ if (Irp->Flags & IRP_CREATE_OPERATION) { - /* Clear the APC Routine and remmeber this */ + /* Clear the APC Routine and remember this */ Irp->Overlay.AsynchronousParameters.UserApcRoutine = NULL; SignaledCreateRequest = TRUE; } @@ -353,6 +380,17 @@ /* Signal the file object and set the status */ KeSetEvent(&FileObject->Event, 0, FALSE); FileObject->FinalStatus = Irp->IoStatus.Status; + + /* + * This could also be a create operation, in which case we want + * to make sure there's no APC fired. + */ + if (Irp->Flags & IRP_CREATE_OPERATION) + { + /* Clear the APC Routine and remember this */ + Irp->Overlay.AsynchronousParameters.UserApcRoutine = NULL; + SignaledCreateRequest = TRUE; + } }
/* Now that we've signaled the events, de-associate the IRP */ @@ -394,7 +432,7 @@ if ((FileObject) && !(SignaledCreateRequest)) { /* Dereference it, since it's not needed anymore either */ - ObDereferenceObject(FileObject); + ObDereferenceObjectDeferDelete(FileObject); } } else @@ -408,7 +446,7 @@ /* So we did return with a synch operation, was it the IRP? */ if (Irp->Flags & IRP_SYNCHRONOUS_API) { - /* Yes, this IRP was synchronous, so retrn the I/O Status */ + /* Yes, this IRP was synchronous, so return the I/O Status */ *Irp->UserIosb = Irp->IoStatus;
/* Now check if the user gave an event */ @@ -438,7 +476,7 @@ if ((FileObject) && !(Irp->Flags & IRP_CREATE_OPERATION)) { /* Dereference the File Object unless this was a create */ - ObDereferenceObject(FileObject); + ObDereferenceObjectDeferDelete(FileObject); }
/* @@ -520,7 +558,7 @@ /* Did we try lookaside and fail? */ if (Flags & IRP_ALLOCATED_FIXED_SIZE) List->L.AllocateMisses++;
- /* Check if we shoudl charge quota */ + /* Check if we should charge quota */ if (ChargeQuota) { /* Irp = ExAllocatePoolWithQuotaTag(NonPagedPool, Size, TAG_IRP); */ @@ -1087,6 +1125,19 @@ Irp); }
+FORCEINLINE +VOID +IopClearStackLocation(IN PIO_STACK_LOCATION IoStackLocation) +{ + IoStackLocation->MinorFunction = 0; + IoStackLocation->Flags = 0; + IoStackLocation->Control &= SL_ERROR_RETURNED; + IoStackLocation->Parameters.Others.Argument1 = 0; + IoStackLocation->Parameters.Others.Argument2 = 0; + IoStackLocation->Parameters.Others.Argument3 = 0; + IoStackLocation->FileObject = NULL; +} + /* * @implemented */ @@ -1095,33 +1146,42 @@ IofCompleteRequest(IN PIRP Irp, IN CCHAR PriorityBoost) { - PIO_STACK_LOCATION StackPtr; + PIO_STACK_LOCATION StackPtr, LastStackPtr; PDEVICE_OBJECT DeviceObject; PFILE_OBJECT FileObject; PETHREAD Thread; NTSTATUS Status; - PMDL Mdl; - ULONG MasterIrpCount; + PMDL Mdl, NextMdl; + ULONG MasterCount; PIRP MasterIrp; ULONG Flags; + NTSTATUS ErrorCode = STATUS_SUCCESS; IOTRACE(IO_IRP_DEBUG, "%s - Completing IRP %p\n", __FUNCTION__, Irp);
/* Make sure this IRP isn't getting completed twice or is invalid */ - if (((Irp->CurrentLocation) > (Irp->StackCount + 1)) || - (Irp->Type != IO_TYPE_IRP)) + if ((Irp->CurrentLocation) > (Irp->StackCount + 1)) { /* Bugcheck */ KeBugCheckEx(MULTIPLE_IRP_COMPLETE_REQUESTS, (ULONG_PTR)Irp, 0, 0, 0); }
/* Some sanity checks */ + ASSERT(Irp->Type == IO_TYPE_IRP); ASSERT(!Irp->CancelRoutine); ASSERT(Irp->IoStatus.Status != STATUS_PENDING); ASSERT(Irp->IoStatus.Status != 0xFFFFFFFF);
+ /* Get the last stack */ + LastStackPtr = (PIO_STACK_LOCATION)(Irp + 1); + if (LastStackPtr->Control & SL_ERROR_RETURNED) + { + /* Get the error code */ + ErrorCode = (NTSTATUS)LastStackPtr->Parameters.Others.Argument4; + } + /* Get the Current Stack and skip it */ StackPtr = IoGetCurrentIrpStackLocation(Irp); IoSkipCurrentIrpStackLocation(Irp); @@ -1131,14 +1191,32 @@ { /* Set Pending Returned */ Irp->PendingReturned = StackPtr->Control & SL_PENDING_RETURNED; + + /* Check if we failed */ + if (!NT_SUCCESS(Irp->IoStatus.Status)) + { + /* Check if it was changed by a completion routine */ + if (Irp->IoStatus.Status != ErrorCode) + { + /* Update the error for the current stack */ + ErrorCode = Irp->IoStatus.Status; + StackPtr->Control |= SL_ERROR_RETURNED; + LastStackPtr->Parameters.Others.Argument4 = (PVOID)ErrorCode; + LastStackPtr->Control |= SL_ERROR_RETURNED; + } + }
/* Check if there is a Completion Routine to Call */ if ((NT_SUCCESS(Irp->IoStatus.Status) && (StackPtr->Control & SL_INVOKE_ON_SUCCESS)) || (!NT_SUCCESS(Irp->IoStatus.Status) && (StackPtr->Control & SL_INVOKE_ON_ERROR)) || - (Irp->Cancel && (StackPtr->Control & SL_INVOKE_ON_CANCEL))) - { + (Irp->Cancel && + (StackPtr->Control & SL_INVOKE_ON_CANCEL))) + { + /* Clear the stack location */ + IopClearStackLocation(StackPtr); + /* Check for highest-level device completion routines */ if (Irp->CurrentLocation == (Irp->StackCount + 1)) { @@ -1168,6 +1246,9 @@ /* Mark it as pending */ IoMarkIrpPending(Irp); } + + /* Clear the stack location */ + IopClearStackLocation(StackPtr); }
/* Move to next stack location and pointer */ @@ -1180,17 +1261,13 @@ { /* Get the master IRP and count */ MasterIrp = Irp->AssociatedIrp.MasterIrp; - MasterIrpCount = InterlockedDecrement(&MasterIrp-> - AssociatedIrp.IrpCount); - - /* Set the thread of this IRP as the master's */ - Irp->Tail.Overlay.Thread = MasterIrp->Tail.Overlay.Thread; + MasterCount = InterlockedDecrement(&MasterIrp->AssociatedIrp.IrpCount);
/* Free the MDLs */ - while ((Mdl = Irp->MdlAddress)) + for (Mdl = Irp->MdlAddress; Mdl; Mdl = NextMdl) { /* Go to the next one */ - Irp->MdlAddress = Mdl->Next; + NextMdl = Mdl->Next; IoFreeMdl(Mdl); }
@@ -1198,7 +1275,7 @@ IoFreeIrp(Irp);
/* Complete the Master IRP */ - if (!MasterIrpCount) IofCompleteRequest(MasterIrp, PriorityBoost); + if (!MasterCount) IofCompleteRequest(MasterIrp, PriorityBoost); return; }
@@ -1216,11 +1293,11 @@ /* Check if this is a Paging I/O or Close Operation */ if (Irp->Flags & (IRP_PAGING_IO | IRP_CLOSE_OPERATION)) { - /* Handle a Close Operation or Sync Paging I/O (see page 165) */ + /* Handle a Close Operation or Sync Paging I/O */ if (Irp->Flags & (IRP_SYNCHRONOUS_PAGING_IO | IRP_CLOSE_OPERATION)) { /* Set the I/O Status and Signal the Event */ - Flags = Irp->Flags & IRP_SYNCHRONOUS_PAGING_IO; + Flags = Irp->Flags & (IRP_SYNCHRONOUS_PAGING_IO | IRP_PAGING_IO); *Irp->UserIosb = Irp->IoStatus; KeSetEvent(Irp->UserEvent, PriorityBoost, FALSE);
@@ -1245,7 +1322,8 @@ PriorityBoost); #else /* Not implemented yet. */ - ASSERT(FALSE); + DPRINT1("Not supported!\n"); + while (TRUE); #endif }