[cgutman] 55157: [USBSTOR] - Rewrite pending SRB handling and change some behavior of the IRP queue - The caller is no longer responsible for checking whether it can call USBSTOR_QueueNextRequest; ...
Author: cgutman Date: Tue Jan 24 23:04:31 2012 New Revision: 55157
URL: http://svn.reactos.org/svn/reactos?rev=55157&view=rev Log: [USBSTOR] - Rewrite pending SRB handling and change some behavior of the IRP queue - The caller is no longer responsible for checking whether it can call USBSTOR_QueueNextRequest; frozen IRP queue and pending SRB are both handled for them - It's no longer required for the caller of USBSTOR_QueueTerminateRequest to know whether the SRB was active (which was impossible before when handling a cancellation) - Many potential race issues with IRP cancellation are eliminated - Debugging hung SRBs is much easier now that pointer to the active one is stored in the FDO
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/error.c branches/usb-bringup-trunk/drivers/usb/usbstor/queue.c branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/error.c URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/us... ============================================================================== --- branches/usb-bringup-trunk/drivers/usb/usbstor/error.c [iso-8859-1] (original) +++ branches/usb-bringup-trunk/drivers/usb/usbstor/error.c [iso-8859-1] Tue Jan 24 23:04:31 2012 @@ -129,10 +129,10 @@ /* Complete the master IRP */ Context->Irp->IoStatus.Status = Status; Context->Irp->IoStatus.Information = 0; + USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Context->Irp); IoCompleteRequest(Context->Irp, IO_NO_INCREMENT);
/* Start the next request */ - USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, TRUE); USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject);
/* Signal the context event */
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/queue.c URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/us... ============================================================================== --- branches/usb-bringup-trunk/drivers/usb/usbstor/queue.c [iso-8859-1] (original) +++ branches/usb-bringup-trunk/drivers/usb/usbstor/queue.c [iso-8859-1] Tue Jan 24 23:04:31 2012 @@ -67,7 +67,13 @@ // // now cancel the irp // + USBSTOR_QueueTerminateRequest(DeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT); + + // + // start the next one + // + USBSTOR_QueueNextRequest(DeviceObject); }
VOID @@ -117,7 +123,13 @@ // // now cancel the irp // + USBSTOR_QueueTerminateRequest(DeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT); + + // + // start the next one + // + USBSTOR_QueueNextRequest(DeviceObject); }
BOOLEAN @@ -358,8 +370,14 @@ // // complete request // + USBSTOR_QueueTerminateRequest(DeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT);
+ // + // start next one + // + USBSTOR_QueueNextRequest(DeviceObject); + // // acquire lock // @@ -375,10 +393,12 @@ VOID USBSTOR_QueueTerminateRequest( IN PDEVICE_OBJECT FDODeviceObject, - IN BOOLEAN ModifySrbState) + IN PIRP Irp) { KIRQL OldLevel; PFDO_DEVICE_EXTENSION FDODeviceExtension; + PIO_STACK_LOCATION IoStack = IoGetCurrentIrpStackLocation(Irp); + PSCSI_REQUEST_BLOCK Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1;
// // get FDO device extension @@ -400,17 +420,15 @@ // FDODeviceExtension->IrpPendingCount--;
- if (ModifySrbState) - { - // - // sanity check - // - ASSERT(FDODeviceExtension->SrbActive == TRUE); - + // + // check if this was our current active SRB + // + if (FDODeviceExtension->ActiveSrb == Request) + { // // indicate processing is completed // - FDODeviceExtension->SrbActive = FALSE; + FDODeviceExtension->ActiveSrb = NULL; }
// @@ -440,6 +458,18 @@ ASSERT(FDODeviceExtension->Common.IsFDO);
// + // check first if there's already a request pending or the queue is frozen + // + if (FDODeviceExtension->ActiveSrb != NULL || + FDODeviceExtension->IrpListFreeze) + { + // + // no work to do yet + // + return; + } + + // // remove first irp from list // Irp = USBSTOR_RemoveIrp(DeviceObject); @@ -470,6 +500,11 @@ // sanity check // ASSERT(Request); + + // + // set the active SRB + // + FDODeviceExtension->ActiveSrb = Request;
// // start next packet @@ -605,21 +640,19 @@ Irp->IoStatus.Information = 0;
// + // terminate request + // + USBSTOR_QueueTerminateRequest(DeviceObject, Irp); + + // // complete request // IoCompleteRequest(Irp, IO_NO_INCREMENT);
// - // check if the queue has been frozen - // - if (FDODeviceExtension->IrpListFreeze == FALSE) - { - // - // queue next request - // - USBSTOR_QueueTerminateRequest(DeviceObject, FALSE); - USBSTOR_QueueNextRequest(DeviceObject); - } + // queue next request + // + USBSTOR_QueueNextRequest(DeviceObject);
// // done @@ -644,12 +677,6 @@ ASSERT(ResetInProgress == FALSE);
// - // sanity check - // - ASSERT(FDODeviceExtension->SrbActive == FALSE); - FDODeviceExtension->SrbActive = TRUE; - - // // release lock // KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel); @@ -679,8 +706,8 @@ // Irp->IoStatus.Information = 0; Irp->IoStatus.Status = STATUS_DEVICE_DOES_NOT_EXIST; + USBSTOR_QueueTerminateRequest(DeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT); - USBSTOR_QueueTerminateRequest(DeviceObject, TRUE); return; }
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/us... ============================================================================== --- branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c [iso-8859-1] (original) +++ branches/usb-bringup-trunk/drivers/usb/usbstor/scsi.c [iso-8859-1] Tue Jan 24 23:04:31 2012 @@ -282,14 +282,14 @@ Context->Irp->IoStatus.Information = Context->TransferDataLength;
// + // terminate current request + // + USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, Context->Irp); + + // // complete request // IoCompleteRequest(Context->Irp, IO_NO_INCREMENT); - - // - // terminate current request - // - USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject, TRUE);
// // start next request @@ -840,6 +840,16 @@ PSCSI_REQUEST_BLOCK Request;
// + // get PDO device extension + // + PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension; + + // + // sanity check + // + ASSERT(PDODeviceExtension->Common.IsFDO == FALSE); + + // // get current stack location // IoStack = IoGetCurrentIrpStackLocation(Irp); @@ -853,22 +863,8 @@ Request->SrbStatus = SRB_STATUS_SUCCESS; Irp->IoStatus.Information = Request->DataTransferLength; Irp->IoStatus.Status = STATUS_SUCCESS; + USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT); - - // - // get PDO device extension - // - PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension; - - // - // sanity check - // - ASSERT(PDODeviceExtension->Common.IsFDO == FALSE); - - // - // terminate current request - // - USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, TRUE);
// // start next request @@ -1204,12 +1200,8 @@ Request->SrbStatus = SRB_STATUS_SUCCESS; Irp->IoStatus.Status = STATUS_SUCCESS; Irp->IoStatus.Information = Request->DataTransferLength; + USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, Irp); IoCompleteRequest(Irp, IO_NO_INCREMENT); - - // - // terminate current request - // - USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject, TRUE);
// // start next request
Modified: branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h URL: http://svn.reactos.org/svn/reactos/branches/usb-bringup-trunk/drivers/usb/us... ============================================================================== --- branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h [iso-8859-1] (original) +++ branches/usb-bringup-trunk/drivers/usb/usbstor/usbstor.h [iso-8859-1] Tue Jan 24 23:04:31 2012 @@ -66,7 +66,7 @@ BOOLEAN IrpListFreeze; // if true the irp list is freezed BOOLEAN ResetInProgress; // if hard reset is in progress ULONG IrpPendingCount; // count of irp pending - BOOLEAN SrbActive; // debug field if srb is pending + PSCSI_REQUEST_BLOCK ActiveSrb; // stores the current active SRB }FDO_DEVICE_EXTENSION, *PFDO_DEVICE_EXTENSION;
typedef struct @@ -439,4 +439,4 @@ VOID USBSTOR_QueueTerminateRequest( IN PDEVICE_OBJECT DeviceObject, - IN BOOLEAN ModifySrbState); + IN PIRP Irp);
-
cgutman＠svn.reactos.org