[PATCH v2 0/5] dm mpath: add 'default_hw_handler' and 'no_partitions' features

[PATCH v2 0/5] dm mpath: add 'default_hw_handler' and 'no_partitions' features

[Mike Snitzer]

I've included all related patches in this v2 post simply to eliminate
any confusion with all the patches that have been in-flight from
Hannes and I.  Patches 1 and 2 are unchanged from previous posts.

Patch 4 is the combination of the different approaches Hannes and I
took.  I pushed the 'no_partitions' feature patch (5) to the end of
the series because it seemed more natural.

Hannes Reinecke (2):
  dm mpath: add 'default_hw_handler' feature
  dm mpath: add ability to disable partition creation

Mike Snitzer (3):
  dm mpath: only try to load the scsi_dh module if the scsi_dh doesn't exist
  scsi_dh: add scsi_dh_attached_handler_name
  dm mpath: reduce size of multipath structure

 drivers/md/dm-mpath.c                 |   53 +++++++++++++++++++++++++-------
 drivers/scsi/device_handler/scsi_dh.c |   31 ++++++++++++++++++-
 include/scsi/scsi_dh.h                |    5 +++
 3 files changed, 75 insertions(+), 14 deletions(-)

-- 
1.7.4.4

[PATCH v2 1/5] dm mpath: only try to load the scsi_dh module if the scsi_dh doesn't exist

[Mike Snitzer]

There have been reports of multipath table loads hanging due to
__request_module hanging (for some unknown reason).

More often than not, the scsi_dh is already available and there is no
need to request_module().

Reported-by: Ben Marzinski <bmarzins@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-mpath.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 922a338..754f38f 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -718,8 +718,8 @@ 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);
-	request_module("scsi_dh_%s", m->hw_handler_name);
-	if (scsi_dh_handler_exist(m->hw_handler_name) == 0) {
+	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;
-- 
1.7.4.4

[PATCH v2 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

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(-)

diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
index 48e46f5..7497c78 100644
--- a/drivers/scsi/device_handler/scsi_dh.c
+++ b/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 *q)
 }
 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
 };
diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
index e3f2db2..94f502b 100644
--- a/include/scsi/scsi_dh.h
+++ b/include/scsi/scsi_dh.h
@@ -60,6 +60,7 @@ 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 *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 request_queue *q)
 {
 	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;
-- 
1.7.4.4

[PATCH v2 3/5] dm mpath: reduce size of multipath structure

[Mike Snitzer]

Move multipath structure's 'lock' and 'queue_size' members to eliminate
2 4-byte holes.  Also use a bit within a single unsigned int for each
existing flag (saves 8-bytes).  This allows future flags to be added
without each consuming an unsigned int.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-mpath.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 754f38f..c351607 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -61,11 +61,11 @@ struct multipath {
 	struct list_head list;
 	struct dm_target *ti;
 
-	spinlock_t lock;
-
 	const char *hw_handler_name;
 	char *hw_handler_params;
 
+	spinlock_t lock;
+
 	unsigned nr_priority_groups;
 	struct list_head priority_groups;
 
@@ -81,16 +81,17 @@ struct multipath {
 	struct priority_group *next_pg;	/* Switch to this PG if set */
 	unsigned repeat_count;		/* I/Os left before calling PS again */
 
-	unsigned queue_io;		/* Must we queue all I/O? */
-	unsigned queue_if_no_path;	/* Queue I/O if last path fails? */
-	unsigned saved_queue_if_no_path;/* Saved state during suspension */
+	unsigned queue_io:1;		/* Must we queue all I/O? */
+	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
+	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
+
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
 	unsigned pg_init_delay_msecs;	/* Number of msecs before pg_init retry */
 
+	unsigned queue_size;
 	struct work_struct process_queued_ios;
 	struct list_head queued_ios;
-	unsigned queue_size;
 
 	struct work_struct trigger_event;
 
-- 
1.7.4.4

[PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature

[Mike Snitzer]

From: Hannes Reinecke <hare@suse.de>

When specifying the feature 'default_hw_handler' multipath will be use
the currently attached hardware handler instead of trying to attach the
one specified during table load.  If no hardware handler is attached the
specified hardware handler will be used.

Leverages 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: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-mpath.c |   27 +++++++++++++++++++++++----
 1 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index c351607..0fc6849 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -84,6 +84,7 @@ struct multipath {
 	unsigned queue_io:1;		/* Must we queue all I/O? */
 	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
 	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
+	unsigned use_default_hw_handler:1; /* Use attached device handler */
 
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
@@ -567,6 +568,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 	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) {
@@ -585,9 +587,18 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
 		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) {
+			kfree(m->hw_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) {
 			/*
@@ -759,7 +770,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 5, "invalid number of feature args"},
+		{0, 6, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -780,6 +791,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "default_hw_handler")) {
+			m->use_default_hw_handler = 1;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1363,13 +1379,16 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 	else {
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
-			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
+			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
+			      m->use_default_hw_handler);
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
 			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
 		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
+		if (m->use_default_hw_handler)
+			DMEMIT("default_hw_handler ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.7.4.4

[PATCH v2 5/5] dm mpath: add ability to disable partition creation

[Mike Snitzer]

From: Hannes Reinecke <hare@suse.de>

When multipath devices are being used as disks for VM Guests any
partition scanning / setup should be done within the VM Guest, not from
host.  So we need to a mechanism to switch off partition scanning and
creation via kpartx.

The new 'no_partitions' feature serves as a notifier to kpartx to _not_
create partitions on these multipath devices.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-mpath.c |   13 +++++++++++--
 1 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 0fc6849..1039e7f 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -85,6 +85,7 @@ struct multipath {
 	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
 	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
 	unsigned use_default_hw_handler:1; /* Use attached device handler */
+	unsigned no_partitions:1;	/* Avoid partition scanning */
 
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
@@ -770,7 +771,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 6, "invalid number of feature args"},
+		{0, 7, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -796,6 +797,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "no_partitions")) {
+			m->no_partitions = 1;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1380,7 +1386,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
 			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
-			      m->use_default_hw_handler);
+			      m->use_default_hw_handler +
+			      m->no_partitions);
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
@@ -1389,6 +1396,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
 		if (m->use_default_hw_handler)
 			DMEMIT("default_hw_handler ");
+		if (m->no_partitions)
+			DMEMIT("no_partitions ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
-- 
1.7.4.4

Re: [PATCH v2 1/5] dm mpath: only try to load the scsi_dh module if the scsi_dh doesn't exist

[Hannes Reinecke]

On 05/08/2012 11:56 PM, Mike Snitzer wrote:
> There have been reports of multipath table loads hanging due to
> __request_module hanging (for some unknown reason).
> 
> More often than not, the scsi_dh is already available and there is no
> need to request_module().
> 
> Reported-by: Ben Marzinski <bmarzins@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 922a338..754f38f 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -718,8 +718,8 @@ 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);
> -	request_module("scsi_dh_%s", m->hw_handler_name);
> -	if (scsi_dh_handler_exist(m->hw_handler_name) == 0) {
> +	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;

Acked-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)

Re: [PATCH v2 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Hannes Reinecke]

On 05/08/2012 11:56 PM, Mike Snitzer wrote:
> 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(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh.c b/drivers/scsi/device_handler/scsi_dh.c
> index 48e46f5..7497c78 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/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 *q)
>  }
>  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
>  };
> diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
> index e3f2db2..94f502b 100644
> --- a/include/scsi/scsi_dh.h
> +++ b/include/scsi/scsi_dh.h
> @@ -60,6 +60,7 @@ 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 *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 request_queue *q)
>  {
>  	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;

Acked-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)

Re: [PATCH v2 3/5] dm mpath: reduce size of multipath structure

[Hannes Reinecke]

