On 06/23/13 23:35, Mike Christie wrote:
On 06/12/2013 07:52 AM, Bart Van Assche wrote:
SCSI devices are added to the shost->__devices list from inside
scsi_alloc_sdev(). 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 or SDEV_BLOCKED. If this happens then
the SCSI device has not yet been added to sysfs (is_visible == 0).
Make sure that if this happens these devices are transitioned
into state SDEV_DEL. This avoids that __scsi_remove_device()
gets invoked a second time by scsi_forget_host().

Signed-off-by: Bart Van Assche <bvanass...@acm.org>
Cc: James Bottomley <jbottom...@parallels.com>
Cc: Mike Christie <micha...@cs.wisc.edu>
Cc: Hannes Reinecke <h...@suse.de>
Cc: Tejun Heo <t...@kernel.org>
---
  drivers/scsi/scsi_lib.c   |    2 ++
  drivers/scsi/scsi_sysfs.c |    7 ++++---
  2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 03d4165..d57b5e1 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2175,6 +2175,8 @@ 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_BLOCK:
+               case SDEV_CREATED_BLOCK:
                        break;
                default:
                        goto illegal;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 34f1b39..f869ef85 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -956,8 +956,9 @@ void __scsi_remove_device(struct scsi_device *sdev)
        unsigned long flags;

        if (sdev->is_visible) {
-               if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
-                       return;
+               WARN(scsi_device_set_state(sdev, SDEV_CANCEL) != 0,
+                    "%s: unexpected state %d\n", dev_name(&sdev->sdev_gendev),
+                    sdev->sdev_state);

                bsg_unregister_queue(sdev->request_queue);
                device_unregister(&sdev->sdev_dev);
@@ -971,7 +972,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
         * scsi_run_queue() invocations have finished before tearing down the
         * device.
         */
-       scsi_device_set_state(sdev, SDEV_DEL);
+       WARN_ON_ONCE(scsi_device_set_state(sdev, SDEV_DEL) != 0);
        blk_cleanup_queue(sdev->request_queue);
        cancel_work_sync(&sdev->requeue_work);

The Addition to scsi_device_set_state looks ok. I was not sure why we
need the new WARNs though. Do we still think there are other cases?

Hello Mike,

The new WARN statements helped me while testing and debugging this code. These statements might also be helpful if anyone would ever add more SCSI device states. I can leave these out if you want.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to