integrate scsi_dh better into the scsi core

integrate scsi_dh better into the scsi core

[Christoph Hellwig]

This series ties scsi_dh deeper into the scsi core, and fixes all kinds
of issues in it, most importantly the race between using and detaching
device handlers.

[PATCH 1/9] dm-mpath: check kstrdup return value in parse_hw_handler

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-mpath.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 6395347..01e5f8e 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -732,6 +732,9 @@ static int parse_hw_handler(struct dm_arg_set *as, struct multipath *m)
 		return 0;
 
 	m->hw_handler_name = kstrdup(dm_shift_arg(as), GFP_KERNEL);
+	if (!m->hw_handler_name)
+		return -ENOMEM;
+
 	if (!try_then_request_module(scsi_dh_handler_exist(m->hw_handler_name),
 				     "scsi_dh_%s", m->hw_handler_name)) {
 		ti->error = "unknown hardware handler type";
-- 
1.9.1

Re: [PATCH 1/9] dm-mpath: check kstrdup return value in parse_hw_handler

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/dm-mpath.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 6395347..01e5f8e 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -732,6 +732,9 @@ static int parse_hw_handler(struct dm_arg_set *as, struct multipath *m)
>  		return 0;
>  
>  	m->hw_handler_name = kstrdup(dm_shift_arg(as), GFP_KERNEL);
> +	if (!m->hw_handler_name)
> +		return -ENOMEM;
> +
>  	if (!try_then_request_module(scsi_dh_handler_exist(m->hw_handler_name),
>  				     "scsi_dh_%s", m->hw_handler_name)) {
>  		ti->error = "unknown hardware handler type";
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/9] dm-mpath: check kstrdup return value in parse_hw_handler

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Mike Snitzer <snitzer@redhat.com>

Re: [PATCH 1/9] dm-mpath: check kstrdup return value in parse_hw_handler

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:



-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/9] dm-mpath: check kstrdup return value in parse_hw_handler

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Christoph Hellwig]

While allowing dm-mpath to attach device handlers is a functionality we need
for backwards compatibility reason there is no reason to reference count
them and detach them if dm-mpath stops using the device for some reason.

If the device handler works for the given device it can just stay attached.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-mpath.c                 |  13 -----
 drivers/scsi/device_handler/scsi_dh.c | 101 +++++++++-------------------------
 include/scsi/scsi_device.h            |   1 -
 include/scsi/scsi_dh.h                |   5 --
 4 files changed, 27 insertions(+), 93 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 01e5f8e..9fb91ca 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -159,12 +159,9 @@ static struct priority_group *alloc_priority_group(void)
 static void free_pgpaths(struct list_head *pgpaths, struct dm_target *ti)
 {
 	struct pgpath *pgpath, *tmp;
-	struct multipath *m = ti->private;
 
 	list_for_each_entry_safe(pgpath, tmp, pgpaths, list) {
 		list_del(&pgpath->list);
-		if (m->hw_handler_name)
-			scsi_dh_detach(bdev_get_queue(pgpath->path.dev->bdev));
 		dm_put_device(ti, pgpath->path.dev);
 		free_pgpath(pgpath);
 	}
@@ -602,15 +599,6 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 		 * already-attached handler.
 		 */
 		r = scsi_dh_attach(q, m->hw_handler_name);
-		if (r == -EBUSY) {
-			/*
-			 * Already attached to different hw_handler:
-			 * try to reattach with correct one.
-			 */
-			scsi_dh_detach(q);
-			r = scsi_dh_attach(q, m->hw_handler_name);
-		}
-
 		if (r < 0) {
 			ti->error = "error attaching hardware handler";
 			dm_put_device(ti, p->path.dev);
@@ -622,7 +610,6 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 			if (r < 0) {
 				ti->error = "unable to set hardware "
 							"handler parameters";
-				scsi_dh_detach(q);
 				dm_put_device(ti, p->path.dev);
 				goto bad;
 			}
diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 1efebc9..93fd7d4 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -100,14 +100,6 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev,
 {
 	struct scsi_dh_data *d;
 
-	if (sdev->scsi_dh_data) {
-		if (sdev->scsi_dh_data->scsi_dh != scsi_dh)
-			return -EBUSY;
-
-		kref_get(&sdev->scsi_dh_data->kref);
-		return 0;
-	}
-
 	if (!try_module_get(scsi_dh->module))
 		return -EINVAL;
 
@@ -120,7 +112,6 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev,
 	}
 
 	d->scsi_dh = scsi_dh;
-	kref_init(&d->kref);
 	d->sdev = sdev;
 
 	spin_lock_irq(sdev->request_queue->queue_lock);
@@ -129,12 +120,14 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev,
 	return 0;
 }
 
-static void __detach_handler (struct kref *kref)
+/*
+ * scsi_dh_handler_detach - Detach a device handler from a device
+ * @sdev - SCSI device the device handler should be detached from
+ */
+static void scsi_dh_handler_detach(struct scsi_device *sdev)
 {
-	struct scsi_dh_data *scsi_dh_data =
-		container_of(kref, struct scsi_dh_data, kref);
+	struct scsi_dh_data *scsi_dh_data = sdev->scsi_dh_data;
 	struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
-	struct scsi_device *sdev = scsi_dh_data->sdev;
 
 	scsi_dh->detach(sdev);
 
@@ -147,30 +140,6 @@ static void __detach_handler (struct kref *kref)
 }
 
 /*
- * scsi_dh_handler_detach - Detach a device handler from a device
- * @sdev - SCSI device the device handler should be detached from
- * @scsi_dh - Device handler to be detached
- *
- * Detach from a device handler. If a device handler is specified,
- * only detach if the currently attached handler matches @scsi_dh.
- */
-static void scsi_dh_handler_detach(struct scsi_device *sdev,
-				   struct scsi_device_handler *scsi_dh)
-{
-	if (!sdev->scsi_dh_data)
-		return;
-
-	if (scsi_dh && scsi_dh != sdev->scsi_dh_data->scsi_dh)
-		return;
-
-	if (!scsi_dh)
-		scsi_dh = sdev->scsi_dh_data->scsi_dh;
-
-	if (scsi_dh)
-		kref_put(&sdev->scsi_dh_data->kref, __detach_handler);
-}
-
-/*
  * Functions for sysfs attribute 'dh_state'
  */
 static ssize_t
@@ -198,7 +167,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 			/*
 			 * Detach from a device handler
 			 */
-			scsi_dh_handler_detach(sdev, scsi_dh);
+			scsi_dh_handler_detach(sdev);
 			err = 0;
 		} else if (!strncmp(buf, "activate", 8)) {
 			/*
@@ -290,7 +259,8 @@ static int scsi_dh_notifier(struct notifier_block *nb,
 			err = scsi_dh_handler_attach(sdev, devinfo);
 	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
 		device_remove_file(dev, &scsi_dh_state_attr);
-		scsi_dh_handler_detach(sdev, NULL);
+		if (sdev->scsi_dh_data)
+			scsi_dh_handler_detach(sdev);
 	}
 	return err;
 }
@@ -335,7 +305,8 @@ static int scsi_dh_notifier_remove(struct device *dev, void *data)
 
 	sdev = to_scsi_device(dev);
 
-	scsi_dh_handler_detach(sdev, scsi_dh);
+	if (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh == scsi_dh)
+		scsi_dh_handler_detach(sdev);
 
 	put_device(dev);
 
@@ -517,45 +488,27 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 		err = -ENODEV;
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
-	if (!err) {
-		err = scsi_dh_handler_attach(sdev, scsi_dh);
-		put_device(&sdev->sdev_gendev);
-	}
-	return err;
-}
-EXPORT_SYMBOL_GPL(scsi_dh_attach);
-
-/*
- * scsi_dh_detach - Detach device handler
- * @q - Request queue that is associated with the scsi_device
- *      the handler should be detached from
- *
- * This function will detach the device handler only
- * if the sdev is not part of the internal list, ie
- * if it has been attached manually.
- */
-void scsi_dh_detach(struct request_queue *q)
-{
-	unsigned long flags;
-	struct scsi_device *sdev;
-	struct scsi_device_handler *scsi_dh = 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);
-
-	if (!sdev)
-		return;
+	if (err)
+		return err;
 
 	if (sdev->scsi_dh_data) {
-		scsi_dh = sdev->scsi_dh_data->scsi_dh;
-		scsi_dh_handler_detach(sdev, scsi_dh);
+		if (sdev->scsi_dh_data->scsi_dh == scsi_dh)
+			goto out_put_device;
+
+		sdev_printk(KERN_WARNING, sdev,
+			"replacing device handler %s with %s!, "
+			"please fix the device handler tables.\n",
+			sdev->scsi_dh_data->scsi_dh->name, name);
+		scsi_dh_handler_detach(sdev);
 	}
+
+	err = scsi_dh_handler_attach(sdev, scsi_dh);
+
+out_put_device:
 	put_device(&sdev->sdev_gendev);
+	return err;
 }