On 05/08/2012 11:56 PM, Mike Snitzer wrote:
> Move multipath structure's 'lock' and 'queue_size' members to eliminate
> 2 4-byte holes.  Also use a bit within a single unsigned int for each
> existing flag (saves 8-bytes).  This allows future flags to be added
> without each consuming an unsigned int.
> 
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |   13 +++++++------
>  1 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 754f38f..c351607 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -61,11 +61,11 @@ struct multipath {
>  	struct list_head list;
>  	struct dm_target *ti;
>  
> -	spinlock_t lock;
> -
>  	const char *hw_handler_name;
>  	char *hw_handler_params;
>  
> +	spinlock_t lock;
> +
>  	unsigned nr_priority_groups;
>  	struct list_head priority_groups;
>  
> @@ -81,16 +81,17 @@ struct multipath {
>  	struct priority_group *next_pg;	/* Switch to this PG if set */
>  	unsigned repeat_count;		/* I/Os left before calling PS again */
>  
> -	unsigned queue_io;		/* Must we queue all I/O? */
> -	unsigned queue_if_no_path;	/* Queue I/O if last path fails? */
> -	unsigned saved_queue_if_no_path;/* Saved state during suspension */
> +	unsigned queue_io:1;		/* Must we queue all I/O? */
> +	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
> +	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
> +
>  	unsigned pg_init_retries;	/* Number of times to retry pg_init */
>  	unsigned pg_init_count;		/* Number of times pg_init called */
>  	unsigned pg_init_delay_msecs;	/* Number of msecs before pg_init retry */
>  
> +	unsigned queue_size;
>  	struct work_struct process_queued_ios;
>  	struct list_head queued_ios;
> -	unsigned queue_size;
>  
>  	struct work_struct trigger_event;
>  

Acked-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)

Re: [PATCH v2 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Moger, Babu]

Mike, I have a question for you below.

> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Tuesday, May 08, 2012 4:56 PM
> To: dm-devel@redhat.com
> Cc: agk@redhat.com; hare@suse.de; Moger, Babu; sekharan@us.ibm.com;
> Mike Snitzer
> Subject: [PATCH v2 2/5] scsi_dh: add scsi_dh_attached_handler_name
> 
> 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(-)
> 
> diff --git a/drivers/scsi/device_handler/scsi_dh.c
> b/drivers/scsi/device_handler/scsi_dh.c
> index 48e46f5..7497c78 100644
> --- a/drivers/scsi/device_handler/scsi_dh.c
> +++ b/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 *q)
>  }
>  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;
> +}


I only see get_device call.  I don't see matching put_device call.  Was  that on purspose?


