[PATCH 2/3] scsi_dh: add scsi_dh_attached_handler_name

[PATCH 2/3] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

The following dm-mpath patch may be considered PATCH 1/3 of this series:
http://www.redhat.com/archives/dm-devel/2012-May/msg00006.html

----8<---------

Introduce scsi_dh_attached_handler_name() to retrieve the name of the
scsi_dh that is attached to the scsi_device associated with the provided
request queue.  Returns NULL if a scsi_dh is not attached.

Also, fix scsi_dh_{attach,detach} function header comments to document
@q rather than @sdev.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/scsi/device_handler/scsi_dh.c |   31 +++++++++++++++++++++++++++++--
 include/scsi/scsi_dh.h                |    5 +++++
 2 files changed, 34 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.orig/drivers/scsi/device_handler/scsi_dh.c
+++ linux-2.6/drivers/scsi/device_handler/scsi_dh.c
@@ -468,7 +468,8 @@ EXPORT_SYMBOL_GPL(scsi_dh_handler_exist)
 
 /*
  * scsi_dh_attach - Attach device handler
- * @sdev - sdev the handler should be attached to
+ * @q - Request queue that is associated with the scsi_device
+ *      the handler should be attached to
  * @name - name of the handler to attach
  */
 int scsi_dh_attach(struct request_queue *q, const char *name)
@@ -498,7 +499,8 @@ EXPORT_SYMBOL_GPL(scsi_dh_attach);
 
 /*
  * scsi_dh_detach - Detach device handler
- * @sdev - sdev the handler should be detached from
+ * @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
@@ -527,6 +529,31 @@ void scsi_dh_detach(struct request_queue
 }
 EXPORT_SYMBOL_GPL(scsi_dh_detach);
 
+/*
+ * scsi_dh_attached_handler_name - Get attached device handler's name
+ * @q - Request queue that is associated with the scsi_device
+ *      that may have a device handler attached
+ *
+ * Returns name of attached scsi_dh, NULL if no handler is attached.
+ */
+const char *scsi_dh_attached_handler_name(struct request_queue *q)
+{
+	unsigned long flags;
+	struct scsi_device *sdev;
+
+	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 || !sdev->scsi_dh_data)
+		return NULL;
+
+	return sdev->scsi_dh_data->scsi_dh->name;
+}
+EXPORT_SYMBOL_GPL(scsi_dh_attached_handler_name);
+
 static struct notifier_block scsi_dh_nb = {
 	.notifier_call = scsi_dh_notifier
 };