-EXPORT_SYMBOL_GPL(scsi_dh_detach);
+EXPORT_SYMBOL_GPL(scsi_dh_attach);
 
 /*
  * scsi_dh_attached_handler_name - Get attached device handler's name
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index a4c9336..b711254 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -220,7 +220,6 @@ struct scsi_device_handler {
 struct scsi_dh_data {
 	struct scsi_device_handler *scsi_dh;
 	struct scsi_device *sdev;
-	struct kref kref;
 };
 
 #define	to_scsi_device(d)	\
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 620c723..99c9196 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -59,7 +59,6 @@ enum {
 extern int scsi_dh_activate(struct request_queue *, activate_complete, void *);
 extern int scsi_dh_handler_exist(const char *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
-extern void scsi_dh_detach(struct request_queue *);
 extern const char *scsi_dh_attached_handler_name(struct request_queue *, gfp_t);
 extern int scsi_dh_set_params(struct request_queue *, const char *);
 #else
@@ -77,10 +76,6 @@ static inline int scsi_dh_attach(struct request_queue *req, const char *name)
 {
 	return SCSI_DH_NOSYS;
 }
-static inline void scsi_dh_detach(struct request_queue *q)
-{
-	return;
-}
 static inline const char *scsi_dh_attached_handler_name(struct request_queue *q,
 							gfp_t gfp)
 {
-- 
1.9.1

Re: [PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> While allowing dm-mpath to attach device handlers is a functionality we need
> for backwards compatibility reason there is no reason to reference count
> them and detach them if dm-mpath stops using the device for some reason.
> 
> If the device handler works for the given device it can just stay attached.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Looks good.  But, unless I'm missing something, these DM and scsi_dh
changes can be made in separate patches so that I can carry the DM
change without needing to touch scsi_dh (SCSI maintainer boundary).

Thanks,
Mike

Re: [PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Christoph Hellwig]

On Thu, Apr 30, 2015 at 02:21:54PM -0400, Mike Snitzer wrote:
> > While allowing dm-mpath to attach device handlers is a functionality we need
> > for backwards compatibility reason there is no reason to reference count
> > them and detach them if dm-mpath stops using the device for some reason.
> > 
> > If the device handler works for the given device it can just stay attached.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> 
> Looks good.  But, unless I'm missing something, these DM and scsi_dh
> changes can be made in separate patches so that I can carry the DM
> change without needing to touch scsi_dh (SCSI maintainer boundary).

Unfortunately they can't.  E.g. if we remove the detach in dm-mpath
but keep the refcounting in scsi_dh we'll leak references.  Of if we
remove the request_module in dm without adding it to scsi we stop
asking for the module.  So I'll need to collect ACKs from both sides
and find a suitable tree to get this in.

Re: [PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> While allowing dm-mpath to attach device handlers is a functionality we need
> for backwards compatibility reason there is no reason to reference count
> them and detach them if dm-mpath stops using the device for some reason.
> 
> If the device handler works for the given device it can just stay attached.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> While allowing dm-mpath to attach device handlers is a functionality we need
> for backwards compatibility reason there is no reason to reference count
> them and detach them if dm-mpath stops using the device for some reason.
> 
> If the device handler works for the given device it can just stay attached.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Mike Snitzer <snitzer@redhat.com>

Re: [PATCH 2/9] dm-mpath, scsi_dh: don't let dm detach device handlers

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> While allowing dm-mpath to attach device handlers is a
Christoph> functionality we need for backwards compatibility reason
Christoph> there is no reason to reference count them and detach them if
Christoph> dm-mpath stops using the device for some reason.

Christoph> If the device handler works for the given device it can just
Christoph> stay attached.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 3/9] dm-mpath: don't call scsi_dh_attach when we want to retain the attached handler

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-mpath.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 9fb91ca..f6d40d3 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -591,9 +591,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 			kfree(m->hw_handler_params);
 			m->hw_handler_params = NULL;
 		}
-	}
-
-	if (m->hw_handler_name) {
+	} else if (m->hw_handler_name) {
 		/*
 		 * Increments scsi_dh reference, even when using an
 		 * already-attached handler.
@@ -604,15 +602,15 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 			dm_put_device(ti, p->path.dev);
 			goto bad;
 		}
+	}
 
-		if (m->hw_handler_params) {
-			r = scsi_dh_set_params(q, m->hw_handler_params);
-			if (r < 0) {
-				ti->error = "unable to set hardware "
-							"handler parameters";
-				dm_put_device(ti, p->path.dev);
-				goto bad;
-			}
+	if (m->hw_handler_name && m->hw_handler_params) {
+		r = scsi_dh_set_params(q, m->hw_handler_params);
+		if (r < 0) {
+			ti->error = "unable to set hardware "
+						"handler parameters";
+			dm_put_device(ti, p->path.dev);
+			goto bad;
 		}
 	}
 
-- 
1.9.1

Re: [PATCH 3/9] dm-mpath: don't call scsi_dh_attach when we want to retain the attached handler

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/md/dm-mpath.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 9fb91ca..f6d40d3 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -591,9 +591,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>  			kfree(m->hw_handler_params);
>  			m->hw_handler_params = NULL;
>  		}
> -	}
> -
> -	if (m->hw_handler_name) {
> +	} else if (m->hw_handler_name) {
>  		/*
>  		 * Increments scsi_dh reference, even when using an
>  		 * already-attached handler.
> @@ -604,15 +602,15 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>  			dm_put_device(ti, p->path.dev);
>  			goto bad;
>  		}
> +	}
>  
> -		if (m->hw_handler_params) {
> -			r = scsi_dh_set_params(q, m->hw_handler_params);
> -			if (r < 0) {
> -				ti->error = "unable to set hardware "
> -							"handler parameters";
> -				dm_put_device(ti, p->path.dev);
> -				goto bad;
> -			}
> +	if (m->hw_handler_name && m->hw_handler_params) {
> +		r = scsi_dh_set_params(q, m->hw_handler_params);
> +		if (r < 0) {
> +			ti->error = "unable to set hardware "
> +						"handler parameters";
> +			dm_put_device(ti, p->path.dev);
> +			goto bad;
>  		}
>  	}
>  

Would prefer to see the old weird error message line wrapping fixed up
to be on a single line (80 cols be damned).

Acked-by: Mike Snitzer <snitzer@redhat.com>

Re: [PATCH 3/9] dm-mpath: don't call scsi_dh_attach when we want to retain the attached handler

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/9] dm-mpath: don't call scsi_dh_attach when we want to retain the attached handler

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

+	} else if (m->hw_handler_name) {
 		/*
 		 * Increments scsi_dh reference, even when using an
 		 * already-attached handler.

Shouldn't this comment be updated to reflect your previous patch?

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 3/9] dm-mpath: don't call scsi_dh_attach when we want to retain the attached handler

[Christoph Hellwig]

On Fri, May 01, 2015 at 11:34:25AM -0400, Martin K. Petersen wrote:
> +	} else if (m->hw_handler_name) {
>  		/*
>  		 * Increments scsi_dh reference, even when using an
>  		 * already-attached handler.
> 
> Shouldn't this comment be updated to reflect your previous patch?

Yes, it should.  I'll fix it for the resend.

[PATCH 4/9] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath

[Christoph Hellwig]

This way we can reused the same code any attachment method, not just those
requested from dm-mpath.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/md/dm-mpath.c                 |  7 -------
 drivers/scsi/device_handler/scsi_dh.c | 35 ++++++++++++++++++-----------------
 include/scsi/scsi_dh.h                |  5 -----
 3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index f6d40d3..04c1a99 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -720,13 +720,6 @@ static int parse_hw_handler(struct dm_arg_set *as, struct multipath *m)
 	if (!m->hw_handler_name)
 		return -ENOMEM;
 
-	if (!try_then_request_module(scsi_dh_handler_exist(m->hw_handler_name),
-				     "scsi_dh_%s", m->hw_handler_name)) {
-		ti->error = "unknown hardware handler type";
-		ret = -EINVAL;
-		goto fail;
-	}
-
 	if (hw_argc > 1) {
 		char *p;
 		int i, j, len = 4;
diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 93fd7d4..e6ed565 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -29,7 +29,7 @@
 static DEFINE_SPINLOCK(list_lock);
 static LIST_HEAD(scsi_dh_list);
 
-static struct scsi_device_handler *get_device_handler(const char *name)
+static struct scsi_device_handler *__scsi_dh_lookup(const char *name)
 {
 	struct scsi_device_handler *tmp, *found = NULL;
 
@@ -44,6 +44,19 @@ static struct scsi_device_handler *get_device_handler(const char *name)
 	return found;
 }
 
+static struct scsi_device_handler *scsi_dh_lookup(const char *name)
+{
+	struct scsi_device_handler *dh;
+
+	dh = __scsi_dh_lookup(name);
+	if (!dh) {
+		request_module(name);
+		dh = __scsi_dh_lookup(name);
+	}
+
+	return dh;
+}
+
 /*
  * device_handler_match_function - Match a device handler to a device
  * @sdev - SCSI device to be tested
@@ -158,7 +171,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 		/*
 		 * Attach to a device handler
 		 */
