Wake blockdev queue in scsi_internal_device_unblock() for SDEV_RUNNING

With the old scsi_lib code, scsi_internal_device_unblock() will return an error at this point because the sdev state is already SDEV_RUNNING. This means we skip the call to blk_start_queue() and never actually start executing commands again.

Fix this by still going ahead and finishing scsi_internal_device_unblock() even if the sdev state is already SDEV_RUNNING.

From: Roland Dreier <[email protected]>

If a SCSI device's old state is already SDEV_RUNNING and we're moving to the same SDEV_RUNNING state, still wake the blockdev queue in scsi_internal_device_unblock(). This fixes a case where we silently hang SCSI commands forever during device discovery. One way this can happen is when mpt2sas is discovering a reasonably big SAS topology, and the sd driver has queued up a bunch of sd_probe_async() instances that are queueing SCSI commands to various devices.

If at the same time a SAS fabric event goes to the HBA, what can happen is the following:

- mpt2sas calls _scsih_block_io_all_device() -> scsi_internal_device_block(sdev)

(In response to some HBA firmware event like MPI2_EVENT_SAS_BROADCAST_PRIMITIVE) Now sdev state is SDEV_BLOCK and blockdev queue has QUEUE_FLAG_STOPPED set.

- Someone like scsi_add_lun() calls scsi_device_set_state(sdev, SDEV_RUNNING)

(SCSI bus scanning runs asynchronously to firmware event handling) Now sdev state is SDEV_RUNNING but blockdev queue still has QUEUE_FLAG_STOPPED set

- mpt2sas calls _scsih_ublock_io_all_device() -> scsi_internal_device_unblock(sdev, SDEV_RUNNING)

(Finishes handling the firmware event)

With the old scsi_lib code, scsi_internal_device_unblock() will return an error at this point because the sdev state is already SDEV_RUNNING. This means we skip the call to blk_start_queue() and never actually start executing commands again.

Fix this by still going ahead and finishing scsi_internal_device_unblock() even if the sdev state is already SDEV_RUNNING.

Signed-off-by Roland Dreier <[email protected]>

---
 drivers/scsi/scsi_lib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/scsi/scsi_lib.c +3 -1

index 765398c..75108ea 100644
@@ -2495,7 +2495,9 @@ scsi_internal_device_unblock(struct scsi_device *sdev,
 		else
 			sdev->sdev_state = SDEV_CREATED;
 	} else if (sdev->sdev_state != SDEV_CANCEL &&
-		 sdev->sdev_state != SDEV_OFFLINE)
+		   sdev->sdev_state != SDEV_OFFLINE &&
+		   (sdev->sdev_state != SDEV_RUNNING ||
+		    new_state != SDEV_RUNNING))
 		return -EINVAL;
 
 	spin_lock_irqsave(q->queue_lock, flags);

Quoted 5 lines

If at the same time a SAS fabric event goes to the HBA, what can happen is the following:

- mpt2sas calls _scsih_block_io_all_device() -> scsi_internal_device_block(sdev)

Quoted 7 lines

(In response to some HBA firmware event like MPI2_EVENT_SAS_BROADCAST_PRIMITIVE) Now sdev state is SDEV_BLOCK and blockdev queue has QUEUE_FLAG_STOPPED set.

- Someone like scsi_add_lun() calls scsi_device_set_state(sdev, SDEV_RUNNING)

(SCSI bus scanning runs asynchronously to firmware event handling) Now sdev state is SDEV_RUNNING but blockdev queue still has QUEUE_FLAG_STOPPED set

Is this a valid state change? Should it be failed in scsi_device_set_state when we try to go from SDEV_BLOCKED -> SDEV_RUNNING? I am not sure if it make senses to ever have a device in SDEV_RUNNING but have the stopped queue bit set.

It used to be that scsi_internal_device_unblock called scsi_device_set_state so the transition from SDEV_BLOCKED to SDEV_RUNNUNG had to be a valid transition. scsi_internal_device_unblock then started the queue. I am not sure if the sequence you were hitting was actually a transition that was intended or just worked by accident.

Should we be failing the above call to scsi_device_set_state, and then the call to scsi_internal_device_unblock would work as expected.