Author: janderwald
Date: Sun May 15 14:59:16 2011
New Revision: 51764
URL: http://svn.reactos.org/svn/reactos?rev=51764&view=rev
Log:
[USBSTOR]
- Add asserts
- Rewrite queue methods to accept the FDO only
- Fix multiple synchronization bugs
- Don't start next irp before current irp has completed
- Use contiouslogicalbytes blocks from srb
- Should fix race conditions
Modified:
branches/usb-bringup/drivers/usb/usbstor/disk.c
branches/usb-bringup/drivers/usb/usbstor/queue.c
branches/usb-bringup/drivers/usb/usbstor/scsi.c
branches/usb-bringup/drivers/usb/usbstor/usbstor.h
Modified: branches/usb-bringup/drivers/usb/usbstor/disk.c
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbstor/disk.c?rev=51764&r1=51763&r2=51764&view=diff
==============================================================================
--- branches/usb-bringup/drivers/usb/usbstor/disk.c [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbstor/disk.c [iso-8859-1] Sun May 15
14:59:16 2011
@@ -32,6 +32,11 @@
Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1;
//
+ // sanity check
+ //
+ ASSERT(Request);
+
+ //
// get device extension
//
PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
@@ -39,8 +44,7 @@
//
// sanity check
//
- ASSERT(Request);
- ASSERT(PDODeviceExtension);
+ ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
switch(Request->Function)
{
@@ -87,7 +91,7 @@
//
// add the request
//
- if (!USBSTOR_QueueAddIrp(DeviceObject, Irp))
+ if (!USBSTOR_QueueAddIrp(PDODeviceExtension->LowerDeviceObject,
Irp))
{
//
// irp was not added to the queue
@@ -154,8 +158,7 @@
//
// release queue
//
- USBSTOR_QueueRelease(DeviceObject);
-
+ USBSTOR_QueueRelease(PDODeviceExtension->LowerDeviceObject);
//
// set status success
@@ -173,7 +176,7 @@
//
// flush all requests
//
- USBSTOR_QueueFlushIrps(DeviceObject);
+ USBSTOR_QueueFlushIrps(PDODeviceExtension->LowerDeviceObject);
//
// set status success
Modified: branches/usb-bringup/drivers/usb/usbstor/queue.c
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbstor/queue.c?rev=51764&r1=51763&r2=51764&view=diff
==============================================================================
--- branches/usb-bringup/drivers/usb/usbstor/queue.c [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbstor/queue.c [iso-8859-1] Sun May 15
14:59:16 2011
@@ -15,6 +15,10 @@
USBSTOR_QueueInitialize(
PFDO_DEVICE_EXTENSION FDODeviceExtension)
{
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// initialize queue lock
@@ -85,21 +89,19 @@
{
PDRIVER_CANCEL OldDriverCancel;
KIRQL OldLevel;
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
PFDO_DEVICE_EXTENSION FDODeviceExtension;
BOOLEAN IrpListFreeze;
BOOLEAN SrbProcessing;
//
- // get pdo device extension
- //
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
- ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-
- //
// get FDO device extension
//
- FDODeviceExtension =
(PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
+ FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// mark irp pending
@@ -183,25 +185,19 @@
IN PDEVICE_OBJECT DeviceObject)
{
KIRQL OldLevel;
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
PFDO_DEVICE_EXTENSION FDODeviceExtension;
PLIST_ENTRY Entry;
PIRP Irp = NULL;
//
- // get pdo device extension
- //
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
-
- //
- // sanity check
- //
- ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-
- //
// get FDO device extension
//
- FDODeviceExtension =
(PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
+ FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// acquire lock
@@ -240,7 +236,6 @@
IN PDEVICE_OBJECT DeviceObject)
{
KIRQL OldLevel;
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
PFDO_DEVICE_EXTENSION FDODeviceExtension;
PLIST_ENTRY Entry;
PIRP Irp;
@@ -248,19 +243,14 @@
PSCSI_REQUEST_BLOCK Request;
//
- // get pdo device extension
- //
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
-
- //
- // sanity check
- //
- ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-
- //
// get FDO device extension
//
- FDODeviceExtension =
(PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
+ FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// acquire lock
@@ -320,10 +310,58 @@
}
VOID
+USBSTOR_QueueTerminateRequest(
+ IN PDEVICE_OBJECT FDODeviceObject,
+ IN BOOLEAN ModifySrbState)
+{
+ KIRQL OldLevel;
+ PFDO_DEVICE_EXTENSION FDODeviceExtension;
+
+ //
+ // get FDO device extension
+ //
+ FDODeviceExtension =
(PFDO_DEVICE_EXTENSION)FDODeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
+
+ //
+ // acquire lock
+ //
+ KeAcquireSpinLock(&FDODeviceExtension->IrpListLock, &OldLevel);
+
+ //
+ // decrement pending irp count
+ //
+ FDODeviceExtension->IrpPendingCount--;
+
+ if (ModifySrbState)
+ {
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->SrbActive == TRUE);
+
+ //
+ // indicate processing is completed
+ //
+ FDODeviceExtension->SrbActive = FALSE;
+ }
+
+ //
+ // release lock
+ //
+ KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel);
+
+}
+
+VOID
USBSTOR_QueueNextRequest(
IN PDEVICE_OBJECT DeviceObject)
{
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
+ PFDO_DEVICE_EXTENSION FDODeviceExtension;
PIRP Irp;
PIO_STACK_LOCATION IoStack;
PSCSI_REQUEST_BLOCK Request;
@@ -331,12 +369,12 @@
//
// get pdo device extension
//
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
-
- //
- // sanity check
- //
- ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
+ FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// remove first irp from list
@@ -351,6 +389,7 @@
//
// no work to do
//
+ IoStartNextPacket(DeviceObject, TRUE);
return;
}
@@ -365,16 +404,25 @@
Request = (PSCSI_REQUEST_BLOCK)IoStack->Parameters.Others.Argument1;
//
+ // sanity check
+ //
+ ASSERT(Request);
+
+ //
// start next packet
//
- IoStartPacket(PDODeviceExtension->LowerDeviceObject, Irp,
&Request->QueueSortKey, USBSTOR_CancelIo);
+ IoStartPacket(DeviceObject, Irp, &Request->QueueSortKey, USBSTOR_CancelIo);
+
+ //
+ // start next request
+ //
+ IoStartNextPacket(DeviceObject, TRUE);
}
VOID
USBSTOR_QueueRelease(
IN PDEVICE_OBJECT DeviceObject)
{
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
PFDO_DEVICE_EXTENSION FDODeviceExtension;
PIRP Irp;
KIRQL OldLevel;
@@ -382,19 +430,14 @@
PSCSI_REQUEST_BLOCK Request;
//
- // get pdo device extension
- //
- PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
-
- //
- // sanity check
- //
- ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
-
- //
// get FDO device extension
//
- FDODeviceExtension =
(PFDO_DEVICE_EXTENSION)PDODeviceExtension->LowerDeviceObject->DeviceExtension;
+ FDODeviceExtension = (PFDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->Common.IsFDO);
//
// acquire lock
@@ -440,7 +483,7 @@
//
// start new packet
//
- IoStartPacket(PDODeviceExtension->LowerDeviceObject, // FDO
+ IoStartPacket(DeviceObject,
Irp,
&Request->QueueSortKey,
USBSTOR_CancelIo);
@@ -511,12 +554,8 @@
//
// queue next request
//
+ USBSTOR_QueueTerminateRequest(DeviceObject, FALSE);
USBSTOR_QueueNextRequest(DeviceObject);
-
- //
- // start next request
- //
- IoStartNextPacket(DeviceObject, TRUE);
}
//
@@ -540,6 +579,12 @@
//
ResetInProgress = FDODeviceExtension->ResetInProgress;
ASSERT(ResetInProgress == FALSE);
+
+ //
+ // sanity check
+ //
+ ASSERT(FDODeviceExtension->SrbActive == FALSE);
+ FDODeviceExtension->SrbActive = TRUE;
//
// release lock
@@ -572,37 +617,16 @@
Irp->IoStatus.Information = 0;
Irp->IoStatus.Status = STATUS_DEVICE_DOES_NOT_EXIST;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
- }
- else
- {
- //
- // execute scsi
- //
- Status = USBSTOR_HandleExecuteSCSI(IoStack->DeviceObject, Irp);
-
- //
- // acquire lock
- //
- KeAcquireSpinLock(&FDODeviceExtension->IrpListLock, &OldLevel);
-
- //
- // FIXME: synchronize with error handler
- //
- FDODeviceExtension->IrpPendingCount--;
-
- //
- // release lock
- //
- KeReleaseSpinLock(&FDODeviceExtension->IrpListLock, OldLevel);
- }
-
- //
- // FIXME: synchronize action with error handling
- //
- USBSTOR_QueueNextRequest(IoStack->DeviceObject);
-
- //
- // start next request
- //
- IoStartNextPacket(DeviceObject, TRUE);
-}
+ USBSTOR_QueueTerminateRequest(DeviceObject, TRUE);
+ return;
+ }
+
+ //
+ // execute scsi
+ //
+ Status = USBSTOR_HandleExecuteSCSI(IoStack->DeviceObject, Irp);
+
+ //
+ // FIXME: handle error
+ //
+}
Modified: branches/usb-bringup/drivers/usb/usbstor/scsi.c
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbstor/scsi.c?rev=51764&r1=51763&r2=51764&view=diff
==============================================================================
--- branches/usb-bringup/drivers/usb/usbstor/scsi.c [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbstor/scsi.c [iso-8859-1] Sun May 15
14:59:16 2011
@@ -233,6 +233,16 @@
// complete request
//
IoCompleteRequest(Context->Irp, IO_NO_INCREMENT);
+
+ //
+ // terminate current request
+ //
+
USBSTOR_QueueTerminateRequest(Context->PDODeviceExtension->LowerDeviceObject,
TRUE);
+
+ //
+ // start next request
+ //
+
USBSTOR_QueueNextRequest(Context->PDODeviceExtension->LowerDeviceObject);
}
if (Context->Event)
@@ -432,7 +442,7 @@
IN PDEVICE_OBJECT DeviceObject,
IN PIRP OriginalRequest,
IN OPTIONAL PKEVENT Event,
- IN ULONG CommandLength,
+ IN UCHAR CommandLength,
IN PUCHAR Command,
IN ULONG TransferDataLength,
IN PUCHAR TransferData)
@@ -740,11 +750,11 @@
UFI_SENSE_CMD Cmd;
NTSTATUS Status;
PVOID Response;
- PPDO_DEVICE_EXTENSION PDODeviceExtension;
PCBW OutControl;
PCDB pCDB;
PUFI_MODE_PARAMETER_HEADER Header;
#endif
+ PPDO_DEVICE_EXTENSION PDODeviceExtension;
PIO_STACK_LOCATION IoStack;
PSCSI_REQUEST_BLOCK Request;
@@ -763,6 +773,26 @@
Irp->IoStatus.Information = Request->DataTransferLength;
Irp->IoStatus.Status = STATUS_SUCCESS;
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
+ //
+ USBSTOR_QueueNextRequest(PDODeviceExtension->LowerDeviceObject);
return STATUS_SUCCESS;
@@ -956,7 +986,8 @@
RtlZeroMemory(&Cmd, sizeof(UFI_READ_WRITE_CMD));
Cmd.Code = pCDB->AsByte[0];
Cmd.LUN = (PDODeviceExtension->LUN & MAX_LUN);
- Cmd.ContiguousLogicBlocks = _byteswap_ushort(BlockCount);
+ Cmd.ContiguousLogicBlocksByte0 = pCDB->CDB10.TransferBlocksMsb;
+ Cmd.ContiguousLogicBlocksByte1 = pCDB->CDB10.TransferBlocksLsb;
Cmd.LogicalBlockByte0 = pCDB->CDB10.LogicalBlockByte0;
Cmd.LogicalBlockByte1 = pCDB->CDB10.LogicalBlockByte1;
Cmd.LogicalBlockByte2 = pCDB->CDB10.LogicalBlockByte2;
@@ -1023,6 +1054,17 @@
NTSTATUS Status;
PIO_STACK_LOCATION IoStack;
PSCSI_REQUEST_BLOCK Request;
+ PPDO_DEVICE_EXTENSION PDODeviceExtension;
+
+ //
+ // get PDO device extension
+ //
+ PDODeviceExtension = (PPDO_DEVICE_EXTENSION)DeviceObject->DeviceExtension;
+
+ //
+ // sanity check
+ //
+ ASSERT(PDODeviceExtension->Common.IsFDO == FALSE);
//
// get current stack location
@@ -1082,6 +1124,17 @@
Irp->IoStatus.Status = STATUS_SUCCESS;
Irp->IoStatus.Information = Request->DataTransferLength;
IoCompleteRequest(Irp, IO_NO_INCREMENT);
+
+ //
+ // terminate current request
+ //
+ USBSTOR_QueueTerminateRequest(PDODeviceExtension->LowerDeviceObject,
TRUE);
+
+ //
+ // start next request
+ //
+ USBSTOR_QueueNextRequest(PDODeviceExtension->LowerDeviceObject);
+
return STATUS_SUCCESS;
}
else if (pCDB->MODE_SENSE.OperationCode == SCSIOP_TEST_UNIT_READY)
Modified: branches/usb-bringup/drivers/usb/usbstor/usbstor.h
URL:
http://svn.reactos.org/svn/reactos/branches/usb-bringup/drivers/usb/usbstor/usbstor.h?rev=51764&r1=51763&r2=51764&view=diff
==============================================================================
--- branches/usb-bringup/drivers/usb/usbstor/usbstor.h [iso-8859-1] (original)
+++ branches/usb-bringup/drivers/usb/usbstor/usbstor.h [iso-8859-1] Sun May 15
14:59:16 2011
@@ -68,6 +68,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
}FDO_DEVICE_EXTENSION, *PFDO_DEVICE_EXTENSION;
typedef struct
@@ -164,7 +165,8 @@
UCHAR LogicalBlockByte2; // lba
byte 2
UCHAR LogicalBlockByte3; // lba
byte 3
UCHAR Reserved; //
reserved 0x00
- USHORT ContiguousLogicBlocks; // num of
contiguous logical blocks
+ UCHAR ContiguousLogicBlocksByte0; // msb
contigious logic blocks byte
+ UCHAR ContiguousLogicBlocksByte1; // msb
contigious logic blocks
UCHAR Reserved1[3]; //
reserved 0x00
}UFI_READ_WRITE_CMD;
@@ -424,3 +426,11 @@
USBSTOR_QueueInitialize(
PFDO_DEVICE_EXTENSION FDODeviceExtension);
+VOID
+USBSTOR_QueueNextRequest(
+ IN PDEVICE_OBJECT DeviceObject);
+
+VOID
+USBSTOR_QueueTerminateRequest(
+ IN PDEVICE_OBJECT DeviceObject,
+ IN BOOLEAN ModifySrbState);