-		if (!(scsi_dh = get_device_handler(buf)))
+		if (!(scsi_dh = scsi_dh_lookup(buf)))
 			return err;
 		err = scsi_dh_handler_attach(sdev, scsi_dh);
 	} else {
@@ -322,8 +335,7 @@ static int scsi_dh_notifier_remove(struct device *dev, void *data)
  */
 int scsi_register_device_handler(struct scsi_device_handler *scsi_dh)
 {
-
-	if (get_device_handler(scsi_dh->name))
+	if (__scsi_dh_lookup(scsi_dh->name))
 		return -EBUSY;
 
 	if (!scsi_dh->attach || !scsi_dh->detach)
@@ -350,7 +362,7 @@ EXPORT_SYMBOL_GPL(scsi_register_device_handler);
 int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh)
 {
 
-	if (!get_device_handler(scsi_dh->name))
+	if (!__scsi_dh_lookup(scsi_dh->name))
 		return -ENODEV;
 
 	bus_for_each_dev(&scsi_bus_type, NULL, scsi_dh,
@@ -455,17 +467,6 @@ int scsi_dh_set_params(struct request_queue *q, const char *params)
 EXPORT_SYMBOL_GPL(scsi_dh_set_params);
 
 /*
- * scsi_dh_handler_exist - Return TRUE(1) if a device handler exists for
- *	the given name. FALSE(0) otherwise.
- * @name - name of the device handler.
- */
-int scsi_dh_handler_exist(const char *name)
-{
-	return (get_device_handler(name) != NULL);
-}
-EXPORT_SYMBOL_GPL(scsi_dh_handler_exist);
-
-/*
  * scsi_dh_attach - Attach device handler
  * @q - Request queue that is associated with the scsi_device
  *      the handler should be attached to
@@ -478,7 +479,7 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 	struct scsi_device_handler *scsi_dh;
 	int err = 0;
 
-	scsi_dh = get_device_handler(name);
+	scsi_dh = scsi_dh_lookup(name);
 	if (!scsi_dh)
 		return -EINVAL;
 
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 99c9196..966b921 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -57,7 +57,6 @@ enum {
 };
 #if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE)
 extern int scsi_dh_activate(struct request_queue *, activate_complete, void *);
-extern int scsi_dh_handler_exist(const char *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
 extern const char *scsi_dh_attached_handler_name(struct request_queue *, gfp_t);
 extern int scsi_dh_set_params(struct request_queue *, const char *);
@@ -68,10 +67,6 @@ static inline int scsi_dh_activate(struct request_queue *req,
 	fn(data, 0);
 	return 0;
 }
-static inline int scsi_dh_handler_exist(const char *name)
-{
-	return 0;
-}
 static inline int scsi_dh_attach(struct request_queue *req, const char *name)
 {
 	return SCSI_DH_NOSYS;
-- 
1.9.1

Re: [PATCH 4/9] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> This way we can reused the same code any attachment method, not just those
> requested from dm-mpath.

Don't think feature bisectability matters on this patchset does it?
Can't we split dm-mpath and scsi_dh changes into separate patches?

Risk is I push the dm changes but SCSI doesn't get around to it within
the same cycle... we just cannot let that happen though.

Re: [PATCH 4/9] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> This way we can reused the same code any attachment method, not just those
> requested from dm-mpath.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/9] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> This way we can reused the same code any attachment method, not just those
> requested from dm-mpath.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Mike Snitzer <snitzer@redhat.com>

Re: [PATCH 4/9] dm-mpath, scsi_dh: request scsi_dh modules in scsi_dh, not dm-mpath

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> This way we can reused the same code any attachment method,
Christoph> not just those requested from dm-mpath.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 5/9] scsi_dh: integrate into the core SCSI code

[Christoph Hellwig]

Stop building scsi_dh as a separate module and integrate it fully into the
core SCSI code with explicit callouts at bus scan time.  For now the
callouts are placed at the same point as the old bus notifiers were called,
but in the future we will be able to look at ALUA INQUIRY data earlier on.

Note that this also means that the device handler modules need to be loaded
by the time we scan the bus.  The next patches will add support for
autoloading device handlers at bus scan time to make sure they are always
loaded if they are enabled in the kernel config.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/Makefile                 |   1 +
 drivers/scsi/device_handler/Kconfig   |   2 +-
 drivers/scsi/device_handler/Makefile  |   1 -
 drivers/scsi/device_handler/scsi_dh.c | 185 +++-------------------------------
 drivers/scsi/scsi_priv.h              |   9 ++
 drivers/scsi/scsi_sysfs.c             |  10 ++
 include/scsi/scsi_dh.h                |   2 +-
 7 files changed, 35 insertions(+), 175 deletions(-)

diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
index dee160a..df2f656 100644
--- a/drivers/scsi/Makefile
+++ b/drivers/scsi/Makefile
@@ -169,6 +169,7 @@ scsi_mod-$(CONFIG_SYSCTL)	+= scsi_sysctl.o
 scsi_mod-$(CONFIG_SCSI_PROC_FS)	+= scsi_proc.o
 scsi_mod-y			+= scsi_trace.o scsi_logging.o
 scsi_mod-$(CONFIG_PM)		+= scsi_pm.o
+scsi_mod-$(CONFIG_SCSI_DH)	+= device_handler/scsi_dh.o
 
 hv_storvsc-y			:= storvsc_drv.o
 
diff --git a/drivers/scsi/device_handler/Kconfig b/drivers/scsi/device_handler/Kconfig
index 69abd0a..e5647d5 100644
--- a/drivers/scsi/device_handler/Kconfig
+++ b/drivers/scsi/device_handler/Kconfig
@@ -3,7 +3,7 @@
 #
 
 menuconfig SCSI_DH
-	tristate "SCSI Device Handlers"
+	bool "SCSI Device Handlers"
 	depends on SCSI
 	default n
 	help
diff --git a/drivers/scsi/device_handler/Makefile b/drivers/scsi/device_handler/Makefile
index e1d2ea0..09866c5 100644
--- a/drivers/scsi/device_handler/Makefile
+++ b/drivers/scsi/device_handler/Makefile
@@ -1,7 +1,6 @@
 #
 # SCSI Device Handler
 #
-obj-$(CONFIG_SCSI_DH)		+= scsi_dh.o
 obj-$(CONFIG_SCSI_DH_RDAC)	+= scsi_dh_rdac.o
 obj-$(CONFIG_SCSI_DH_HP_SW)	+= scsi_dh_hp_sw.o
 obj-$(CONFIG_SCSI_DH_EMC)	+= scsi_dh_emc.o
diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index e6ed565..cb336a4 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -24,7 +24,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <scsi/scsi_dh.h>
-#include "../scsi_priv.h"
+#include "scsi_priv.h"
 
 static DEFINE_SPINLOCK(list_lock);
 static LIST_HEAD(scsi_dh_list);
@@ -57,15 +57,8 @@ static struct scsi_device_handler *scsi_dh_lookup(const char *name)
 	return dh;
 }
 
-/*
- * device_handler_match_function - Match a device handler to a device
- * @sdev - SCSI device to be tested
- *
- * Tests @sdev against the match function of all registered device_handler.
- * Returns the found device handler or NULL if not found.
- */
 static struct scsi_device_handler *
-device_handler_match_function(struct scsi_device *sdev)
+device_handler_match(struct scsi_device *sdev)
 {
 	struct scsi_device_handler *tmp_dh, *found_dh = NULL;
 
@@ -81,29 +74,6 @@ device_handler_match_function(struct scsi_device *sdev)
 }
 
 /*
- * device_handler_match - Attach a device handler to a device
- * @scsi_dh - The device handler to match against or NULL
- * @sdev - SCSI device to be tested against @scsi_dh
- *
- * Tests @sdev against the device handler @scsi_dh or against
- * all registered device_handler if @scsi_dh == NULL.
- * Returns the found device handler or NULL if not found.
- */
-static struct scsi_device_handler *
-device_handler_match(struct scsi_device_handler *scsi_dh,
-		     struct scsi_device *sdev)
-{
-	struct scsi_device_handler *found_dh;
-
-	found_dh = device_handler_match_function(sdev);
-
-	if (scsi_dh && found_dh != scsi_dh)
-		found_dh = NULL;
-
-	return found_dh;
-}
-
-/*
  * scsi_dh_handler_attach - Attach a device handler to a device
  * @sdev - SCSI device the device handler should attach to
  * @scsi_dh - The device handler to attach
@@ -211,119 +181,26 @@ static struct device_attribute scsi_dh_state_attr =
 	__ATTR(dh_state, S_IRUGO | S_IWUSR, show_dh_state,
 	       store_dh_state);
 
-/*
- * scsi_dh_sysfs_attr_add - Callback for scsi_init_dh
- */
-static int scsi_dh_sysfs_attr_add(struct device *dev, void *data)
+int scsi_dh_add_device(struct scsi_device *sdev)
 {
-	struct scsi_device *sdev;
+	struct scsi_device_handler *devinfo;
 	int err;
 
-	if (!scsi_is_sdev_device(dev))
-		return 0;
-
-	sdev = to_scsi_device(dev);
-
-	err = device_create_file(&sdev->sdev_gendev,
-				 &scsi_dh_state_attr);
-
-	return 0;
-}
-
-/*
- * scsi_dh_sysfs_attr_remove - Callback for scsi_exit_dh
- */
-static int scsi_dh_sysfs_attr_remove(struct device *dev, void *data)
-{
-	struct scsi_device *sdev;
-
-	if (!scsi_is_sdev_device(dev))
-		return 0;
-
-	sdev = to_scsi_device(dev);
-
-	device_remove_file(&sdev->sdev_gendev,
-			   &scsi_dh_state_attr);
-
-	return 0;
-}
+	err = device_create_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
+	if (err)
+		return err;
 
-/*
- * scsi_dh_notifier - notifier chain callback
- */
-static int scsi_dh_notifier(struct notifier_block *nb,
-			    unsigned long action, void *data)
-{
-	struct device *dev = data;
-	struct scsi_device *sdev;
-	int err = 0;
-	struct scsi_device_handler *devinfo = NULL;
-
-	if (!scsi_is_sdev_device(dev))
-		return 0;
-
-	sdev = to_scsi_device(dev);
-
-	if (action == BUS_NOTIFY_ADD_DEVICE) {
-		err = device_create_file(dev, &scsi_dh_state_attr);
-		/* don't care about err */
-		devinfo = device_handler_match(NULL, sdev);
-		if (devinfo)
-			err = scsi_dh_handler_attach(sdev, devinfo);
-	} else if (action == BUS_NOTIFY_DEL_DEVICE) {
-		device_remove_file(dev, &scsi_dh_state_attr);
-		if (sdev->scsi_dh_data)
-			scsi_dh_handler_detach(sdev);
-	}
+	devinfo = device_handler_match(sdev);
+	if (devinfo)
+		err = scsi_dh_handler_attach(sdev, devinfo);
 	return err;
 }
 
-/*
- * scsi_dh_notifier_add - Callback for scsi_register_device_handler
- */
-static int scsi_dh_notifier_add(struct device *dev, void *data)
+void scsi_dh_remove_device(struct scsi_device *sdev)
 {
-	struct scsi_device_handler *scsi_dh = data;
-	struct scsi_device *sdev;
-
-	if (!scsi_is_sdev_device(dev))
-		return 0;
-
-	if (!get_device(dev))
-		return 0;
-
-	sdev = to_scsi_device(dev);
-
-	if (device_handler_match(scsi_dh, sdev))
-		scsi_dh_handler_attach(sdev, scsi_dh);
-
-	put_device(dev);
-
-	return 0;
-}
-
-/*
- * scsi_dh_notifier_remove - Callback for scsi_unregister_device_handler
- */
-static int scsi_dh_notifier_remove(struct device *dev, void *data)
-{
-	struct scsi_device_handler *scsi_dh = data;
-	struct scsi_device *sdev;
-
-	if (!scsi_is_sdev_device(dev))
-		return 0;
-
-	if (!get_device(dev))
-		return 0;
-
-	sdev = to_scsi_device(dev);
-
-	if (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh == scsi_dh)
+	if (sdev->scsi_dh_data)
 		scsi_dh_handler_detach(sdev);
-
-	put_device(dev);
-
-	return 0;
+	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
 }
 
 /*
@@ -345,7 +222,6 @@ int scsi_register_device_handler(struct scsi_device_handler *scsi_dh)
 	list_add(&scsi_dh->list, &scsi_dh_list);
 	spin_unlock(&list_lock);
 
-	bus_for_each_dev(&scsi_bus_type, NULL, scsi_dh, scsi_dh_notifier_add);
 	printk(KERN_INFO "%s: device handler registered\n", scsi_dh->name);
 
 	return SCSI_DH_OK;
@@ -361,13 +237,9 @@ EXPORT_SYMBOL_GPL(scsi_register_device_handler);
  */
 int scsi_unregister_device_handler(struct scsi_device_handler *scsi_dh)
 {
-
 	if (!__scsi_dh_lookup(scsi_dh->name))
 		return -ENODEV;
 
-	bus_for_each_dev(&scsi_bus_type, NULL, scsi_dh,
-			 scsi_dh_notifier_remove);
-
 	spin_lock(&list_lock);
 	list_del(&scsi_dh->list);
 	spin_unlock(&list_lock);
@@ -542,34 +414,3 @@ const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
 	return handler_name;
 }
 EXPORT_SYMBOL_GPL(scsi_dh_attached_handler_name);
-
-static struct notifier_block scsi_dh_nb = {
-	.notifier_call = scsi_dh_notifier
-};
-
-static int __init scsi_dh_init(void)
-{
-	int r;
-
-	r = bus_register_notifier(&scsi_bus_type, &scsi_dh_nb);
-
-	if (!r)
-		bus_for_each_dev(&scsi_bus_type, NULL, NULL,
-				 scsi_dh_sysfs_attr_add);
-
-	return r;
-}
-
-static void __exit scsi_dh_exit(void)
-{
-	bus_for_each_dev(&scsi_bus_type, NULL, NULL,
-			 scsi_dh_sysfs_attr_remove);
-	bus_unregister_notifier(&scsi_bus_type, &scsi_dh_nb);
-}
-
-module_init(scsi_dh_init);
-module_exit(scsi_dh_exit);
-
-MODULE_DESCRIPTION("SCSI device handler");
-MODULE_AUTHOR("Chandra Seetharaman <sekharan@us.ibm.com>");
-MODULE_LICENSE("GPL");
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index e3902fc..644bb73 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -170,6 +170,15 @@ static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}
 extern struct async_domain scsi_sd_pm_domain;
 extern struct async_domain scsi_sd_probe_domain;
 
+/* scsi_dh.c */
+#ifdef CONFIG_SCSI_DH
+int scsi_dh_add_device(struct scsi_device *sdev);
+void scsi_dh_remove_device(struct scsi_device *sdev);
+#else
+static inline int scsi_dh_add_device(struct scsi_device *sdev) { return 0; }
+static inline void scsi_dh_remove_device(struct scsi_device *sdev) { }
+#endif
+
 /* 
  * internal scsi timeout functions: for use by mid-layer and transport
  * classes.
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 1ac38e7..c8a120f 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1030,11 +1030,20 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 				"failed to add device: %d\n", error);
 		return error;
 	}
+
+	error = scsi_dh_add_device(sdev);
+	if (error) {
+		sdev_printk(KERN_INFO, sdev,
+				"failed to add device handler: %d\n", error);
+		return error;
+	}
+
 	device_enable_async_suspend(&sdev->sdev_dev);
 	error = device_add(&sdev->sdev_dev);
 	if (error) {
 		sdev_printk(KERN_INFO, sdev,
 				"failed to add class device: %d\n", error);
+		scsi_dh_remove_device(sdev);
 		device_del(&sdev->sdev_gendev);
 		return error;
 	}
@@ -1074,6 +1083,7 @@ void __scsi_remove_device(struct scsi_device *sdev)
 		bsg_unregister_queue(sdev->request_queue);
 		device_unregister(&sdev->sdev_dev);
 		transport_remove_device(dev);
+		scsi_dh_remove_device(sdev);
 		device_del(dev);
 	} else
 		put_device(&sdev->sdev_dev);
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 966b921..3a37b4c45 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -55,7 +55,7 @@ enum {
 	SCSI_DH_NOSYS,
 	SCSI_DH_DRIVER_MAX,
 };
-#if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE)
+#ifdef CONFIG_SCSI_DH
 extern int scsi_dh_activate(struct request_queue *, activate_complete, void *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
 extern const char *scsi_dh_attached_handler_name(struct request_queue *, gfp_t);
-- 
1.9.1

Re: [PATCH 5/9] scsi_dh: integrate into the core SCSI code

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Stop building scsi_dh as a separate module and integrate it fully into the
> core SCSI code with explicit callouts at bus scan time.  For now the
> callouts are placed at the same point as the old bus notifiers were called,
> but in the future we will be able to look at ALUA INQUIRY data earlier on.
> 
> Note that this also means that the device handler modules need to be loaded
> by the time we scan the bus.  The next patches will add support for
> autoloading device handlers at bus scan time to make sure they are always
> loaded if they are enabled in the kernel config.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] scsi_dh: integrate into the core SCSI code

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> Stop building scsi_dh as a separate module and integrate it
Christoph> fully into the core SCSI code with explicit callouts at bus
Christoph> scan time.  For now the callouts are placed at the same point
Christoph> as the old bus notifiers were called, but in the future we
Christoph> will be able to look at ALUA INQUIRY data earlier on.

Christoph> Note that this also means that the device handler modules
Christoph> need to be loaded by the time we scan the bus.  The next
Christoph> patches will add support for autoloading device handlers at
Christoph> bus scan time to make sure they are always loaded if they are
Christoph> enabled in the kernel config.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 5/9] scsi_dh: integrate into the core SCSI code

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Stop building scsi_dh as a separate module and integrate it fully into the
> core SCSI code with explicit callouts at bus scan time.  For now the
> callouts are placed at the same point as the old bus notifiers were called,
> but in the future we will be able to look at ALUA INQUIRY data earlier on.
> 
> Note that this also means that the device handler modules need to be loaded
> by the time we scan the bus.  The next patches will add support for
> autoloading device handlers at bus scan time to make sure they are always
> loaded if they are enabled in the kernel config.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/Makefile                 |   1 +
>  drivers/scsi/device_handler/Kconfig   |   2 +-
>  drivers/scsi/device_handler/Makefile  |   1 -
>  drivers/scsi/device_handler/scsi_dh.c | 185 +++-------------------------------
>  drivers/scsi/scsi_priv.h              |   9 ++
>  drivers/scsi/scsi_sysfs.c             |  10 ++
>  include/scsi/scsi_dh.h                |   2 +-
>  7 files changed, 35 insertions(+), 175 deletions(-)
> 
> diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile
> index dee160a..df2f656 100644
> --- a/drivers/scsi/Makefile
> +++ b/drivers/scsi/Makefile
> @@ -169,6 +169,7 @@ scsi_mod-$(CONFIG_SYSCTL)	+= scsi_sysctl.o
>  scsi_mod-$(CONFIG_SCSI_PROC_FS)	+= scsi_proc.o
>  scsi_mod-y			+= scsi_trace.o scsi_logging.o
>  scsi_mod-$(CONFIG_PM)		+= scsi_pm.o
> +scsi_mod-$(CONFIG_SCSI_DH)	+= device_handler/scsi_dh.o
>  
>  hv_storvsc-y			:= storvsc_drv.o
>  
> diff --git a/drivers/scsi/device_handler/Kconfig b/drivers/scsi/device_handler/Kconfig
> index 69abd0a..e5647d5 100644
> --- a/drivers/scsi/device_handler/Kconfig
> +++ b/drivers/scsi/device_handler/Kconfig
> @@ -3,7 +3,7 @@
>  #
>  
>  menuconfig SCSI_DH
> -	tristate "SCSI Device Handlers"
> +	bool "SCSI Device Handlers"
>  	depends on SCSI
>  	default n
>  	help
> diff --git a/drivers/scsi/device_handler/Makefile b/drivers/scsi/device_handler/Makefile
> index e1d2ea0..09866c5 100644
> --- a/drivers/scsi/device_handler/Makefile
> +++ b/drivers/scsi/device_handler/Makefile
> @@ -1,7 +1,6 @@
>  #
>  # SCSI Device Handler
>  #
> -obj-$(CONFIG_SCSI_DH)		+= scsi_dh.o
>  obj-$(CONFIG_SCSI_DH_RDAC)	+= scsi_dh_rdac.o
>  obj-$(CONFIG_SCSI_DH_HP_SW)	+= scsi_dh_hp_sw.o
>  obj-$(CONFIG_SCSI_DH_EMC)	+= scsi_dh_emc.o
> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
> index e6ed565..cb336a4 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/drivers/scsi/device_handler/scsi_dh.c
> @@ -24,7 +24,7 @@
>  #include <linux/slab.h>
>  #include <linux/module.h>
>  #include <scsi/scsi_dh.h>
> -#include "../scsi_priv.h"
> +#include "scsi_priv.h"
>  
This doesn't compile; 'scsi_dh' is still at it's old location.

Please remove this hunk.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/9] scsi_dh: integrate into the core SCSI code

[Christoph Hellwig]

On Mon, May 04, 2015 at 09:50:38AM +0200, Hannes Reinecke wrote:
> This doesn't compile; 'scsi_dh' is still at it's old location.

What error do you get?  While I haven't move the scsi_dh.c file it
now is built from driver/scsi using a relative path in the makefile,
and I needed this change for it to compile.

[PATCH 6/9] scsi_dh: move device matching to the core code

[Christoph Hellwig]

Add a single list of devices that need non-ALUA device handlers to the core
scsi_dh code so that we can autoload the modules for them at probe time.

While this is a little ugly in terms of architecture it actually
significantly simplifies the code in addition to the new autoloading
functionality.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/device_handler/scsi_dh.c       | 84 ++++++++++++++++++++++-------
 drivers/scsi/device_handler/scsi_dh_alua.c  |  6 ---
 drivers/scsi/device_handler/scsi_dh_emc.c   | 29 ----------
 drivers/scsi/device_handler/scsi_dh_hp_sw.c | 30 -----------
 drivers/scsi/device_handler/scsi_dh_rdac.c  | 50 -----------------
 include/scsi/scsi_device.h                  |  1 -
 6 files changed, 66 insertions(+), 134 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index cb336a4..d4a0702 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -29,6 +29,67 @@
 static DEFINE_SPINLOCK(list_lock);
 static LIST_HEAD(scsi_dh_list);
 
+struct scsi_dh_blist {
+	const char *vendor;
+	const char *model;
+	const char *driver;
+};
+
+static const struct scsi_dh_blist scsi_dh_blist[] = {
+	{"DGC", "RAID",			"clariion" },
+	{"DGC", "DISK",			"clariion" },
+	{"DGC", "VRAID"			"clariion" },
+
+	{"COMPAQ", "MSA1000 VOLUME",	"hp_sw" },
+	{"COMPAQ", "HSV110",		"hp_sw" },
+	{"HP", "HSV100",		"hp_sw"},
+	{"DEC", "HSG80",		"hp_sw"},
+
+	{"IBM", "1722",			"rdac", },
+	{"IBM", "1724",			"rdac", },
+	{"IBM", "1726",			"rdac", },
+	{"IBM", "1742"			"rdac", },
+	{"IBM", "1745"			"rdac", },
+	{"IBM", "1746"			"rdac", },
+	{"IBM", "1813"			"rdac", },
+	{"IBM", "1814"			"rdac", },
+	{"IBM", "1815"			"rdac", },
+	{"IBM", "1818"			"rdac", },
+	{"IBM", "3526"			"rdac", },
+	{"SGI", "TP9"			"rdac", },
+	{"SGI", "IS"			"rdac", },
+	{"STK", "OPENstorage D280"	"rdac", },
+	{"STK", "FLEXLINE 380"		"rdac", },
+	{"SUN", "CSM"			"rdac", },
+	{"SUN", "LCSM100"		"rdac", },
+	{"SUN", "STK6580_6780"		"rdac", },
+	{"SUN", "SUN_6180"		"rdac", },
+	{"SUN", "ArrayStorage"		"rdac", },
+	{"DELL", "MD3"			"rdac", },
+	{"NETAPP", "INF-01-00"		"rdac", },
+	{"LSI", "INF-01-00"		"rdac", },
+	{"ENGENIO", "INF-01-00"		"rdac", },
+	{NULL, NULL},
+};
+
+static const char *
+scsi_dh_find_driver(struct scsi_device *sdev)
+{
+	const struct scsi_dh_blist *b;
+
+	if (scsi_device_tpgs(sdev))
+		return "alua";
+
+	for (b = scsi_dh_blist; b->vendor; b++) {
+		if (!strncmp(sdev->vendor, b->vendor, strlen(b->vendor)) &&
+		    !strncmp(sdev->model, b->model, strlen(b->model))) {
+			return b->driver;
+		}
+	}
+	return NULL;
+}
+
+
 static struct scsi_device_handler *__scsi_dh_lookup(const char *name)
 {
 	struct scsi_device_handler *tmp, *found = NULL;
@@ -57,22 +118,6 @@ static struct scsi_device_handler *scsi_dh_lookup(const char *name)
 	return dh;
 }
 
-static struct scsi_device_handler *
-device_handler_match(struct scsi_device *sdev)
-{
-	struct scsi_device_handler *tmp_dh, *found_dh = NULL;
-
-	spin_lock(&list_lock);
-	list_for_each_entry(tmp_dh, &scsi_dh_list, list) {
-		if (tmp_dh->match && tmp_dh->match(sdev)) {
-			found_dh = tmp_dh;
-			break;
-		}
-	}
-	spin_unlock(&list_lock);
-	return found_dh;
-}
-
 /*
  * scsi_dh_handler_attach - Attach a device handler to a device
  * @sdev - SCSI device the device handler should attach to
@@ -183,14 +228,17 @@ static struct device_attribute scsi_dh_state_attr =
 
 int scsi_dh_add_device(struct scsi_device *sdev)
 {
-	struct scsi_device_handler *devinfo;
+	struct scsi_device_handler *devinfo = NULL;
+	const char *drv;
 	int err;
 
 	err = device_create_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
 	if (err)
 		return err;
 
-	devinfo = device_handler_match(sdev);
+	drv = scsi_dh_find_driver(sdev);
+	if (drv)
+		devinfo = scsi_dh_lookup(drv);
 	if (devinfo)
 		err = scsi_dh_handler_attach(sdev, devinfo);
 	return err;
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 854b568..ace2457 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -819,11 +819,6 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 
 }
 
-static bool alua_match(struct scsi_device *sdev)
-{
-	return (scsi_device_tpgs(sdev) != 0);
-}
-
 /*
  * alua_bus_attach - Attach device handler
  * @sdev: device to be attached to
@@ -877,7 +872,6 @@ static struct scsi_device_handler alua_dh = {
 	.check_sense = alua_check_sense,
 	.activate = alua_activate,
 	.set_params = alua_set_params,
-	.match = alua_match,
 };
 
 static int __init alua_init(void)
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 6ed1caa..fd31e67 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -622,34 +622,6 @@ done:
 	return result;
 }
 
-static const struct {
-	char *vendor;
-	char *model;
-} clariion_dev_list[] = {
-	{"DGC", "RAID"},
-	{"DGC", "DISK"},
-	{"DGC", "VRAID"},
-	{NULL, NULL},
-};
-
-static bool clariion_match(struct scsi_device *sdev)
-{
-	int i;
-
-	if (scsi_device_tpgs(sdev))
-		return false;
-
-	for (i = 0; clariion_dev_list[i].vendor; i++) {
-		if (!strncmp(sdev->vendor, clariion_dev_list[i].vendor,
-			strlen(clariion_dev_list[i].vendor)) &&
-		    !strncmp(sdev->model, clariion_dev_list[i].model,
-			strlen(clariion_dev_list[i].model))) {
-			return true;
-		}
-	}
-	return false;
-}
-
 static struct scsi_dh_data *clariion_bus_attach(struct scsi_device *sdev)
 {
 	struct clariion_dh_data *h;
@@ -698,7 +670,6 @@ static struct scsi_device_handler clariion_dh = {
 	.activate	= clariion_activate,
 	.prep_fn	= clariion_prep_fn,
 	.set_params	= clariion_set_params,
-	.match		= clariion_match,
 };
 
 static int __init clariion_init(void)
diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
index 485d995..1bf10d3 100644
--- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -311,35 +311,6 @@ static int hp_sw_activate(struct scsi_device *sdev,
 	return 0;
 }
 
-static const struct {
-	char *vendor;
-	char *model;
-} hp_sw_dh_data_list[] = {
-	{"COMPAQ", "MSA1000 VOLUME"},
-	{"COMPAQ", "HSV110"},
-	{"HP", "HSV100"},
-	{"DEC", "HSG80"},
-	{NULL, NULL},
-};
-
-static bool hp_sw_match(struct scsi_device *sdev)
-{
-	int i;
-
-	if (scsi_device_tpgs(sdev))
-		return false;
-
-	for (i = 0; hp_sw_dh_data_list[i].vendor; i++) {
-		if (!strncmp(sdev->vendor, hp_sw_dh_data_list[i].vendor,
-			strlen(hp_sw_dh_data_list[i].vendor)) &&
-		    !strncmp(sdev->model, hp_sw_dh_data_list[i].model,
-			strlen(hp_sw_dh_data_list[i].model))) {
-			return true;
-		}
-	}
-	return false;
-}
-
 static struct scsi_dh_data *hp_sw_bus_attach(struct scsi_device *sdev)
 {
 	struct hp_sw_dh_data *h;
@@ -379,7 +350,6 @@ static struct scsi_device_handler hp_sw_dh = {
 	.detach		= hp_sw_bus_detach,
 	.activate	= hp_sw_activate,
 	.prep_fn	= hp_sw_prep_fn,
-	.match		= hp_sw_match,
 };
 
 static int __init hp_sw_init(void)
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index b46ace3..d89616f 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -778,55 +778,6 @@ static int rdac_check_sense(struct scsi_device *sdev,
 	return SCSI_RETURN_NOT_HANDLED;
 }
 
-static const struct {
-	char *vendor;
-	char *model;
-} rdac_dev_list[] = {
-	{"IBM", "1722"},
-	{"IBM", "1724"},
-	{"IBM", "1726"},
-	{"IBM", "1742"},
-	{"IBM", "1745"},
-	{"IBM", "1746"},
-	{"IBM", "1813"},
-	{"IBM", "1814"},
-	{"IBM", "1815"},
-	{"IBM", "1818"},
-	{"IBM", "3526"},
-	{"SGI", "TP9"},
-	{"SGI", "IS"},
-	{"STK", "OPENstorage D280"},
-	{"STK", "FLEXLINE 380"},
-	{"SUN", "CSM"},
-	{"SUN", "LCSM100"},
-	{"SUN", "STK6580_6780"},
-	{"SUN", "SUN_6180"},
-	{"SUN", "ArrayStorage"},
-	{"DELL", "MD3"},
-	{"NETAPP", "INF-01-00"},
-	{"LSI", "INF-01-00"},
-	{"ENGENIO", "INF-01-00"},
-	{NULL, NULL},
-};
-
-static bool rdac_match(struct scsi_device *sdev)
-{
-	int i;
-
-	if (scsi_device_tpgs(sdev))
-		return false;
-
-	for (i = 0; rdac_dev_list[i].vendor; i++) {
-		if (!strncmp(sdev->vendor, rdac_dev_list[i].vendor,
-			strlen(rdac_dev_list[i].vendor)) &&
-		    !strncmp(sdev->model, rdac_dev_list[i].model,
-			strlen(rdac_dev_list[i].model))) {
-			return true;
-		}
-	}
-	return false;
-}
-
 static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev)
 {
 	struct rdac_dh_data *h;
@@ -895,7 +846,6 @@ static struct scsi_device_handler rdac_dh = {
 	.attach = rdac_bus_attach,
 	.detach = rdac_bus_detach,
 	.activate = rdac_activate,
-	.match = rdac_match,
 };
 
 static int __init rdac_init(void)
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index b711254..dcc5c69 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -214,7 +214,6 @@ struct scsi_device_handler {
 	int (*activate)(struct scsi_device *, activate_complete, void *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
-	bool (*match)(struct scsi_device *);
 };
 
 struct scsi_dh_data {
-- 
1.9.1

Re: [PATCH 6/9] scsi_dh: move device matching to the core code

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> Add a single list of devices that need non-ALUA device handlers to the core
> scsi_dh code so that we can autoload the modules for them at probe time.
> 
> While this is a little ugly in terms of architecture it actually
> significantly simplifies the code in addition to the new autoloading
> functionality.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Reviewed-by: Mike Snitzer <snitzer@redhat.com>

Re: [PATCH 6/9] scsi_dh: move device matching to the core code

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Add a single list of devices that need non-ALUA device handlers to the core
> scsi_dh code so that we can autoload the modules for them at probe time.
> 
> While this is a little ugly in terms of architecture it actually
> significantly simplifies the code in addition to the new autoloading
> functionality.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] scsi_dh: move device matching to the core code

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> Add a single list of devices that need non-ALUA device
Christoph> handlers to the core scsi_dh code so that we can autoload the
Christoph> modules for them at probe time.

Christoph> While this is a little ugly in terms of architecture

I think that approach is perfectly fine.

Christoph> it actually significantly simplifies the code in addition to
Christoph> the new autoloading functionality.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 7/9] scsi_dh: kill struct scsi_dh_data

[Christoph Hellwig]

Add a ->handler and a ->handler_data field to struct scsi_device and kill
this indirection.  Also move struct scsi_device_handler to scsi_dh.h so that
changes to it don't require rebuilding every SCSI LLDD.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/device_handler/scsi_dh.c       | 74 +++++++++++------------------
 drivers/scsi/device_handler/scsi_dh_alua.c  | 24 ++++------
 drivers/scsi/device_handler/scsi_dh_emc.c   | 28 ++++-------
 drivers/scsi/device_handler/scsi_dh_hp_sw.c | 24 ++++------
 drivers/scsi/device_handler/scsi_dh_rdac.c  | 29 +++++------
 drivers/scsi/scsi_error.c                   |  6 +--
 drivers/scsi/scsi_lib.c                     |  6 +--
 include/scsi/scsi_device.h                  | 25 ++--------
 include/scsi/scsi_dh.h                      | 17 +++++++
 9 files changed, 93 insertions(+), 140 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index d4a0702..ddc5b9c 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -126,26 +126,19 @@ static struct scsi_device_handler *scsi_dh_lookup(const char *name)
 static int scsi_dh_handler_attach(struct scsi_device *sdev,
 				  struct scsi_device_handler *scsi_dh)
 {
-	struct scsi_dh_data *d;
+	int error;
 
 	if (!try_module_get(scsi_dh->module))
 		return -EINVAL;
 
-	d = scsi_dh->attach(sdev);
-	if (IS_ERR(d)) {
-		sdev_printk(KERN_ERR, sdev, "%s: Attach failed (%ld)\n",
-			    scsi_dh->name, PTR_ERR(d));
+	error = scsi_dh->attach(sdev);
+	if (error) {
+		sdev_printk(KERN_ERR, sdev, "%s: Attach failed (%d)\n",
+			    scsi_dh->name, error);
 		module_put(scsi_dh->module);
-		return PTR_ERR(d);
 	}
 
-	d->scsi_dh = scsi_dh;
-	d->sdev = sdev;
-
-	spin_lock_irq(sdev->request_queue->queue_lock);
-	sdev->scsi_dh_data = d;
-	spin_unlock_irq(sdev->request_queue->queue_lock);
-	return 0;
+	return error;
 }
 
 /*
@@ -154,17 +147,9 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev,
  */
 static void scsi_dh_handler_detach(struct scsi_device *sdev)
 {
-	struct scsi_dh_data *scsi_dh_data = sdev->scsi_dh_data;
-	struct scsi_device_handler *scsi_dh = scsi_dh_data->scsi_dh;
-
-	scsi_dh->detach(sdev);
-
-	spin_lock_irq(sdev->request_queue->queue_lock);
-	sdev->scsi_dh_data = NULL;
-	spin_unlock_irq(sdev->request_queue->queue_lock);
-
-	sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", scsi_dh->name);
-	module_put(scsi_dh->module);
+	sdev->handler->detach(sdev);
+	sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", sdev->handler->name);
+	module_put(sdev->handler->module);
 }
 
 /*
@@ -182,7 +167,7 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 	    sdev->sdev_state == SDEV_DEL)
 		return -ENODEV;
 
-	if (!sdev->scsi_dh_data) {
+	if (!sdev->handler) {
 		/*
 		 * Attach to a device handler
 		 */
@@ -190,7 +175,6 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 			return err;
 		err = scsi_dh_handler_attach(sdev, scsi_dh);
 	} else {
-		scsi_dh = sdev->scsi_dh_data->scsi_dh;
 		if (!strncmp(buf, "detach", 6)) {
 			/*
 			 * Detach from a device handler
@@ -201,8 +185,8 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 			/*
 			 * Activate a device handler
 			 */
-			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev, NULL, NULL);
+			if (sdev->handler->activate)
+				err = sdev->handler->activate(sdev, NULL, NULL);
 			else
 				err = 0;
 		}
@@ -216,10 +200,10 @@ show_dh_state(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct scsi_device *sdev = to_scsi_device(dev);
 
-	if (!sdev->scsi_dh_data)
+	if (!sdev->handler)
 		return snprintf(buf, 20, "detached\n");
 
-	return snprintf(buf, 20, "%s\n", sdev->scsi_dh_data->scsi_dh->name);
+	return snprintf(buf, 20, "%s\n", sdev->handler->name);
 }
 
 static struct device_attribute scsi_dh_state_attr =
@@ -246,7 +230,7 @@ int scsi_dh_add_device(struct scsi_device *sdev)
 
 void scsi_dh_remove_device(struct scsi_device *sdev)
 {
-	if (sdev->scsi_dh_data)
+	if (sdev->handler)
 		scsi_dh_handler_detach(sdev);
 	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
 }
@@ -315,7 +299,6 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 	int err = 0;
 	unsigned long flags;
 	struct scsi_device *sdev;
-	struct scsi_device_handler *scsi_dh = NULL;
 	struct device *dev = NULL;
 
 	spin_lock_irqsave(q->queue_lock, flags);
@@ -328,10 +311,8 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 		return err;
 	}
 