Index: linux-2.6/include/scsi/scsi_dh.h
===================================================================
--- linux-2.6.orig/include/scsi/scsi_dh.h
+++ linux-2.6/include/scsi/scsi_dh.h
@@ -60,6 +60,7 @@ extern int scsi_dh_activate(struct reque
 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 *q);
 extern int scsi_dh_set_params(struct request_queue *, const char *);
 #else
 static inline int scsi_dh_activate(struct request_queue *req,
@@ -80,6 +81,10 @@ static inline void scsi_dh_detach(struct
 {
 	return;
 }
+static inline const char *scsi_dh_attached_handler_name(struct request_queue *q)
+{
+	return NULL;
+}
 static inline int scsi_dh_set_params(struct request_queue *req, const char *params)
 {
 	return -SCSI_DH_NOSYS;

[PATCH 3/3] dm mpath: add support for using previously attached scsi_dh

[Mike Snitzer]

Use the previously attached scsi_dh if "default" is provided as the
hardware handler name (the new 'use_default_hw_handler' flag is set in
the multipath structure).

Leverage scsi_dh_attach's ability to increment the scsi_dh's reference
count if the same scsi_dh name is provided when attaching -- currently
attached scsi_dh name is determined with scsi_dh_attached_handler_name.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-mpath.c |   23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-mpath.c
+++ linux-2.6/drivers/md/dm-mpath.c
@@ -63,6 +63,7 @@ struct multipath {
 
 	spinlock_t lock;
 
+	unsigned use_default_hw_handler:1;
 	const char *hw_handler_name;
 	char *hw_handler_params;
 
@@ -566,6 +567,7 @@ static struct pgpath *parse_path(struct 
 	int r;
 	struct pgpath *p;
 	struct multipath *m = ti->private;
+	struct request_queue *q = NULL;
 
 	/* we need at least a path arg */
 	if (as->argc < 1) {
@@ -584,9 +586,16 @@ static struct pgpath *parse_path(struct 
 		goto bad;
 	}
 
-	if (m->hw_handler_name) {
-		struct request_queue *q = bdev_get_queue(p->path.dev->bdev);
+	if (m->use_default_hw_handler || m->hw_handler_name)
+		q = bdev_get_queue(p->path.dev->bdev);
+
+	if (m->use_default_hw_handler) {
+		const char *attached_handler_name = scsi_dh_attached_handler_name(q);
+		if (attached_handler_name)
+			m->hw_handler_name = kstrdup(attached_handler_name, GFP_KERNEL);
+	}
 
+	if (m->hw_handler_name) {
 		r = scsi_dh_attach(q, m->hw_handler_name);
 		if (r == -EBUSY) {
 			/*
@@ -706,6 +715,7 @@ static int parse_hw_handler(struct dm_ar
 	unsigned hw_argc;
 	int ret;
 	struct dm_target *ti = m->ti;
+	const char *hw_handler_name;
 
 	static struct dm_arg _args[] = {
 		{0, 1024, "invalid number of hardware handler args"},
@@ -717,7 +727,13 @@ static int parse_hw_handler(struct dm_ar
 	if (!hw_argc)
 		return 0;
 
-	m->hw_handler_name = kstrdup(dm_shift_arg(as), GFP_KERNEL);
+	hw_handler_name = dm_shift_arg(as);
+	if (!strncmp(hw_handler_name, "default", 8)) {
+		m->use_default_hw_handler = 1;
+		goto parse_hw_handler_params;
+	}
+
+	m->hw_handler_name = kstrdup(hw_handler_name, GFP_KERNEL);
 	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";
@@ -725,6 +741,7 @@ static int parse_hw_handler(struct dm_ar
 		goto fail;
 	}
 
+parse_hw_handler_params:
 	if (hw_argc > 1) {
 		char *p;
 		int i, j, len = 4;

Re: [PATCH 3/3] dm mpath: add support for using previously attached scsi_dh

[Hannes Reinecke]

On 05/08/2012 05:19 AM, Mike Snitzer wrote:
> Use the previously attached scsi_dh if "default" is provided as the
> hardware handler name (the new 'use_default_hw_handler' flag is set in
> the multipath structure).
> 
> Leverage scsi_dh_attach's ability to increment the scsi_dh's reference
> count if the same scsi_dh name is provided when attaching -- currently
> attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> 
Not sure it that's the correct way.

Using 'default' as the hardware handler name has the drawback that
the original information (ie the original hardware handler name)
is lost.

So if there is no 'default', eg if the kernel module hasn't been
loaded, we wouldn't know which hardware handler to load.

I'd rather have a feature 'default_hw_handler' which then could be
passed into the kernel module. With that we would retain both, the
original implementation and the default case.

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/3] dm mpath: add support for using previously attached scsi_dh

[Mike Snitzer]

On Tue, May 08 2012 at  2:22am -0400,
Hannes Reinecke <hare@suse.de> wrote:

> On 05/08/2012 05:19 AM, Mike Snitzer wrote:
> > Use the previously attached scsi_dh if "default" is provided as the
> > hardware handler name (the new 'use_default_hw_handler' flag is set in
> > the multipath structure).
> > 
> > Leverage scsi_dh_attach's ability to increment the scsi_dh's reference
> > count if the same scsi_dh name is provided when attaching -- currently
> > attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> > 
> Not sure it that's the correct way.
> 
> Using 'default' as the hardware handler name has the drawback that
> the original information (ie the original hardware handler name)
> is lost.
> 
> So if there is no 'default', eg if the kernel module hasn't been
> loaded, we wouldn't know which hardware handler to load.

What I've done does depend on all relevant scsi_dh being available
very early (either as builtins or loaded before the HBA drivers) so that
the scsi_dh gets attached during the drivers' SCSI device scan.

> I'd rather have a feature 'default_hw_handler' which then could be
> passed into the kernel module. With that we would retain both, the
> original implementation and the default case.

My change retains the original and allows "default" to have multipath
just use what the kernel already detected and attached via .match

Please be more precise with what it is you're proposing (either with
description or working patches).

Thanks!
Mike

Re: [PATCH 3/3] dm mpath: add support for using previously attached scsi_dh

[Mike Snitzer]

On Tue, May 08 2012 at  2:22am -0400,
Hannes Reinecke <hare@suse.de> wrote:

> On 05/08/2012 05:19 AM, Mike Snitzer wrote:
> > Use the previously attached scsi_dh if "default" is provided as the
> > hardware handler name (the new 'use_default_hw_handler' flag is set in
> > the multipath structure).
> > 
> > Leverage scsi_dh_attach's ability to increment the scsi_dh's reference
> > count if the same scsi_dh name is provided when attaching -- currently
> > attached scsi_dh name is determined with scsi_dh_attached_handler_name.
> > 
> Not sure it that's the correct way.
> 
> Using 'default' as the hardware handler name has the drawback that
> the original information (ie the original hardware handler name)
> is lost.

BTW, on this point: How is the original handler's name lost with my
patches?  I set m->hw_handler_name to the original (aka attached)
handler's name (as returned scsi_dh_attached_handler_name).

"default" is only used as a key to trigger the use of the attached
handler (it causes m->use_default_hw_handler to be set).

Was my description or code somehow unclear?  Guess so... otherwise I'd
not be clarifying this... unfortunately I'm not seeing where I fell
short.

Mike

Re: [PATCH 3/3] dm mpath: add support for using previously attached scsi_dh

[Hannes Reinecke]

On 05/08/2012 04:05 PM, Mike Snitzer wrote:
> On Tue, May 08 2012 at  2:22am -0400,
> Hannes Reinecke<hare@suse.de>  wrote:
>
>> On 05/08/2012 05:19 AM, Mike Snitzer wrote:
>>> Use the previously attached scsi_dh if "default" is provided as the
>>> hardware handler name (the new 'use_default_hw_handler' flag is set in
>>> the multipath structure).
>>>
>>> Leverage scsi_dh_attach's ability to increment the scsi_dh's reference
>>> count if the same scsi_dh name is provided when attaching -- currently
>>> attached scsi_dh name is determined with scsi_dh_attached_handler_name.
>>>
>> Not sure it that's the correct way.
>>
>> Using 'default' as the hardware handler name has the drawback that
>> the original information (ie the original hardware handler name)
>> is lost.
>
> BTW, on this point: How is the original handler's name lost with my
> patches?  I set m->hw_handler_name to the original (aka attached)
> handler's name (as returned scsi_dh_attached_handler_name).
>
> "default" is only used as a key to trigger the use of the attached
> handler (it causes m->use_default_hw_handler to be set).
>
What I meant is that within multipath you can only specify one hardware 
handler. So if it's 'default' and no hardware handler is attached we 
wouldn't know what to look for.

> Was my description or code somehow unclear?  Guess so... otherwise I'd
> not be clarifying this... unfortunately I'm not seeing where I fell
> short.
>
Meanwhile I've sent the patchset, Please check that for details.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, 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