> +EXPORT_SYMBOL_GPL(scsi_dh_attached_handler_name);
> +
>  static struct notifier_block scsi_dh_nb = {
>  	.notifier_call = scsi_dh_notifier
>  };
> diff --git a/include/scsi/scsi_dh.h b/include/scsi/scsi_dh.h
> index e3f2db2..94f502b 100644
> --- a/include/scsi/scsi_dh.h
> +++ b/include/scsi/scsi_dh.h
> @@ -60,6 +60,7 @@ 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
> *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 request_queue
> *q)
>  {
>  	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;
> --
> 1.7.4.4

Re: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature

[Moger, Babu]

Mike,

> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Tuesday, May 08, 2012 4:56 PM
> To: dm-devel@redhat.com
> Cc: agk@redhat.com; hare@suse.de; Moger, Babu; sekharan@us.ibm.com;
> Mike Snitzer
> Subject: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature
> 
> From: Hannes Reinecke <hare@suse.de>
> 
> When specifying the feature 'default_hw_handler' multipath will be use
> the currently attached hardware handler instead of trying to attach the
> one specified during table load.  If no hardware handler is attached the
> specified hardware handler will be used.

I am trying to test these patches right now.  What is the expectation from
Multipath tools for this to work correctly.  Do I have to pass following 
Parameters?

hardware_handler "1 alua"  
features "1 default_hw_handler"
 
It appears to me that the first line will forcibly load the alua handler even if
the default handler is different. 

> 
> Leverages 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: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |   27 +++++++++++++++++++++++----
>  1 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index c351607..0fc6849 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -84,6 +84,7 @@ struct multipath {
>  	unsigned queue_io:1;		/* Must we queue all I/O? */
>  	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
>  	unsigned saved_queue_if_no_path:1; /* Saved state during
> suspension */
> +	unsigned use_default_hw_handler:1; /* Use attached device
> handler */
> 
>  	unsigned pg_init_retries;	/* Number of times to retry pg_init
> */
>  	unsigned pg_init_count;		/* Number of times pg_init
> called */
> @@ -567,6 +568,7 @@ static struct pgpath *parse_path(struct dm_arg_set
> *as, struct path_selector *ps
>  	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) {
> @@ -585,9 +587,18 @@ static struct pgpath *parse_path(struct dm_arg_set
> *as, struct path_selector *ps
>  		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) {
> +			kfree(m->hw_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) {
>  			/*
> @@ -759,7 +770,7 @@ static int parse_features(struct dm_arg_set *as,
> struct multipath *m)
>  	const char *arg_name;
> 
>  	static struct dm_arg _args[] = {
> -		{0, 5, "invalid number of feature args"},
> +		{0, 6, "invalid number of feature args"},
>  		{1, 50, "pg_init_retries must be between 1 and 50"},
>  		{0, 60000, "pg_init_delay_msecs must be between 0 and
> 60000"},
>  	};
> @@ -780,6 +791,11 @@ static int parse_features(struct dm_arg_set *as,
> struct multipath *m)
>  			continue;
>  		}
> 
> +		if (!strcasecmp(arg_name, "default_hw_handler")) {
> +			m->use_default_hw_handler = 1;
> +			continue;
> +		}
> +
>  		if (!strcasecmp(arg_name, "pg_init_retries") &&
>  		    (argc >= 1)) {
>  			r = dm_read_arg(_args + 1, as, &m->pg_init_retries,
> &ti->error);
> @@ -1363,13 +1379,16 @@ static int multipath_status(struct dm_target *ti,
> status_type_t type,
>  	else {
>  		DMEMIT("%u ", m->queue_if_no_path +
>  			      (m->pg_init_retries > 0) * 2 +
> -			      (m->pg_init_delay_msecs !=
> DM_PG_INIT_DELAY_DEFAULT) * 2);
> +			      (m->pg_init_delay_msecs !=
> DM_PG_INIT_DELAY_DEFAULT) * 2 +
> +			      m->use_default_hw_handler);
>  		if (m->queue_if_no_path)
>  			DMEMIT("queue_if_no_path ");
>  		if (m->pg_init_retries)
>  			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
>  		if (m->pg_init_delay_msecs !=
> DM_PG_INIT_DELAY_DEFAULT)
>  			DMEMIT("pg_init_delay_msecs %u ", m-
> >pg_init_delay_msecs);
> +		if (m->use_default_hw_handler)
> +			DMEMIT("default_hw_handler ");
>  	}
> 
>  	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
> --
> 1.7.4.4

Re: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature

[Mike Snitzer]

On Wed, May 09 2012 at  2:10pm -0400,
Moger, Babu <Babu.Moger@netapp.com> wrote:

> Mike,
> 
> > -----Original Message-----
> > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > Sent: Tuesday, May 08, 2012 4:56 PM
> > To: dm-devel@redhat.com
> > Cc: agk@redhat.com; hare@suse.de; Moger, Babu; sekharan@us.ibm.com;
> > Mike Snitzer
> > Subject: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature
> > 
> > From: Hannes Reinecke <hare@suse.de>
> > 
> > When specifying the feature 'default_hw_handler' multipath will be use
> > the currently attached hardware handler instead of trying to attach the
> > one specified during table load.  If no hardware handler is attached the
> > specified hardware handler will be used.
> 
> I am trying to test these patches right now.  What is the expectation from
> Multipath tools for this to work correctly.  Do I have to pass following 
> Parameters?
> 
> hardware_handler "1 alua"  
> features "1 default_hw_handler"

Yes.
  
> It appears to me that the first line will forcibly load the alua handler even if
> the default handler is different. 

Are you saying that based on testing or based on code review?

> > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> > index c351607..0fc6849 100644
> > --- a/drivers/md/dm-mpath.c
> > +++ b/drivers/md/dm-mpath.c
> > @@ -585,9 +587,18 @@ static struct pgpath *parse_path(struct dm_arg_set
> > *as, struct path_selector *ps
> >  		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) {
> > +			kfree(m->hw_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) {
> >  			/*

The above hunk is what will cause the currently attached device handler
to be used.  But if a device handler is _not_ attached then we fallback
to using the provided 'hardware_handler'.

So are you testing this patch with a device handler having already been
attached?

Mike

Re: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature

[Moger, Babu]

> -----Original Message-----
> From: Mike Snitzer [mailto:snitzer@redhat.com]
> Sent: Wednesday, May 09, 2012 2:17 PM
> To: Moger, Babu
> Cc: dm-devel@redhat.com; agk@redhat.com; hare@suse.de;
> sekharan@us.ibm.com
> Subject: Re: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature
> 
> On Wed, May 09 2012 at  2:10pm -0400,
> Moger, Babu <Babu.Moger@netapp.com> wrote:
> 
> > Mike,
> >
> > > -----Original Message-----
> > > From: Mike Snitzer [mailto:snitzer@redhat.com]
> > > Sent: Tuesday, May 08, 2012 4:56 PM
> > > To: dm-devel@redhat.com
> > > Cc: agk@redhat.com; hare@suse.de; Moger, Babu;
> sekharan@us.ibm.com;
> > > Mike Snitzer
> > > Subject: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature
> > >
> > > From: Hannes Reinecke <hare@suse.de>
> > >
> > > When specifying the feature 'default_hw_handler' multipath will be use
> > > the currently attached hardware handler instead of trying to attach the
> > > one specified during table load.  If no hardware handler is attached the
> > > specified hardware handler will be used.
> >
> > I am trying to test these patches right now.  What is the expectation from
> > Multipath tools for this to work correctly.  Do I have to pass following
> > Parameters?
> >
> > hardware_handler "1 alua"
> > features "1 default_hw_handler"
> 
> Yes.
Ok. thanks

> 
> > It appears to me that the first line will forcibly load the alua handler even if
> > the default handler is different.
> 
> Are you saying that based on testing or based on code review?

I was only reviewing the code.  Looking at the code again, It should work fine.
I am going to test it anyway.

> 
> > > diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> > > index c351607..0fc6849 100644
> > > --- a/drivers/md/dm-mpath.c
> > > +++ b/drivers/md/dm-mpath.c
> > > @@ -585,9 +587,18 @@ static struct pgpath *parse_path(struct
> dm_arg_set
> > > *as, struct path_selector *ps
> > >  		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) {
> > > +			kfree(m->hw_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) {
> > >  			/*
> 
> The above hunk is what will cause the currently attached device handler
> to be used.  But if a device handler is _not_ attached then we fallback
> to using the provided 'hardware_handler'.

Got it.
> 
> So are you testing this patch with a device handler having already been
> attached?

Yes. I will provide the feedback only tomorrow.

[PATCH v3 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

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 |   36 ++++++++++++++++++++++++++++++++--
 include/scsi/scsi_dh.h                |    5 ++++
 2 files changed, 39 insertions(+), 2 deletions(-)

v3: fixed missing put_device that Babu pointed out

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,36 @@ 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 handler, NULL if no handler is attached.
+ */
+const char *scsi_dh_attached_handler_name(struct request_queue *q)
+{
+	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);
+
+	if (!sdev)
+		return NULL;
+
+	if (sdev->scsi_dh_data)
+		handler_name = sdev->scsi_dh_data->scsi_dh->name;
+
+	put_device(&sdev->sdev_gendev);
+	return handler_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 v3 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Mike Snitzer]

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 |   36 ++++++++++++++++++++++++++++++++--
 include/scsi/scsi_dh.h                |    5 ++++
 2 files changed, 39 insertions(+), 2 deletions(-)

v3: fixed missing put_device that Babu pointed out

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,36 @@ 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 handler, NULL if no handler is attached.
+ */
+const char *scsi_dh_attached_handler_name(struct request_queue *q)
+{
+	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);
+
+	if (!sdev)
+		return NULL;
+
+	if (sdev->scsi_dh_data)
+		handler_name = sdev->scsi_dh_data->scsi_dh->name;
+
+	put_device(&sdev->sdev_gendev);
+	return handler_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;

Re: [PATCH v2 1/5] dm mpath: only try to load the scsi_dh module if the scsi_dh doesn't exist

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

On Tue, 2012-05-08 at 17:56 -0400, Mike Snitzer wrote:
> There have been reports of multipath table loads hanging due to
> __request_module hanging (for some unknown reason).
> 
> More often than not, the scsi_dh is already available and there is no
> need to request_module().
> 
> Reported-by: Ben Marzinski <bmarzins@redhat.com>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 922a338..754f38f 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -718,8 +718,8 @@ 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);
> -	request_module("scsi_dh_%s", m->hw_handler_name);
> -	if (scsi_dh_handler_exist(m->hw_handler_name) == 0) {
> +	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;

Re: [PATCH v3 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

Mike, you may have to cross post to linux-scsi for James to pick it up.

On Wed, 2012-05-09 at 16:41 -0400, Mike Snitzer wrote:
> 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 |   36 ++++++++++++++++++++++++++++++++--
>  include/scsi/scsi_dh.h                |    5 ++++
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> v3: fixed missing put_device that Babu pointed out
> 
> 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,36 @@ 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 handler, NULL if no handler is attached.
> + */
> +const char *scsi_dh_attached_handler_name(struct request_queue *q)
> +{
> +	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);
> +
> +	if (!sdev)
> +		return NULL;
> +
> +	if (sdev->scsi_dh_data)
> +		handler_name = sdev->scsi_dh_data->scsi_dh->name;
> +
> +	put_device(&sdev->sdev_gendev);
> +	return handler_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;
> 

Re: [PATCH v3 2/5] scsi_dh: add scsi_dh_attached_handler_name

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

Mike, you may have to cross post to linux-scsi for James to pick it up.

On Wed, 2012-05-09 at 16:41 -0400, Mike Snitzer wrote:
> 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 |   36 ++++++++++++++++++++++++++++++++--
>  include/scsi/scsi_dh.h                |    5 ++++
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> v3: fixed missing put_device that Babu pointed out
> 
> 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,36 @@ 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 handler, NULL if no handler is attached.
> + */
> +const char *scsi_dh_attached_handler_name(struct request_queue *q)
> +{
> +	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);
> +
> +	if (!sdev)
> +		return NULL;
> +
> +	if (sdev->scsi_dh_data)
> +		handler_name = sdev->scsi_dh_data->scsi_dh->name;
> +
> +	put_device(&sdev->sdev_gendev);
> +	return handler_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;
> 

Re: [PATCH v2 4/5] dm mpath: add 'default_hw_handler' feature

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

On Tue, 2012-05-08 at 17:56 -0400, Mike Snitzer wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> When specifying the feature 'default_hw_handler' multipath will be use
> the currently attached hardware handler instead of trying to attach the
> one specified during table load.  If no hardware handler is attached the
> specified hardware handler will be used.
> 
> Leverages 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: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |   27 +++++++++++++++++++++++----
>  1 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index c351607..0fc6849 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -84,6 +84,7 @@ struct multipath {
>  	unsigned queue_io:1;		/* Must we queue all I/O? */
>  	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
>  	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
> +	unsigned use_default_hw_handler:1; /* Use attached device handler */
> 
>  	unsigned pg_init_retries;	/* Number of times to retry pg_init */
>  	unsigned pg_init_count;		/* Number of times pg_init called */
> @@ -567,6 +568,7 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>  	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) {
> @@ -585,9 +587,18 @@ static struct pgpath *parse_path(struct dm_arg_set *as, struct path_selector *ps
>  		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) {
> +			kfree(m->hw_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) {
>  			/*
> @@ -759,7 +770,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
>  	const char *arg_name;
> 
>  	static struct dm_arg _args[] = {
> -		{0, 5, "invalid number of feature args"},
> +		{0, 6, "invalid number of feature args"},
>  		{1, 50, "pg_init_retries must be between 1 and 50"},
>  		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
>  	};
> @@ -780,6 +791,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
>  			continue;
>  		}
> 
> +		if (!strcasecmp(arg_name, "default_hw_handler")) {
> +			m->use_default_hw_handler = 1;
> +			continue;
> +		}
> +
>  		if (!strcasecmp(arg_name, "pg_init_retries") &&
>  		    (argc >= 1)) {
>  			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
> @@ -1363,13 +1379,16 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
>  	else {
>  		DMEMIT("%u ", m->queue_if_no_path +
>  			      (m->pg_init_retries > 0) * 2 +
> -			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
> +			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
> +			      m->use_default_hw_handler);
>  		if (m->queue_if_no_path)
>  			DMEMIT("queue_if_no_path ");
>  		if (m->pg_init_retries)
>  			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
>  		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
>  			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
> +		if (m->use_default_hw_handler)
> +			DMEMIT("default_hw_handler ");
>  	}
> 
>  	if (!m->hw_handler_name || type == STATUSTYPE_INFO)

Re: [PATCH v2 5/5] dm mpath: add ability to disable partition creation

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>


On Tue, 2012-05-08 at 17:56 -0400, Mike Snitzer wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> When multipath devices are being used as disks for VM Guests any
> partition scanning / setup should be done within the VM Guest, not from
> host.  So we need to a mechanism to switch off partition scanning and
> creation via kpartx.
> 
> The new 'no_partitions' feature serves as a notifier to kpartx to _not_
> create partitions on these multipath devices.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Mike Snitzer <snitzer@redhat.com>
> ---
>  drivers/md/dm-mpath.c |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
> index 0fc6849..1039e7f 100644
> --- a/drivers/md/dm-mpath.c
> +++ b/drivers/md/dm-mpath.c
> @@ -85,6 +85,7 @@ struct multipath {
>  	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
>  	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
>  	unsigned use_default_hw_handler:1; /* Use attached device handler */
> +	unsigned no_partitions:1;	/* Avoid partition scanning */
> 
>  	unsigned pg_init_retries;	/* Number of times to retry pg_init */
>  	unsigned pg_init_count;		/* Number of times pg_init called */
> @@ -770,7 +771,7 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
>  	const char *arg_name;
> 
>  	static struct dm_arg _args[] = {
> -		{0, 6, "invalid number of feature args"},
> +		{0, 7, "invalid number of feature args"},
>  		{1, 50, "pg_init_retries must be between 1 and 50"},
>  		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
>  	};
> @@ -796,6 +797,11 @@ static int parse_features(struct dm_arg_set *as, struct multipath *m)
>  			continue;
>  		}
> 
> +		if (!strcasecmp(arg_name, "no_partitions")) {
> +			m->no_partitions = 1;
> +			continue;
> +		}
> +
>  		if (!strcasecmp(arg_name, "pg_init_retries") &&
>  		    (argc >= 1)) {
>  			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
> @@ -1380,7 +1386,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
>  		DMEMIT("%u ", m->queue_if_no_path +
>  			      (m->pg_init_retries > 0) * 2 +
>  			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
> -			      m->use_default_hw_handler);
> +			      m->use_default_hw_handler +
> +			      m->no_partitions);
>  		if (m->queue_if_no_path)
>  			DMEMIT("queue_if_no_path ");
>  		if (m->pg_init_retries)
> @@ -1389,6 +1396,8 @@ static int multipath_status(struct dm_target *ti, status_type_t type,
>  			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
>  		if (m->use_default_hw_handler)
>  			DMEMIT("default_hw_handler ");
> +		if (m->no_partitions)
> +			DMEMIT("no_partitions ");
>  	}
> 
>  	if (!m->hw_handler_name || type == STATUSTYPE_INFO)

[PATCH v3 4/5] dm mpath: add 'default_hw_handler' feature

[Mike Snitzer]

When specifying the feature 'default_hw_handler' multipath will use
the currently attached hardware handler instead of trying to attach the
one specified during table load.  If no hardware handler is attached the
specified hardware handler will be used.

Leverages 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>
Tested-by: Babu Moger <babu.moger@netapp.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Chandra Seetharaman <sekharan@us.ibm.com>
---
 drivers/md/dm-mpath.c |   28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

v3: updated to not kstrdup since scsi_dh_attached_handler_name now does

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
@@ -84,6 +84,7 @@ struct multipath {
 	unsigned queue_io:1;		/* Must we queue all I/O? */
 	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
 	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
+	unsigned use_default_hw_handler:1; /* Use attached device handler */
 
 	unsigned pg_init_retries;	/* Number of times to retry pg_init */
 	unsigned pg_init_count;		/* Number of times pg_init called */
@@ -567,6 +568,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) {
@@ -585,9 +587,19 @@ 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, GFP_KERNEL);
+		if (attached_handler_name) {
+			kfree(m->hw_handler_name);
+			m->hw_handler_name = attached_handler_name;
+		}
+	}
 
+	if (m->hw_handler_name) {
 		r = scsi_dh_attach(q, m->hw_handler_name);
 		if (r == -EBUSY) {
 			/*
@@ -759,7 +771,7 @@ static int parse_features(struct dm_arg_
 	const char *arg_name;
 
 	static struct dm_arg _args[] = {
-		{0, 5, "invalid number of feature args"},
+		{0, 6, "invalid number of feature args"},
 		{1, 50, "pg_init_retries must be between 1 and 50"},
 		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
 	};
@@ -780,6 +792,11 @@ static int parse_features(struct dm_arg_
 			continue;
 		}
 
+		if (!strcasecmp(arg_name, "default_hw_handler")) {
+			m->use_default_hw_handler = 1;
+			continue;
+		}
+
 		if (!strcasecmp(arg_name, "pg_init_retries") &&
 		    (argc >= 1)) {
 			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
@@ -1363,13 +1380,16 @@ static int multipath_status(struct dm_ta
 	else {
 		DMEMIT("%u ", m->queue_if_no_path +
 			      (m->pg_init_retries > 0) * 2 +
-			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
+			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
+			      m->use_default_hw_handler);
 		if (m->queue_if_no_path)
 			DMEMIT("queue_if_no_path ");
 		if (m->pg_init_retries)
 			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
 		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
 			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
+		if (m->use_default_hw_handler)
+			DMEMIT("default_hw_handler ");
 	}
 
 	if (!m->hw_handler_name || type == STATUSTYPE_INFO)

Re: [PATCH v3 4/5] dm mpath: add 'default_hw_handler' feature

[Chandra Seetharaman]

Reviewed-by: Chandra Seetharaman <sekharan@us.ibm.com>

On Thu, 2012-05-10 at 17:31 -0400, Mike Snitzer wrote:
> When specifying the feature 'default_hw_handler' multipath will use
> the currently attached hardware handler instead of trying to attach the
> one specified during table load.  If no hardware handler is attached the
> specified hardware handler will be used.
> 
> Leverages 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>
> Tested-by: Babu Moger <babu.moger@netapp.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Chandra Seetharaman <sekharan@us.ibm.com>
> ---
>  drivers/md/dm-mpath.c |   28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> v3: updated to not kstrdup since scsi_dh_attached_handler_name now does
> 
> 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
> @@ -84,6 +84,7 @@ struct multipath {
>  	unsigned queue_io:1;		/* Must we queue all I/O? */
>  	unsigned queue_if_no_path:1;	/* Queue I/O if last path fails? */
>  	unsigned saved_queue_if_no_path:1; /* Saved state during suspension */
> +	unsigned use_default_hw_handler:1; /* Use attached device handler */
> 
>  	unsigned pg_init_retries;	/* Number of times to retry pg_init */
>  	unsigned pg_init_count;		/* Number of times pg_init called */
> @@ -567,6 +568,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) {
> @@ -585,9 +587,19 @@ 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, GFP_KERNEL);
> +		if (attached_handler_name) {
> +			kfree(m->hw_handler_name);
> +			m->hw_handler_name = attached_handler_name;
> +		}
> +	}
> 
> +	if (m->hw_handler_name) {
>  		r = scsi_dh_attach(q, m->hw_handler_name);
>  		if (r == -EBUSY) {
>  			/*
> @@ -759,7 +771,7 @@ static int parse_features(struct dm_arg_
>  	const char *arg_name;
> 
>  	static struct dm_arg _args[] = {
> -		{0, 5, "invalid number of feature args"},
> +		{0, 6, "invalid number of feature args"},
>  		{1, 50, "pg_init_retries must be between 1 and 50"},
>  		{0, 60000, "pg_init_delay_msecs must be between 0 and 60000"},
>  	};
> @@ -780,6 +792,11 @@ static int parse_features(struct dm_arg_
>  			continue;
>  		}
> 
> +		if (!strcasecmp(arg_name, "default_hw_handler")) {
> +			m->use_default_hw_handler = 1;
> +			continue;
> +		}
> +
>  		if (!strcasecmp(arg_name, "pg_init_retries") &&
>  		    (argc >= 1)) {
>  			r = dm_read_arg(_args + 1, as, &m->pg_init_retries, &ti->error);
> @@ -1363,13 +1380,16 @@ static int multipath_status(struct dm_ta
>  	else {
>  		DMEMIT("%u ", m->queue_if_no_path +
>  			      (m->pg_init_retries > 0) * 2 +
> -			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2);
> +			      (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT) * 2 +
> +			      m->use_default_hw_handler);
>  		if (m->queue_if_no_path)
>  			DMEMIT("queue_if_no_path ");
>  		if (m->pg_init_retries)
>  			DMEMIT("pg_init_retries %u ", m->pg_init_retries);
>  		if (m->pg_init_delay_msecs != DM_PG_INIT_DELAY_DEFAULT)
>  			DMEMIT("pg_init_delay_msecs %u ", m->pg_init_delay_msecs);
> +		if (m->use_default_hw_handler)
> +			DMEMIT("default_hw_handler ");
>  	}
> 
>  	if (!m->hw_handler_name || type == STATUSTYPE_INFO)
>