-	if (sdev->scsi_dh_data)
-		scsi_dh = sdev->scsi_dh_data->scsi_dh;
 	dev = get_device(&sdev->sdev_gendev);
-	if (!scsi_dh || !dev ||
+	if (!sdev->handler || !dev ||
 	    sdev->sdev_state == SDEV_CANCEL ||
 	    sdev->sdev_state == SDEV_DEL)
 		err = SCSI_DH_NOSYS;
@@ -345,8 +326,8 @@ int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 		goto out;
 	}
 
-	if (scsi_dh->activate)
-		err = scsi_dh->activate(sdev, fn, data);
+	if (sdev->handler->activate)
+		err = sdev->handler->activate(sdev, fn, data);
 out:
 	put_device(dev);
 	return err;
@@ -368,19 +349,18 @@ int scsi_dh_set_params(struct request_queue *q, const char *params)
 	int err = -SCSI_DH_NOSYS;
 	unsigned long flags;
 	struct scsi_device *sdev;
-	struct scsi_device_handler *scsi_dh = NULL;
 
 	spin_lock_irqsave(q->queue_lock, flags);
 	sdev = q->queuedata;
-	if (sdev && sdev->scsi_dh_data)
-		scsi_dh = sdev->scsi_dh_data->scsi_dh;
-	if (scsi_dh && scsi_dh->set_params && get_device(&sdev->sdev_gendev))
+	if (sdev->handler &&
+	    sdev->handler->set_params &&
+	    get_device(&sdev->sdev_gendev))
 		err = 0;
 	spin_unlock_irqrestore(q->queue_lock, flags);
 
 	if (err)
 		return err;
