SCSI device removal fixes

Fix a few issues related to SCSI device removal:
- Fix a race between starved list processing and device removal that can trigger a kernel oops.
- Avoid that __scsi_remove_device() is called twice for the same SCSI device, which also can cause a kernel oops.
- Restrict the SCSI device state changes allowed via sysfs.
- Avoid re-enabling I/O after the transport layer became offline.

Changes compared to v12:
- Clarified the description of the patch for handling a transport layer failure during LUN scanning: mentioned that this patch was developed after analyzing the cause of a kernel oops triggered by asynchronous LUN scanning.
- Restored the previous version of the patch for restricting sysfs SCSI device state changes, namely the version that only disallows changing the device state into "cancel" or "deleted".
- Added a comment in patch 4/4.
- Left out a patch that was the result of source reading and also an intermediate patch that is no longer needed in this series.

Changes compared to v11:
- Left out a patch that was not a device removal bug fix.
- Left out the patches about which there is not yet an agreement.

Changes compared to v10:
- Rebased and retested on top of Linux kernel v3.10-rc5.

Changes compared to v9:
- Changed one WARN_ON() statement into a WARN() statement.

Changes compared to v8:
- Addressed the feedback from Joe Lawrence - dropped the patch that makes scsi_remove_host() wait until the last sdev user is gone.
- Eliminated Scsi_Host.tmf_in_progress since it duplicates state information available in Scsi_Host.eh_active.
- Added a patch to avoid reenabling I/O after the transport layer became offline.

Changes compared to v7:
- Addressed the review comments posted by Hannes Reinecke and Rolf Eike Beer.
- Modified patch "Make scsi_remove_host() wait until error handling finished" such that it is also safe for SCSI timeout values below the maximum LLD response time by modifying scsi_send_eh_cmnd() such that it does not invoke any LLD code after scsi_remove_host() started.
- Added a patch to save and restore the host_scribble field.
- Refined / clarified several patch descriptions.
- Rebased and retested on top of kernel v3.8-rc6.

Changes compared to v6:
- Dropped the first six patches since Jens queued these for 3.8.
- Added patch to avoid that __scsi_remove_device() is invoked twice.
- Restore error recovery in the SHOST_CANCEL state.

Changes compared to v5:
- Avoid that block layer work can be scheduled on a dead queue.
- Do not invoke any SCSI LLD callback after scsi_remove_host() finished.
- Stop error handling as soon as scsi_remove_host() started.
- Remove the unused function bsg_goose_queue().
- Avoid that scsi_device_set_state() triggers a race condition.

Changes compared to v4:
- Moved queue_flag_set(QUEUE_FLAG_DEAD, q) from blk_drain_queue() into blk_cleanup_queue().
- Declared the new __blk_run_queue_uncond() function inline. Checked in the generated assembler code that this function is really inlined in __blk_run_queue().
- Elaborated several patch descriptions.
- Added sparse annotations to scsi_request_fn().
- Split several patches.

Changes compared to v3:
- Fixed a race condition by setting QUEUE_FLAG_DEAD earlier.
- Added a patch for fixing a race between starved list processing and device removal to this series.

Changes compared to v2:
- Split second patch into two patches.
- Refined patch descriptions.

Changes compared to v1:
- Included a patch to rename QUEUE_FLAG_DEAD.
- Refined the descriptions of the __blk_run_queue_uncond() and blk_cleanup_queue() functions.

From: James Bottomley <[email protected]>

scsi_run_queue() examines all SCSI devices that are present on the starved list. Since scsi_run_queue() unlocks the SCSI host lock a SCSI device can get removed after it has been removed from the starved list and before its queue is run. Protect against that race condition by holding a reference on the queue while running it.

Reference: http://lkml.org/lkml/2012/8/2/96
Cc: Tejun Heo <[email protected]>
Cc: Mike Christie <[email protected]>
Cc: Hannes Reinecke <[email protected]>
Cc: <[email protected]>

---
 drivers/scsi/scsi_lib.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

