scsi_dh: add a common helper to get a scsi_device from a request_queue

And cleanup the various messy opencoded versions of this.  Note that this
moves the sdev_state checks outside the queue_lock coverage, but as
we don't hold the lock over the activation they are only advisory anyway.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: James Bottomley <JBottomley@Odin.com>

1 files changed, 46 insertions, 53 deletions

diff --git a/drivers/scsi/scsi_dh.c b/drivers/scsi/scsi_dh.c
index 33c1148ff3b5..2e1db5738ae1 100644
--- a/drivers/scsi/scsi_dh.c
+++ b/drivers/scsi/scsi_dh.c
@@ -283,6 +283,20 @@ int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh)
 }
 EXPORT_SYMBOL_GPL(scsi_unregister_device_handler);
 
+static struct scsi_device *get_sdev_from_queue(struct request_queue *q)
+{
+        struct scsi_device *sdev;
+        unsigned long flags;
+
+        spin_lock_irqsave(q->queue_lock, flags);
+        sdev = q->queuedata;
+        if (!sdev || !get_device(&sdev->sdev_gendev))
+                sdev = NULL;
+        spin_unlock_irqrestore(q->queue_lock, flags);
+
+        return sdev;
+}
+
 /*
  * scsi_dh_activate - activate the path associated with the scsi_device
  *      corresponding to the given request queue.
@@ -298,41 +312,37 @@ EXPORT_SYMBOL_GPL(scsi_unregister_device_handler);
  */
 int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 {
-        int err = 0;
-        unsigned long flags;
         struct scsi_device *sdev;
-        struct device *dev = NULL;
+        int err = SCSI_DH_NOSYS;
 
-        spin_lock_irqsave(q->queue_lock, flags);
-        sdev = q->queuedata;
+        sdev = get_sdev_from_queue(q);
         if (!sdev) {
-                spin_unlock_irqrestore(q->queue_lock, flags);
-                err = SCSI_DH_NOSYS;
                 if (fn)
                         fn(data, err);
                 return err;
         }
 
-        dev = get_device(&sdev->sdev_gendev);
-        if (!sdev->handler || !dev ||
-            sdev->sdev_state == SDEV_CANCEL ||
+        if (!sdev->handler)
+                goto out_fn;
+        if (sdev->sdev_state == SDEV_CANCEL ||
             sdev->sdev_state == SDEV_DEL)
-                err = SCSI_DH_NOSYS;
-        if (sdev->sdev_state == SDEV_OFFLINE)
-                err = SCSI_DH_DEV_OFFLINED;
-        spin_unlock_irqrestore(q->queue_lock, flags);
+                goto out_fn;
 
-        if (err) {
-                if (fn)
-                        fn(data, err);
-                goto out;
-        }
+        err = SCSI_DH_DEV_OFFLINED;
+        if (sdev->sdev_state == SDEV_OFFLINE)
+                goto out_fn;
 
         if (sdev->handler->activate)
                 err = sdev->handler->activate(sdev, fn, data);
-out:
-        put_device(dev);
+
+out_put_device:
+        put_device(&sdev->sdev_gendev);
         return err;
+
+out_fn:
+        if (fn)
+                fn(data, err);
+        goto out_put_device;
 }
 EXPORT_SYMBOL_GPL(scsi_dh_activate);
 
@@ -348,21 +358,15 @@ EXPORT_SYMBOL_GPL(scsi_dh_activate);
  */
 int scsi_dh_set_params(struct request_queue *q, const char *params)
 {
-        int err = -SCSI_DH_NOSYS;
-        unsigned long flags;
         struct scsi_device *sdev;
+        int err = -SCSI_DH_NOSYS;
 
-        spin_lock_irqsave(q->queue_lock, flags);
-        sdev = q->queuedata;
-        if (sdev->handler &&
-            sdev->handler->set_params &&
-            get_device(&sdev->sdev_gendev))
-                err = 0;
-        spin_unlock_irqrestore(q->queue_lock, flags);
-
-        if (err)
+        sdev = get_sdev_from_queue(q);
+        if (!sdev)
                 return err;
-        err = sdev->handler->set_params(sdev, params);
+
+        if (sdev->handler && sdev->handler->set_params)
+                err = sdev->handler->set_params(sdev, params);
         put_device(&sdev->sdev_gendev);
         return err;
 }
@@ -376,23 +380,19 @@ EXPORT_SYMBOL_GPL(scsi_dh_set_params);
  */
 int scsi_dh_attach(struct request_queue *q, const char *name)
 {
-        unsigned long flags;
         struct scsi_device *sdev;
         struct scsi_device_handler *scsi_dh;
         int err = 0;
 
-        scsi_dh = scsi_dh_lookup(name);
-        if (!scsi_dh)
-                return -EINVAL;
-
-        spin_lock_irqsave(q->queue_lock, flags);
-        sdev = q->queuedata;
-        if (!sdev || !get_device(&sdev->sdev_gendev))
-                err = -ENODEV;
-        spin_unlock_irqrestore(q->queue_lock, flags);
+        sdev = get_sdev_from_queue(q);
+        if (!sdev)
+                return -ENODEV;
 
-        if (err)
-                return err;
+        scsi_dh = scsi_dh_lookup(name);
+        if (!scsi_dh) {
+                err = -EINVAL;
+                goto out_put_device;
+        }
 
         if (sdev->handler) {
                 if (sdev->handler != scsi_dh)
@@ -419,22 +419,15 @@ EXPORT_SYMBOL_GPL(scsi_dh_attach);
  */
 const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
 {
-        unsigned long flags;
         struct scsi_device *sdev;
         const char *handler_name = NULL;
 
-        spin_lock_irqsave(q->queue_lock, flags);
-        sdev = q->queuedata;
-        if (!sdev || !get_device(&sdev->sdev_gendev))
-                sdev = NULL;
-        spin_unlock_irqrestore(q->queue_lock, flags);
-
+        sdev = get_sdev_from_queue(q);
         if (!sdev)
                 return NULL;
 
         if (sdev->handler)
                 handler_name = kstrdup(sdev->handler->name, gfp);
-
         put_device(&sdev->sdev_gendev);
         return handler_name;
 }