[janderwald] 51764: [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 contious...
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/... ============================================================================== --- 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/... ============================================================================== --- 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/... ============================================================================== --- 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/... ============================================================================== --- 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);
-
janderwald＠svn.reactos.org