index 86d5220..df8bd5a 100644
@@ -434,6 +434,8 @@ static void scsi_run_queue(struct request_queue *q)
 	list_splice_init(&shost->starved_list, &starved_list);
 
 	while (!list_empty(&starved_list)) {
+		struct request_queue *slq;
+
 		/*
 		 * As long as shost is accepting commands and we have
 		 * starved queues, call blk_run_queue. scsi_request_fn

@@ -456,11 +458,25 @@ static void scsi_run_queue(struct request_queue *q)
 			continue;
 		}
 
-		spin_unlock(shost->host_lock);
-		spin_lock(sdev->request_queue->queue_lock);
-		__blk_run_queue(sdev->request_queue);
-		spin_unlock(sdev->request_queue->queue_lock);
-		spin_lock(shost->host_lock);
+		/*
+		 * Once we drop the host lock, a racing scsi_remove_device()
+		 * call may remove the sdev from the starved list and destroy
+		 * it and the queue.  Mitigate by taking a reference to the
+		 * queue and never touching the sdev again after we drop the
+		 * host lock.  Note: if __scsi_remove_device() invokes
+		 * blk_cleanup_queue() before the queue is run from this
+		 * function then blk_run_queue() will return immediately since
+		 * blk_cleanup_queue() marks the queue with QUEUE_FLAG_DYING.
+		 */
+		slq = sdev->request_queue;
+		if (!blk_get_queue(slq))
+			continue;
+		spin_unlock_irqrestore(shost->host_lock, flags);
+
+		blk_run_queue(slq);
+		blk_put_queue(slq);
+
+		spin_lock_irqsave(shost->host_lock, flags);
 	}
 	/* put any unprocessed entries back */
 	list_splice(&starved_list, &shost->starved_list);

1.7.10.4

If something goes wrong during LUN scanning, e.g. a transport layer failure occurs, then __scsi_remove_device() can get invoked by the LUN scanning code for a SCSI device in state SDEV_CREATED_BLOCK and before the SCSI device has been added to sysfs (is_visible == 0). Make sure that even in this case the transition into state SDEV_DEL occurs. This avoids that __scsi_remove_device() can get invoked a second time by scsi_forget_host() if this last function is invoked from another thread than the thread that performs LUN scanning.

---
 drivers/scsi/scsi_lib.c |    1 +
 1 file changed, 1 insertion(+)

index df8bd5a..124392f 100644
@@ -2193,6 +2193,7 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_OFFLINE:
 		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_CANCEL:
+		case SDEV_CREATED_BLOCK:
 			break;
 		default:
 			goto illegal;

1.7.10.4

Disallow the SDEV_TRANSPORT_OFFLINE to SDEV_CANCEL transition such that no I/O is sent to devices for which the transport is offline. Notes:
- Functions like sd_shutdown() use scsi_execute_req() and hence set the REQ_PREEMPT flag. Such requests are passed to the LLD queuecommand callback in the SDEV_CANCEL state.
- This patch does not affect Fibre Channel LLD drivers since these drivers invoke fc_remote_port_chkready() before submitting a SCSI request to the HBA. That prevents a timeout to occur in state SDEV_CANCEL if the transport is offline.

---
 drivers/scsi/scsi_lib.c   |    1 -
 drivers/scsi/scsi_sysfs.c |    9 ++++++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

index 124392f..a0fb56b 100644
@@ -2178,7 +2178,6 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state)
 		case SDEV_RUNNING:
 		case SDEV_QUIESCE:
 		case SDEV_OFFLINE:
-		case SDEV_TRANSPORT_OFFLINE:
 		case SDEV_BLOCK:
 			break;
 		default:

index 931a7d9..1711617 100644
@@ -955,7 +955,14 @@ void __scsi_remove_device(struct scsi_device *sdev)
 	struct device *dev = &sdev->sdev_gendev;
 
 	if (sdev->is_visible) {
-		if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
+		/*
+		 * The transition from SDEV_TRANSPORT_OFFLINE into
+		 * SDEV_CANCEL is not allowed since this transition would
+		 * reenable I/O. However, if the device state was already
+		 * SDEV_TRANSPORT_OFFLINE, proceed with device removal.
+		 */
+		if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0 &&
+		    sdev->sdev_state != SDEV_TRANSPORT_OFFLINE)
 			return;
 
 		bsg_unregister_queue(sdev->request_queue);

1.7.10.4

Changing the state of a SCSI device via sysfs into "cancel" or "deleted" prevents removal of these devices by scsi_remove_host(). Hence do not allow this.

---
 drivers/scsi/scsi_sysfs.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

index 1711617..292df85 100644
@@ -605,10 +605,8 @@ store_state_field(struct device *dev, struct device_attribute *attr,
 			break;
 		}
 	}
-	if (!state)
-		return -EINVAL;
-
-	if (scsi_device_set_state(sdev, state))
+	if (state == 0 || state == SDEV_CANCEL || state == SDEV_DEL ||
+	    scsi_device_set_state(sdev, state))
 		return -EINVAL;
 	return count;
 }

1.7.10.4