From 995e33bedcadf2d193085bdf2889ea651b1a6abe Mon Sep 17 00:00:00 2001 From: Hao Wu Date: Wed, 23 Dec 2015 01:37:50 +0000 Subject: MdeModulePkg ScsiDiskDxe: Raise the Tpl of async IO callback to TPL_NOTIFY When reading data from non-blockingly from a CD-ROM logic partition, the procedure can be shown by the following call stack: (The write process is similar) |-------------------| | DiskIoDxe (logic) |<---Raise Tpl to TPL_CALLBACK |-------------------| | | Sub-task 1 (UnderRun) succeeds | | |-----------------| |-----------------| |-----------------| +--->| PartitionDxe |---->| DiskIoDxe (Phy) |---->| ScsiDiskDxe | | |-----------------| |-----------------| |-----------------| | | Sub-task 2 (OverRun) fails | | |-----------------| |-----------------| |-----------------| +--->| PartitionDxe |---->| DiskIoDxe (Phy) |---->| ScsiDiskDxe | | |-----------------| |-----------------| |-----------------| | ^ | | More subtasks... Wait indefinitely | |<---Restore Tpl | Completes In PartitionDxe, if the 'Lba' and 'BufferSize' parameters passed to function PartitionReadBlocksEx() are invalid, the function will issue a blocking ReadDisk call (in function ProbeMediaStatusEx()). In DiskIoDxe, blocking I/O request will wait for all the non-blocking I/O requests to complete first before sending down the blocking request. If the Tpl of the async I/O callback in ScsiDiskDxe is TPL_CALLBACK and Sub-task 1 (UnderRun) succeeds but Sub-task 2 (OverRun) fails with an invalid parameter, DiskIoDxe will wait indefinitely for the event created by ScsiDiskDxe of Sub-task 1 to signal. Hence, this commit will raise the Tpl of async IO callback in ScsiDiskDxe to TPL_NOTIFY so that the indefinite wait in DiskIoDxe can be avoided. (Sync patch r19452 from main trunk.) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu Reviewed-by: Feng Tian git-svn-id: https://svn.code.sf.net/p/edk2/code/branches/UDK2015@19473 6f19259b-4bc3-4df7-8a09-765794883524 --- MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 109 ++++++++++++++++++++++++--- MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h | 8 +- 2 files changed, 105 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c index 1a8d83ecaf..74f53c58b3 100644 --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c @@ -2568,6 +2568,7 @@ ScsiDiskAsyncReadSectors ( UINT64 Timeout; SCSI_BLKIO2_REQUEST *BlkIo2Req; EFI_STATUS Status; + EFI_TPL OldTpl; if ((Token == NULL) || (Token->Event == NULL)) { return EFI_INVALID_PARAMETER; @@ -2579,7 +2580,11 @@ ScsiDiskAsyncReadSectors ( } BlkIo2Req->Token = Token; + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&ScsiDiskDevice->BlkIo2Queue, &BlkIo2Req->Link); + gBS->RestoreTPL (OldTpl); + InitializeListHead (&BlkIo2Req->ScsiRWQueue); Status = EFI_SUCCESS; @@ -2686,6 +2691,7 @@ ScsiDiskAsyncReadSectors ( } } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); if (IsListEmpty (&BlkIo2Req->ScsiRWQueue)) { // // Free the SCSI_BLKIO2_REQUEST structure only when there is no other @@ -2694,19 +2700,25 @@ ScsiDiskAsyncReadSectors ( // RemoveEntryList (&BlkIo2Req->Link); FreePool (BlkIo2Req); + BlkIo2Req = NULL; + gBS->RestoreTPL (OldTpl); // // It is safe to return error status to the caller, since there is no // previous SCSI sub-task executing. // - return EFI_DEVICE_ERROR; + Status = EFI_DEVICE_ERROR; + goto Done; } else { + gBS->RestoreTPL (OldTpl); + // // There are previous SCSI commands still running, EFI_SUCCESS should // be returned to make sure that the caller does not free resources // still using by these SCSI commands. // - return EFI_SUCCESS; + Status = EFI_SUCCESS; + goto Done; } } @@ -2720,7 +2732,24 @@ ScsiDiskAsyncReadSectors ( BlocksRemaining -= SectorCount; } - return EFI_SUCCESS; + Status = EFI_SUCCESS; + +Done: + if (BlkIo2Req != NULL) { + BlkIo2Req->LastScsiRW = TRUE; + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); + if (IsListEmpty (&BlkIo2Req->ScsiRWQueue)) { + RemoveEntryList (&BlkIo2Req->Link); + FreePool (BlkIo2Req); + BlkIo2Req = NULL; + + gBS->SignalEvent (Token->Event); + } + gBS->RestoreTPL (OldTpl); + } + + return Status; } /** @@ -2756,6 +2785,7 @@ ScsiDiskAsyncWriteSectors ( UINT64 Timeout; SCSI_BLKIO2_REQUEST *BlkIo2Req; EFI_STATUS Status; + EFI_TPL OldTpl; if ((Token == NULL) || (Token->Event == NULL)) { return EFI_INVALID_PARAMETER; @@ -2767,7 +2797,11 @@ ScsiDiskAsyncWriteSectors ( } BlkIo2Req->Token = Token; + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&ScsiDiskDevice->BlkIo2Queue, &BlkIo2Req->Link); + gBS->RestoreTPL (OldTpl); + InitializeListHead (&BlkIo2Req->ScsiRWQueue); Status = EFI_SUCCESS; @@ -2874,6 +2908,7 @@ ScsiDiskAsyncWriteSectors ( } } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); if (IsListEmpty (&BlkIo2Req->ScsiRWQueue)) { // // Free the SCSI_BLKIO2_REQUEST structure only when there is no other @@ -2882,19 +2917,25 @@ ScsiDiskAsyncWriteSectors ( // RemoveEntryList (&BlkIo2Req->Link); FreePool (BlkIo2Req); + BlkIo2Req = NULL; + gBS->RestoreTPL (OldTpl); // // It is safe to return error status to the caller, since there is no // previous SCSI sub-task executing. // - return EFI_DEVICE_ERROR; + Status = EFI_DEVICE_ERROR; + goto Done; } else { + gBS->RestoreTPL (OldTpl); + // // There are previous SCSI commands still running, EFI_SUCCESS should // be returned to make sure that the caller does not free resources // still using by these SCSI commands. // - return EFI_SUCCESS; + Status = EFI_SUCCESS; + goto Done; } } @@ -2908,7 +2949,24 @@ ScsiDiskAsyncWriteSectors ( BlocksRemaining -= SectorCount; } - return EFI_SUCCESS; + Status = EFI_SUCCESS; + +Done: + if (BlkIo2Req != NULL) { + BlkIo2Req->LastScsiRW = TRUE; + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); + if (IsListEmpty (&BlkIo2Req->ScsiRWQueue)) { + RemoveEntryList (&BlkIo2Req->Link); + FreePool (BlkIo2Req); + BlkIo2Req = NULL; + + gBS->SignalEvent (Token->Event); + } + gBS->RestoreTPL (OldTpl); + } + + return Status; } @@ -3672,7 +3730,8 @@ Retry: Exit: RemoveEntryList (&Request->Link); - if (IsListEmpty (&Request->BlkIo2Req->ScsiRWQueue)) { + if ((IsListEmpty (&Request->BlkIo2Req->ScsiRWQueue)) && + (Request->BlkIo2Req->LastScsiRW)) { // // The last SCSI R/W command of a BlockIo2 request completes // @@ -3722,6 +3781,7 @@ ScsiDiskAsyncRead10 ( EFI_STATUS Status; SCSI_ASYNC_RW_REQUEST *Request; EFI_EVENT AsyncIoEvent; + EFI_TPL OldTpl; AsyncIoEvent = NULL; @@ -3729,7 +3789,10 @@ ScsiDiskAsyncRead10 ( if (Request == NULL) { return EFI_OUT_OF_RESOURCES; } + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&BlkIo2Req->ScsiRWQueue, &Request->Link); + gBS->RestoreTPL (OldTpl); Request->SenseDataLength = (UINT8) (6 * sizeof (EFI_SCSI_SENSE_DATA)); Request->SenseData = AllocateZeroPool (Request->SenseDataLength); @@ -3752,7 +3815,7 @@ ScsiDiskAsyncRead10 ( // Status = gBS->CreateEvent ( EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, + TPL_NOTIFY, ScsiDiskNotify, Request, &AsyncIoEvent @@ -3790,7 +3853,10 @@ ErrorExit: FreePool (Request->SenseData); } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); RemoveEntryList (&Request->Link); + gBS->RestoreTPL (OldTpl); + FreePool (Request); } @@ -3834,6 +3900,7 @@ ScsiDiskAsyncWrite10 ( EFI_STATUS Status; SCSI_ASYNC_RW_REQUEST *Request; EFI_EVENT AsyncIoEvent; + EFI_TPL OldTpl; AsyncIoEvent = NULL; @@ -3841,7 +3908,10 @@ ScsiDiskAsyncWrite10 ( if (Request == NULL) { return EFI_OUT_OF_RESOURCES; } + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&BlkIo2Req->ScsiRWQueue, &Request->Link); + gBS->RestoreTPL (OldTpl); Request->SenseDataLength = (UINT8) (6 * sizeof (EFI_SCSI_SENSE_DATA)); Request->SenseData = AllocateZeroPool (Request->SenseDataLength); @@ -3864,7 +3934,7 @@ ScsiDiskAsyncWrite10 ( // Status = gBS->CreateEvent ( EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, + TPL_NOTIFY, ScsiDiskNotify, Request, &AsyncIoEvent @@ -3902,7 +3972,10 @@ ErrorExit: FreePool (Request->SenseData); } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); RemoveEntryList (&Request->Link); + gBS->RestoreTPL (OldTpl); + FreePool (Request); } @@ -3946,6 +4019,7 @@ ScsiDiskAsyncRead16 ( EFI_STATUS Status; SCSI_ASYNC_RW_REQUEST *Request; EFI_EVENT AsyncIoEvent; + EFI_TPL OldTpl; AsyncIoEvent = NULL; @@ -3953,7 +4027,10 @@ ScsiDiskAsyncRead16 ( if (Request == NULL) { return EFI_OUT_OF_RESOURCES; } + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&BlkIo2Req->ScsiRWQueue, &Request->Link); + gBS->RestoreTPL (OldTpl); Request->SenseDataLength = (UINT8) (6 * sizeof (EFI_SCSI_SENSE_DATA)); Request->SenseData = AllocateZeroPool (Request->SenseDataLength); @@ -3976,7 +4053,7 @@ ScsiDiskAsyncRead16 ( // Status = gBS->CreateEvent ( EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, + TPL_NOTIFY, ScsiDiskNotify, Request, &AsyncIoEvent @@ -4014,7 +4091,10 @@ ErrorExit: FreePool (Request->SenseData); } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); RemoveEntryList (&Request->Link); + gBS->RestoreTPL (OldTpl); + FreePool (Request); } @@ -4058,6 +4138,7 @@ ScsiDiskAsyncWrite16 ( EFI_STATUS Status; SCSI_ASYNC_RW_REQUEST *Request; EFI_EVENT AsyncIoEvent; + EFI_TPL OldTpl; AsyncIoEvent = NULL; @@ -4065,7 +4146,10 @@ ScsiDiskAsyncWrite16 ( if (Request == NULL) { return EFI_OUT_OF_RESOURCES; } + + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); InsertTailList (&BlkIo2Req->ScsiRWQueue, &Request->Link); + gBS->RestoreTPL (OldTpl); Request->SenseDataLength = (UINT8) (6 * sizeof (EFI_SCSI_SENSE_DATA)); Request->SenseData = AllocateZeroPool (Request->SenseDataLength); @@ -4088,7 +4172,7 @@ ScsiDiskAsyncWrite16 ( // Status = gBS->CreateEvent ( EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, + TPL_NOTIFY, ScsiDiskNotify, Request, &AsyncIoEvent @@ -4126,7 +4210,10 @@ ErrorExit: FreePool (Request->SenseData); } + OldTpl = gBS->RaiseTPL (TPL_NOTIFY); RemoveEntryList (&Request->Link); + gBS->RestoreTPL (OldTpl); + FreePool (Request); } diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h index 7a287d3bb3..2406df51b8 100644 --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.h @@ -98,7 +98,13 @@ typedef struct { typedef struct { EFI_BLOCK_IO2_TOKEN *Token; // - // The queue for Scsi Read/Write requests of a BlockIo2 + // The flag indicates if the last Scsi Read/Write sub-task for a BlockIo2 + // request is sent to device + // + BOOLEAN LastScsiRW; + + // + // The queue for Scsi Read/Write sub-tasks of a BlockIo2 request // LIST_ENTRY ScsiRWQueue; -- cgit v1.2.3