-	err = scsi_dh->set_params(sdev, params);
+	err = sdev->handler->set_params(sdev, params);
 	put_device(&sdev->sdev_gendev);
 	return err;
 }
@@ -412,14 +392,14 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 	if (err)
 		return err;
 
-	if (sdev->scsi_dh_data) {
-		if (sdev->scsi_dh_data->scsi_dh == scsi_dh)
+	if (sdev->handler) {
+		if (sdev->handler == scsi_dh)
 			goto out_put_device;
 
 		sdev_printk(KERN_WARNING, sdev,
 			"replacing device handler %s with %s!, "
 			"please fix the device handler tables.\n",
-			sdev->scsi_dh_data->scsi_dh->name, name);
+			sdev->handler->name, name);
 		scsi_dh_handler_detach(sdev);
 	}
 
@@ -455,8 +435,8 @@ const char *scsi_dh_attached_handler_name(struct request_queue *q, gfp_t gfp)
 	if (!sdev)
 		return NULL;
 
-	if (sdev->scsi_dh_data)
-		handler_name = kstrdup(sdev->scsi_dh_data->scsi_dh->name, gfp);
+	if (sdev->handler)
+		handler_name = kstrdup(sdev->handler->name, gfp);
 
 	put_device(&sdev->sdev_gendev);
 	return handler_name;
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index ace2457..e418849 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -62,7 +62,6 @@
 #define ALUA_OPTIMIZE_STPG		1
 
 struct alua_dh_data {
-	struct scsi_dh_data	dh_data;
 	int			group_id;
 	int			rel_port;
 	int			tpgs;
@@ -86,11 +85,6 @@ struct alua_dh_data {
 static char print_alua_state(int);
 static int alua_check_sense(struct scsi_device *, struct scsi_sense_hdr *);
 
-static inline struct alua_dh_data *get_alua_data(struct scsi_device *sdev)
-{
-	return container_of(sdev->scsi_dh_data, struct alua_dh_data, dh_data);
-}
-
 static int realloc_buffer(struct alua_dh_data *h, unsigned len)
 {
 	if (h->buff && h->buff != h->inq)
@@ -708,7 +702,7 @@ out:
  */
 static int alua_set_params(struct scsi_device *sdev, const char *params)
 {
-	struct alua_dh_data *h = get_alua_data(sdev);
+	struct alua_dh_data *h = sdev->handler_data;
 	unsigned int optimize = 0, argc;
 	const char *p = params;
 	int result = SCSI_DH_OK;
@@ -746,7 +740,7 @@ MODULE_PARM_DESC(optimize_stpg, "Allow use of a non-optimized path, rather than
 static int alua_activate(struct scsi_device *sdev,
 			activate_complete fn, void *data)
 {
-	struct alua_dh_data *h = get_alua_data(sdev);
+	struct alua_dh_data *h = sdev->handler_data;
 	int err = SCSI_DH_OK;
 	int stpg = 0;
 
@@ -804,7 +798,7 @@ out:
  */
 static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
 {
-	struct alua_dh_data *h = get_alua_data(sdev);
+	struct alua_dh_data *h = sdev->handler_data;
 	int ret = BLKPREP_OK;
 
 	if (h->state == TPGS_STATE_TRANSITIONING)
@@ -823,14 +817,14 @@ static int alua_prep_fn(struct scsi_device *sdev, struct request *req)
  * alua_bus_attach - Attach device handler
  * @sdev: device to be attached to
  */
-static struct scsi_dh_data *alua_bus_attach(struct scsi_device *sdev)
+static int alua_bus_attach(struct scsi_device *sdev)
 {
 	struct alua_dh_data *h;
 	int err;
 
 	h = kzalloc(sizeof(*h) , GFP_KERNEL);
 	if (!h)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	h->tpgs = TPGS_MODE_UNINITIALIZED;
 	h->state = TPGS_STATE_OPTIMIZED;
 	h->group_id = -1;
@@ -843,11 +837,11 @@ static struct scsi_dh_data *alua_bus_attach(struct scsi_device *sdev)
 	if (err != SCSI_DH_OK && err != SCSI_DH_DEV_OFFLINED)
 		goto failed;
 
-	sdev_printk(KERN_NOTICE, sdev, "%s: Attached\n", ALUA_DH_NAME);
-	return &h->dh_data;
+	sdev->handler_data = h;
+	return 0;
 failed:
 	kfree(h);
-	return ERR_PTR(-EINVAL);
+	return -EINVAL;
 }
 
 /*
@@ -856,7 +850,7 @@ failed:
  */
 static void alua_bus_detach(struct scsi_device *sdev)
 {
-	struct alua_dh_data *h = get_alua_data(sdev);
+	struct alua_dh_data *h = sdev->handler_data;
 
 	if (h->buff && h->inq != h->buff)
 		kfree(h->buff);
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index fd31e67..4b75b50 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -72,7 +72,6 @@ static const char * lun_state[] =
 };
 
 struct clariion_dh_data {
-	struct scsi_dh_data dh_data;
 	/*
 	 * Flags:
 	 *  CLARIION_SHORT_TRESPASS
@@ -114,13 +113,6 @@ struct clariion_dh_data {
 	int current_sp;
 };
 
-static inline struct clariion_dh_data
-			*get_clariion_data(struct scsi_device *sdev)
-{
-	return container_of(sdev->scsi_dh_data, struct clariion_dh_data,
-			dh_data);
-}
-
 /*
  * Parse MODE_SELECT cmd reply.
  */
@@ -450,7 +442,7 @@ static int clariion_check_sense(struct scsi_device *sdev,
 
 static int clariion_prep_fn(struct scsi_device *sdev, struct request *req)
 {
-	struct clariion_dh_data *h = get_clariion_data(sdev);
+	struct clariion_dh_data *h = sdev->handler_data;
 	int ret = BLKPREP_OK;
 
 	if (h->lun_state != CLARIION_LUN_OWNED) {
@@ -533,7 +525,7 @@ retry:
 static int clariion_activate(struct scsi_device *sdev,
 				activate_complete fn, void *data)
 {
-	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	struct clariion_dh_data *csdev = sdev->handler_data;
 	int result;
 
 	result = clariion_send_inquiry(sdev, csdev);
@@ -574,7 +566,7 @@ done:
  */
 static int clariion_set_params(struct scsi_device *sdev, const char *params)
 {
-	struct clariion_dh_data *csdev = get_clariion_data(sdev);
+	struct clariion_dh_data *csdev = sdev->handler_data;
 	unsigned int hr = 0, st = 0, argc;
 	const char *p = params;
 	int result = SCSI_DH_OK;
@@ -622,14 +614,14 @@ done:
 	return result;
 }
 
-static struct scsi_dh_data *clariion_bus_attach(struct scsi_device *sdev)
+static int clariion_bus_attach(struct scsi_device *sdev)
 {
 	struct clariion_dh_data *h;
 	int err;
 
 	h = kzalloc(sizeof(*h) , GFP_KERNEL);
 	if (!h)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	h->lun_state = CLARIION_LUN_UNINITIALIZED;
 	h->default_sp = CLARIION_UNBOUND_LU;
 	h->current_sp = CLARIION_UNBOUND_LU;
@@ -647,18 +639,18 @@ static struct scsi_dh_data *clariion_bus_attach(struct scsi_device *sdev)
 		    CLARIION_NAME, h->current_sp + 'A',
 		    h->port, lun_state[h->lun_state],
 		    h->default_sp + 'A');
-	return &h->dh_data;
+
+	sdev->handler_data = h;
+	return 0;
 
 failed:
 	kfree(h);
-	return ERR_PTR(-EINVAL);
+	return -EINVAL;
 }
 
 static void clariion_bus_detach(struct scsi_device *sdev)
 {
-	struct clariion_dh_data *h = get_clariion_data(sdev);
-
-	kfree(h);
+	kfree(sdev->handler_data);
 }
 
 static struct scsi_device_handler clariion_dh = {
diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
index 1bf10d3..d5390ba 100644
--- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -38,7 +38,6 @@
 #define HP_SW_PATH_PASSIVE		1
 
 struct hp_sw_dh_data {
-	struct scsi_dh_data dh_data;
 	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
 	int path_state;
 	int retries;
@@ -50,11 +49,6 @@ struct hp_sw_dh_data {
 
 static int hp_sw_start_stop(struct hp_sw_dh_data *);
 
-static inline struct hp_sw_dh_data *get_hp_sw_data(struct scsi_device *sdev)
-{
-	return container_of(sdev->scsi_dh_data, struct hp_sw_dh_data, dh_data);
-}
-
 /*
  * tur_done - Handle TEST UNIT READY return status
  * @sdev: sdev the command has been sent to
@@ -267,7 +261,7 @@ static int hp_sw_start_stop(struct hp_sw_dh_data *h)
 
 static int hp_sw_prep_fn(struct scsi_device *sdev, struct request *req)
 {
-	struct hp_sw_dh_data *h = get_hp_sw_data(sdev);
+	struct hp_sw_dh_data *h = sdev->handler_data;
 	int ret = BLKPREP_OK;
 
 	if (h->path_state != HP_SW_PATH_ACTIVE) {
@@ -292,7 +286,7 @@ static int hp_sw_activate(struct scsi_device *sdev,
 				activate_complete fn, void *data)
 {
 	int ret = SCSI_DH_OK;
-	struct hp_sw_dh_data *h = get_hp_sw_data(sdev);
+	struct hp_sw_dh_data *h = sdev->handler_data;
 
 	ret = hp_sw_tur(sdev, h);
 
@@ -311,14 +305,14 @@ static int hp_sw_activate(struct scsi_device *sdev,
 	return 0;
 }
 
-static struct scsi_dh_data *hp_sw_bus_attach(struct scsi_device *sdev)
+static int hp_sw_bus_attach(struct scsi_device *sdev)
 {
 	struct hp_sw_dh_data *h;
 	int ret;
 
 	h = kzalloc(sizeof(*h), GFP_KERNEL);
 	if (!h)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	h->path_state = HP_SW_PATH_UNINITIALIZED;
 	h->retries = HP_SW_RETRIES;
 	h->sdev = sdev;
@@ -330,17 +324,17 @@ static struct scsi_dh_data *hp_sw_bus_attach(struct scsi_device *sdev)
 	sdev_printk(KERN_INFO, sdev, "%s: attached to %s path\n",
 		    HP_SW_NAME, h->path_state == HP_SW_PATH_ACTIVE?
 		    "active":"passive");
-	return &h->dh_data;
+
+	sdev->handler_data = h;
+	return 0;
 failed:
 	kfree(h);
-	return ERR_PTR(-EINVAL);
+	return -EINVAL;
 }
 
 static void hp_sw_bus_detach( struct scsi_device *sdev )
 {
-	struct hp_sw_dh_data *h = get_hp_sw_data(sdev);
-
-	kfree(h);
+	kfree(sdev->handler_data);
 }
 
 static struct scsi_device_handler hp_sw_dh = {
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index d89616f..0c6d612 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -181,7 +181,6 @@ struct c2_inquiry {
 };
 
 struct rdac_dh_data {
-	struct scsi_dh_data	dh_data;
 	struct rdac_controller	*ctlr;
 #define UNINITIALIZED_LUN	(1 << 8)
 	unsigned		lun;
@@ -260,11 +259,6 @@ do { \
 		sdev_printk(KERN_INFO, sdev, RDAC_NAME ": " f "\n", ## arg); \
 } while (0);
 
-static inline struct rdac_dh_data *get_rdac_data(struct scsi_device *sdev)
-{
-	return container_of(sdev->scsi_dh_data, struct rdac_dh_data, dh_data);
-}
-
 static struct request *get_rdac_req(struct scsi_device *sdev,
 			void *buffer, unsigned buflen, int rw)
 {
@@ -544,7 +538,7 @@ static int mode_select_handle_sense(struct scsi_device *sdev,
 {
 	struct scsi_sense_hdr sense_hdr;
 	int err = SCSI_DH_IO, ret;
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 
 	ret = scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE, &sense_hdr);
 	if (!ret)
@@ -589,7 +583,7 @@ static void send_mode_select(struct work_struct *work)
 		container_of(work, struct rdac_controller, ms_work);
 	struct request *rq;
 	struct scsi_device *sdev = ctlr->ms_sdev;
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 	struct request_queue *q = sdev->request_queue;
 	int err, retry_cnt = RDAC_RETRY_COUNT;
 	struct rdac_queue_data *tmp, *qdata;
@@ -648,7 +642,7 @@ static int queue_mode_select(struct scsi_device *sdev,
 	if (!qdata)
 		return SCSI_DH_RETRY;
 
-	qdata->h = get_rdac_data(sdev);
+	qdata->h = sdev->handler_data;
 	qdata->callback_fn = fn;
 	qdata->callback_data = data;
 
@@ -667,7 +661,7 @@ static int queue_mode_select(struct scsi_device *sdev,
 static int rdac_activate(struct scsi_device *sdev,
 			activate_complete fn, void *data)
 {
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 	int err = SCSI_DH_OK;
 	int act = 0;
 
@@ -702,7 +696,7 @@ done:
 
 static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)
 {
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 	int ret = BLKPREP_OK;
 
 	if (h->state != RDAC_STATE_ACTIVE) {
@@ -716,7 +710,7 @@ static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)
 static int rdac_check_sense(struct scsi_device *sdev,
 				struct scsi_sense_hdr *sense_hdr)
 {
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 
 	RDAC_LOG(RDAC_LOG_SENSE, sdev, "array %s, ctlr %d, "
 			"I/O returned with sense %02x/%02x/%02x",
@@ -778,7 +772,7 @@ static int rdac_check_sense(struct scsi_device *sdev,
 	return SCSI_RETURN_NOT_HANDLED;
 }
 
-static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev)
+static int rdac_bus_attach(struct scsi_device *sdev)
 {
 	struct rdac_dh_data *h;
 	int err;
@@ -787,7 +781,7 @@ static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev)
 
 	h = kzalloc(sizeof(*h) , GFP_KERNEL);
 	if (!h)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 	h->lun = UNINITIALIZED_LUN;
 	h->state = RDAC_STATE_ACTIVE;
 
@@ -812,7 +806,8 @@ static struct scsi_dh_data *rdac_bus_attach(struct scsi_device *sdev)
 		    RDAC_NAME, h->lun, mode[(int)h->mode],
 		    lun_state[(int)h->lun_state]);
 
-	return &h->dh_data;
+	sdev->handler_data = h;
+	return 0;
 
 clean_ctlr:
 	spin_lock(&list_lock);
@@ -821,12 +816,12 @@ clean_ctlr:
 
 failed:
 	kfree(h);
-	return ERR_PTR(-EINVAL);
+	return -EINVAL;
 }
 
 static void rdac_bus_detach( struct scsi_device *sdev )
 {
-	struct rdac_dh_data *h = get_rdac_data(sdev);
+	struct rdac_dh_data *h = sdev->handler_data;
 
 	if (h->ctlr && h->ctlr->ms_queued)
 		flush_workqueue(kmpath_rdacd);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c95a4e9..a29c0c9 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -37,6 +37,7 @@
 #include <scsi/scsi_transport.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_ioctl.h>
+#include <scsi/scsi_dh.h>
 #include <scsi/sg.h>
 
 #include "scsi_priv.h"
@@ -460,11 +461,10 @@ static int scsi_check_sense(struct scsi_cmnd *scmd)
 	if (scsi_sense_is_deferred(&sshdr))
 		return NEEDS_RETRY;
 
-	if (sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh &&
-			sdev->scsi_dh_data->scsi_dh->check_sense) {
+	if (sdev->handler && sdev->handler->check_sense) {
 		int rc;
 
-		rc = sdev->scsi_dh_data->scsi_dh->check_sense(sdev, &sshdr);
+		rc = sdev->handler->check_sense(sdev, &sshdr);
 		if (rc != SCSI_RETURN_NOT_HANDLED)
 			return rc;
 		/* handler does not care. Drop down to default handling */
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b1a2631..01c92ca 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -31,6 +31,7 @@
 #include <scsi/scsi_driver.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_host.h>
+#include <scsi/scsi_dh.h>
 
 #include <trace/events/scsi.h>
 
@@ -1248,9 +1249,8 @@ static int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 {
 	struct scsi_cmnd *cmd = req->special;
 
-	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
-			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
-		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
+	if (unlikely(sdev->handler && sdev->handler->prep_fn)) {
+		int ret = sdev->handler->prep_fn(sdev, req);
 		if (ret != BLKPREP_OK)
 			return ret;
 	}
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index dcc5c69..05583d8 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -195,32 +195,13 @@ struct scsi_device {
 	struct execute_work	ew; /* used to get process context on put */
 	struct work_struct	requeue_work;
 
-	struct scsi_dh_data	*scsi_dh_data;
+	struct scsi_device_handler *handler;
+	void			*handler_data;
+
 	enum scsi_device_state sdev_state;
 	unsigned long		sdev_data[0];
 } __attribute__((aligned(sizeof(unsigned long))));
 
-typedef void (*activate_complete)(void *, int);
-struct scsi_device_handler {
-	/* Used by the infrastructure */
-	struct list_head list; /* list of scsi_device_handlers */
-
-	/* Filled by the hardware handler */
-	struct module *module;
-	const char *name;
-	int (*check_sense)(struct scsi_device *, struct scsi_sense_hdr *);
-	struct scsi_dh_data *(*attach)(struct scsi_device *);
-	void (*detach)(struct scsi_device *);
-	int (*activate)(struct scsi_device *, activate_complete, void *);
-	int (*prep_fn)(struct scsi_device *, struct request *);
-	int (*set_params)(struct scsi_device *, const char *);
-};
-
-struct scsi_dh_data {
-	struct scsi_device_handler *scsi_dh;
-	struct scsi_device *sdev;
-};
-
 #define	to_scsi_device(d)	\
 	container_of(d, struct scsi_device, sdev_gendev)
 #define	class_to_sdev(d)	\
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index 3a37b4c45..85d7317 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -55,6 +55,23 @@ enum {
 	SCSI_DH_NOSYS,
 	SCSI_DH_DRIVER_MAX,
 };
+
+typedef void (*activate_complete)(void *, int);
+struct scsi_device_handler {
+	/* Used by the infrastructure */
+	struct list_head list; /* list of scsi_device_handlers */
+
+	/* Filled by the hardware handler */
+	struct module *module;
+	const char *name;
+	int (*check_sense)(struct scsi_device *, struct scsi_sense_hdr *);
+	int (*attach)(struct scsi_device *);
+	void (*detach)(struct scsi_device *);
+	int (*activate)(struct scsi_device *, activate_complete, void *);
+	int (*prep_fn)(struct scsi_device *, struct request *);
+	int (*set_params)(struct scsi_device *, const char *);
+};
+
 #ifdef CONFIG_SCSI_DH
 extern int scsi_dh_activate(struct request_queue *, activate_complete, void *);
 extern int scsi_dh_attach(struct request_queue *, const char *);
-- 
1.9.1

Re: [PATCH 7/9] scsi_dh: kill struct scsi_dh_data

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> Add a ->handler and a ->handler_data field to struct scsi_device and kill
> this indirection.  Also move struct scsi_device_handler to scsi_dh.h so that
> changes to it don't require rebuilding every SCSI LLDD.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/9] scsi_dh: kill struct scsi_dh_data

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> Add a ->handler and a ->handler_data field to struct
Christoph> scsi_device and kill this indirection.  Also move struct
Christoph> scsi_device_handler to scsi_dh.h so that changes to it don't
Christoph> require rebuilding every SCSI LLDD.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 8/9] scsi_dh: add a common helper to get a scsi_device from a request_queue

[Christoph Hellwig]

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>
---
 drivers/scsi/device_handler/scsi_dh.c | 100 ++++++++++++++++------------------
 1 file changed, 47 insertions(+), 53 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index ddc5b9c..0d6ab33 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -281,6 +281,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.
@@ -296,41 +310,38 @@ 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);
 
@@ -346,21 +357,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;
 }
@@ -374,23 +379,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)
@@ -422,22 +423,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;
 }
-- 
1.9.1

Re: [PATCH 8/9] scsi_dh: add a common helper to get a scsi_device from a request_queue

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> 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: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] scsi_dh: add a common helper to get a scsi_device from a request_queue

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

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

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 9/9] scsi_dh: don't allow to detach device handlers at runtime

[Christoph Hellwig]

The I/O submission and completion pathes call into the device handler
without any synchronization agains detachment.  So disallow detaching
device handlers at runtime.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/device_handler/scsi_dh.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 0d6ab33..1e945aa 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/drivers/scsi/device_handler/scsi_dh.c
@@ -142,17 +142,6 @@ static int scsi_dh_handler_attach(struct scsi_device *sdev,
 }
 
 /*
- * scsi_dh_handler_detach - Detach a device handler from a device
- * @sdev - SCSI device the device handler should be detached from
- */
-static void scsi_dh_handler_detach(struct scsi_device *sdev)
-{
-	sdev->handler->detach(sdev);
-	sdev_printk(KERN_NOTICE, sdev, "%s: Detached\n", sdev->handler->name);
-	module_put(sdev->handler->module);
-}
-
-/*
  * Functions for sysfs attribute 'dh_state'
  */
 static ssize_t
@@ -179,8 +168,9 @@ store_dh_state(struct device *dev, struct device_attribute *attr,
 			/*
 			 * Detach from a device handler
 			 */
-			scsi_dh_handler_detach(sdev);
-			err = 0;
+			sdev_printk(KERN_WARNING, sdev,
+				"can't detach handler %s!\n", buf);
+			err = -EINVAL;
 		} else if (!strncmp(buf, "activate", 8)) {
 			/*
 			 * Activate a device handler
@@ -230,8 +220,11 @@ int scsi_dh_add_device(struct scsi_device *sdev)
 
 void scsi_dh_remove_device(struct scsi_device *sdev)
 {
-	if (sdev->handler)
-		scsi_dh_handler_detach(sdev);
+	if (sdev->handler) {
+		sdev->handler->detach(sdev);
+		module_put(sdev->handler->module);
+	}
+
 	device_remove_file(&sdev->sdev_gendev, &scsi_dh_state_attr);
 }
 
@@ -393,15 +386,19 @@ int scsi_dh_attach(struct request_queue *q, const char *name)
 		goto out_put_device;
 	}
 
+	if (err)
+		return err;
+
 	if (sdev->handler) {
 		if (sdev->handler == scsi_dh)
 			goto out_put_device;
 
 		sdev_printk(KERN_WARNING, sdev,
-			"replacing device handler %s with %s!, "
+			"can't replace device handler %s with %s!, "
 			"please fix the device handler tables.\n",
 			sdev->handler->name, name);
-		scsi_dh_handler_detach(sdev);
+		err = -EINVAL;
+		goto out_put_device;
 	}
 
 	err = scsi_dh_handler_attach(sdev, scsi_dh);
-- 
1.9.1

Re: [PATCH 9/9] scsi_dh: don't allow to detach device handlers at runtime

[Hannes Reinecke]

On 04/30/2015 07:32 PM, Christoph Hellwig wrote:
> The I/O submission and completion pathes call into the device handler
> without any synchronization agains detachment.  So disallow detaching
> device handlers at runtime.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 9/9] scsi_dh: don't allow to detach device handlers at runtime

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> The I/O submission and completion pathes call into the device
Christoph> handler without any synchronization agains detachment.  So
Christoph> disallow detaching device handlers at runtime.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: integrate scsi_dh better into the scsi core

[Mike Snitzer]

On Thu, Apr 30 2015 at  1:32pm -0400,
Christoph Hellwig <hch@lst.de> wrote:

> This series ties scsi_dh deeper into the scsi core, and fixes all kinds
> of issues in it, most importantly the race between using and detaching
> device handlers.
> 

Yes please, thanks for doing this!

I worked around the various scan issues (due to scsi_dh mods not being
loaded before SCSI scan) by making all scsi_dh code builtin for RHEL7 --
would be nice to make the scsi_dh_handlers modules again. ;)

Re: integrate scsi_dh better into the scsi core

[Martin K. Petersen]

>>>>> "Christoph" == Christoph Hellwig <hch@lst.de> writes:

Christoph> This series ties scsi_dh deeper into the scsi core, and fixes
Christoph> all kinds of issues in it, most importantly the race between
Christoph> using and detaching device handlers.

Very nice work!

-- 
Martin K. Petersen	Oracle Linux Engineering