24 Jan
2012
24 Jan
'12
11:04 p.m.
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/u…
==============================================================================
--- 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/u…
==============================================================================
--- 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/u…
==============================================================================
--- 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/u…
==============================================================================
--- 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);