[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

svn path=/branches/usb-bringup-trunk/; revision=55157

drivers/usb/usbstor/error.c

@@ -129,10 +129,10 @@ USBSTOR_HandleTransferError(
         /* 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 */

drivers/usb/usbstor/queue.c

@@ -67,7 +67,13 @@ USBSTOR_CancelIo(
     //
     // 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 @@ USBSTOR_Cancel(
     //
     // 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 @@ USBSTOR_QueueFlushIrps(
         //
         // complete request
         //
+        USBSTOR_QueueTerminateRequest(DeviceObject, Irp);
         IoCompleteRequest(Irp, IO_NO_INCREMENT);
         
+        //
+        // start next one
+        //
+        USBSTOR_QueueNextRequest(DeviceObject);
+
         //
         // acquire lock
         //
@@ -375,10 +393,12 @@ USBSTOR_QueueFlushIrps(
 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 @@ USBSTOR_QueueTerminateRequest(
     //
     FDODeviceExtension->IrpPendingCount--;
 
-    if (ModifySrbState)
+    //
+    // check if this was our current active SRB
+    //
+    if (FDODeviceExtension->ActiveSrb == Request)
     {
-        //
-        // sanity check
-        //
-        ASSERT(FDODeviceExtension->SrbActive == TRUE);
-
         //
         // indicate processing is completed
         //
-        FDODeviceExtension->SrbActive = FALSE;
+        FDODeviceExtension->ActiveSrb = NULL;
     }
 
     //
@@ -439,6 +457,18 @@ USBSTOR_QueueNextRequest(
     //
     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
     //
@@ -471,6 +501,11 @@ USBSTOR_QueueNextRequest(
     //
     ASSERT(Request);
 
+    //
+    // set the active SRB
+    //
+    FDODeviceExtension->ActiveSrb = Request;
+
     //
     // start next packet
     //
@@ -604,22 +639,20 @@ USBSTOR_StartIo(
         Irp->IoStatus.Status = STATUS_CANCELLED;
         Irp->IoStatus.Information = 0;
 
+        //
+        // terminate request
+        //
+        USBSTOR_QueueTerminateRequest(DeviceObject, Irp);
+
         //
         // complete request
         //
         IoCompleteRequest(Irp, IO_NO_INCREMENT);
 
         //
-        // check if the queue has been frozen
+        // queue next request
         //
-        if (FDODeviceExtension->IrpListFreeze == FALSE)
+        USBSTOR_QueueNextRequest(DeviceObject);
-        {
-            //
-            // queue next request
-            //
-            USBSTOR_QueueTerminateRequest(DeviceObject, FALSE);
-            USBSTOR_QueueNextRequest(DeviceObject);
-        }
 
         //
         // done
@@ -643,12 +676,6 @@ USBSTOR_StartIo(
     ResetInProgress = FDODeviceExtension->ResetInProgress;
     ASSERT(ResetInProgress == FALSE);
 
-    //
-    // sanity check
-    //
-    ASSERT(FDODeviceExtension->SrbActive == FALSE);
-    FDODeviceExtension->SrbActive = TRUE;
-
     //
     // release lock
     //
@@ -679,8 +706,8 @@ USBSTOR_StartIo(
         //
         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;
     }
 

drivers/usb/usbstor/scsi.c

@@ -281,16 +281,16 @@ USBSTOR_CSWCompletionRoutine(
         Context->Irp->IoStatus.Status = Irp->IoStatus.Status;
         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
         //
@@ -839,6 +839,16 @@ USBSTOR_SendModeSenseCmd(
     PIO_STACK_LOCATION IoStack;
     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
     //
@@ -853,23 +863,9 @@ USBSTOR_SendModeSenseCmd(
     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,13 +1200,9 @@ USBSTOR_HandleExecuteSCSI(
         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
         //

drivers/usb/usbstor/usbstor.h

@@ -66,7 +66,7 @@ typedef struct
     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 @@ USBSTOR_QueueNextRequest(
 VOID
 USBSTOR_QueueTerminateRequest(
     IN PDEVICE_OBJECT DeviceObject,
-    IN BOOLEAN ModifySrbState);
+    IN PIRP Irp);