scsi_dh: Make scsi_dh_activate asynchronous

PATCH In review Chandra Seetharaman 4 participants 10 messages Oct 21, 2009 – Oct 22, 2009

Subsystems (2)

SCSI Device mapper

lore

Share & tools

lore

Chandra Seetharaman

author

2009-10-21 16:22 UTC #0 lore raw

cover scsi_dh: Make scsi_dh_activate asynchronous

Hello All,

Currently, device handlers process path activation in series. This leads to a lot of time delay when more than 100 luns are involved. For example, with lsi rdac 100+ luns take about 12-15 minutes. This was found by Moger Babu of LSI.

This time delay can be avoided if we can do the activations asynchronously. By making scsi_dh_activate() async, we can give the device handlers an oppurtunity to decide on how to send the device activation down the wire to make the turn around time faster. They can send the commands asynchronously or send them in batches.

I brought up this issue on the mailing list
(http://marc.info/?l=linux-scsi&m=123888063818755&w=2) and provided a set of patch a loooong while back http://marc.info/?l=linux-scsi&m=124088712709821&w=2

I posted another set of patches against 2.6.31 on 2009/09/29:
http://marc.info/?l=linux-scsi&m=125427648406736&w=2

This set of patches applies cleanly on 2.6.32-rc5 and I tested the rdac handler and alua handler on the same.

Please review and provide comments.

This set of patched adds asynchronous support for rdac, hp, and alua handlers.

Didn't add async support for EMC for the following reasons:
- EMC handler is getting more hairy (when I try to move to _nowait())
- I do not know if EMC has a special command to group them (like rdac).
- I wasn't sure if EMC has this problem in the first place ( I did not get any response from EMC).

Can somebody from EMC comment, please.

Thanks & Regards,

chandra

Chandra Seetharaman

author

2009-10-21 16:22 UTC #1 lore raw

PATCH 1/4 scsi_dh: Change the scsidh_activate interface to be asynchronous

Make scsi_dh_activate() function asynchronous, by taking in two additional parameters, one is the callback function and the other is the data to call the callback function with.

Signed-off-by Chandra Seetharaman <[email protected]>

--------------

 drivers/md/dm-mpath.c                       |    8 ++++----
 drivers/scsi/device_handler/scsi_dh.c       |   17 ++++++++++++-----
 drivers/scsi/device_handler/scsi_dh_alua.c  |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_emc.c   |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_rdac.c  |    7 +++++--
 include/scsi/scsi_device.h                  |    3 ++-
 include/scsi/scsi_dh.h                      |    6 ++++--
 8 files changed, 42 insertions(+), 20 deletions(-)

include/scsi/scsi_dh.h +4 -2

Index: linux-2.6.32-rc5/include/scsi/scsi_dh.h
===================================================================
@@ -56,14 +56,16 @@
 	SCSI_DH_DRIVER_MAX,
 };
 #if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE)
-extern int scsi_dh_activate(struct request_queue *);
+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 int scsi_dh_set_params(struct request_queue *, const char *);
 #else
-static inline int scsi_dh_activate(struct request_queue *req)
+static inline int scsi_dh_activate(struct request_queue *req,
+					activate_complete fn, void *data)
 {
+	fn(data, 0);
 	return 0;
 }
 static inline int scsi_dh_handler_exist(const char *name)

drivers/scsi/device_handler/scsi_dh.c +12 -5

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh.c
===================================================================
@@ -226,7 +226,7 @@
 			 * Activate a device handler
 			 */
 			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev);
+				err = scsi_dh->activate(sdev, NULL, NULL);
 			else
 				err = 0;
 		}

@@ -423,10 +423,17 @@
 /*
  * scsi_dh_activate - activate the path associated with the scsi_device
  *      corresponding to the given request queue.
- * @q - Request queue that is associated with the scsi_device to be
- *      activated.
+ *     Returns immediately without waiting for activation to be completed.
+ * @q    - Request queue that is associated with the scsi_device to be
+ *         activated.
+ * @fn   - Function to be called upon completion of the activation.
+ *         Function fn is called with data (below) and the error code.
+ *         Function fn may be called from the same calling context. So,
+ *         do not hold the lock in the caller which may be needed in fn.
+ * @data - data passed to the function fn upon completion.
+ *
  */
-int scsi_dh_activate(struct request_queue *q)
+int scsi_dh_activate(struct request_queue *q, activate_complete fn, void *data)
 {
 	int err = 0;
 	unsigned long flags;

@@ -445,7 +452,7 @@
 		return err;
 
 	if (scsi_dh->activate)
-		err = scsi_dh->activate(sdev);
+		err = scsi_dh->activate(sdev, fn, data);
 	put_device(&sdev->sdev_gendev);
 	return err;
 }

drivers/scsi/device_handler/scsi_dh_rdac.c +5 -2

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
@@ -568,7 +568,8 @@
 	return err;
 }
 
-static int rdac_activate(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);
 	int err = SCSI_DH_OK;

@@ -580,7 +581,9 @@
 	if (h->lun_state == RDAC_LUN_UNOWNED)
 		err = send_mode_select(sdev, h);
 done:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)

drivers/scsi/device_handler/scsi_dh_alua.c +5 -2

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
@@ -652,7 +652,8 @@
  * based on a certain policy. But until we actually encounter them it
  * should be okay.
  */
-static int alua_activate(struct scsi_device *sdev)
+static int alua_activate(struct scsi_device *sdev,
+			activate_complete fn, void *data)
 {
 	struct alua_dh_data *h = get_alua_data(sdev);
 	int err = SCSI_DH_OK;

@@ -667,7 +668,9 @@
 		err = alua_stpg(sdev, TPGS_STATE_OPTIMIZED, h);
 
 out:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 /*

drivers/scsi/device_handler/scsi_dh_emc.c +5 -2

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
@@ -528,7 +528,8 @@
 	return err;
 }
 
-static int clariion_activate(struct scsi_device *sdev)
+static int clariion_activate(struct scsi_device *sdev,
+				activate_complete fn, void *data)
 {
 	struct clariion_dh_data *csdev = get_clariion_data(sdev);
 	int result;

@@ -559,7 +560,9 @@
 		    csdev->port, lun_state[csdev->lun_state],
 		    csdev->default_sp + 'A');
 
-	return result;
+	if (fn)
+		fn(data, result);
+	return 0;
 }
 /*
  * params - parameters in the following format

drivers/scsi/device_handler/scsi_dh_hp_sw.c +5 -2

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================
@@ -268,7 +268,8 @@
  * activate the passive path (and deactivate the
  * previously active one).
  */
-static int hp_sw_activate(struct scsi_device *sdev)
+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);

@@ -283,7 +284,9 @@
 				    HP_SW_NAME);
 	}
 
-	return ret;
+	if (fn)
+		fn(data, ret);
+	return 0;
 }
 
 static const struct scsi_dh_devlist hp_sw_dh_data_list[] = {

include/scsi/scsi_device.h +2 -1

Index: linux-2.6.32-rc5/include/scsi/scsi_device.h
===================================================================
@@ -174,6 +174,7 @@
 	char *model;
 };
 
+typedef void (*activate_complete)(void *, int);
 struct scsi_device_handler {
 	/* Used by the infrastructure */
 	struct list_head list; /* list of scsi_device_handlers */

@@ -185,7 +186,7 @@
 	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 *);
+	int (*activate)(struct scsi_device *, activate_complete, void *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
 };

drivers/md/dm-mpath.c +4 -4

Index: linux-2.6.32-rc5/drivers/md/dm-mpath.c
===================================================================
@@ -1116,8 +1116,9 @@
 	return limit_reached;
 }
 
-static void pg_init_done(struct dm_path *path, int errors)
+static void pg_init_done(void *data, int errors)
 {
+	struct dm_path *path = data;
 	struct pgpath *pgpath = path_to_pgpath(path);
 	struct priority_group *pg = pgpath->pg;
 	struct multipath *m = pg->m;

@@ -1183,12 +1184,11 @@
 
 static void activate_path(struct work_struct *work)
 {
-	int ret;
 	struct pgpath *pgpath =
 		container_of(work, struct pgpath, activate_path);
 
-	ret = scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev));
-	pg_init_done(&pgpath->path, ret);
+	scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev),
+				pg_init_done, &pgpath->path);
 }
 
 /*

Moger, Babu

Tested-by Babu on v1 1/4

2009-10-22 23:26 UTC #2 lore raw

RE: [PATCH 1/4] scsi_dh: Change the scsidh_activate interface to be asynchronous

I have tested this patch on LSI rdac storage. It works pretty well..

Quoted 11 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Wednesday, October 21, 2009 11:23 AM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 1/4] scsi_dh: Change the scsidh_activate interface to be asynchronous

Quoted 7 lines

Make scsi_dh_activate() function asynchronous, by taking in two additional
parameters, one is the callback function and the other is the data to call
the callback function with.
Signed-off-by Chandra Seetharaman <[email protected]>

Tested-by Babu [email protected]

Quoted 257 lines

--------------
 drivers/md/dm-mpath.c                       |    8 ++++----
 drivers/scsi/device_handler/scsi_dh.c       |   17 ++++++++++++-----
 drivers/scsi/device_handler/scsi_dh_alua.c  |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_emc.c   |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_rdac.c  |    7 +++++--
 include/scsi/scsi_device.h                  |    3 ++-
 include/scsi/scsi_dh.h                      |    6 ++++--
 8 files changed, 42 insertions(+), 20 deletions(-)
Index: linux-2.6.32-rc5/include/scsi/scsi_dh.h
===================================================================
--- linux-2.6.32-rc5.orig/include/scsi/scsi_dh.h 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/include/scsi/scsi_dh.h 2009-10-20
18:26:14.000000000 -0700
include/scsi/scsi_dh.h +4 -2
@@ -56,14 +56,16 @@
 	SCSI_DH_DRIVER_MAX,
 };
 #if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE)
-extern int scsi_dh_activate(struct request_queue *);
+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 int scsi_dh_set_params(struct request_queue *, const char *);
 #else
-static inline int scsi_dh_activate(struct request_queue *req)
+static inline int scsi_dh_activate(struct request_queue *req,
+					activate_complete fn, void *data)
 {
+	fn(data, 0);
 	return 0;
 }
static inline int scsi_dh_handler_exist(const char *name)
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh.c 2009-
10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh.c 2009-10-20
18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh.c +1 -1
@@ -226,7 +226,7 @@
 			 * Activate a device handler
 			 */
 			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev);
+				err = scsi_dh->activate(sdev, NULL, NULL);
 			else
 				err = 0;
 		}
@@ -423,10 +423,17 @@
 /*
  * scsi_dh_activate - activate the path associated with the
scsi_device
* corresponding to the given request queue.
- * @q - Request queue that is associated with the scsi_device to be
- * activated.
+ * Returns immediately without waiting for activation to be
completed.
+ * @q - Request queue that is associated with the scsi_device to be
+ * activated.
+ * @fn - Function to be called upon completion of the activation.
+ * Function fn is called with data (below) and the error code.
+ * Function fn may be called from the same calling context.
So,
+ * do not hold the lock in the caller which may be needed in
fn.
+ * @data - data passed to the function fn upon completion.
+ *
*/
(quoted diff) +1 -1
-int scsi_dh_activate(struct request_queue *q)
+int scsi_dh_activate(struct request_queue *q, activate_complete fn,
void *data)
{
drivers/scsi/device_handler/scsi_dh.c +1 -1
 	int err = 0;
 	unsigned long flags;
@@ -445,7 +452,7 @@
 		return err;
 
 	if (scsi_dh->activate)
-		err = scsi_dh->activate(sdev);
+		err = scsi_dh->activate(sdev, fn, data);
 	put_device(&sdev->sdev_gendev);
 	return err;
 }
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_rdac.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_rdac.c +5 -2
@@ -568,7 +568,8 @@
 	return err;
 }
 
-static int rdac_activate(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);
 	int err = SCSI_DH_OK;
@@ -580,7 +581,9 @@
 	if (h->lun_state == RDAC_LUN_UNOWNED)
 		err = send_mode_select(sdev, h);
 done:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_alua.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_alua.c +5 -2
@@ -652,7 +652,8 @@
  * based on a certain policy. But until we actually encounter them it
  * should be okay.
  */
-static int alua_activate(struct scsi_device *sdev)
+static int alua_activate(struct scsi_device *sdev,
+			activate_complete fn, void *data)
 {
 	struct alua_dh_data *h = get_alua_data(sdev);
 	int err = SCSI_DH_OK;
@@ -667,7 +668,9 @@
 		err = alua_stpg(sdev, TPGS_STATE_OPTIMIZED, h);
 
 out:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 /*
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_emc.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_emc.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_emc.c +5 -2
@@ -528,7 +528,8 @@
 	return err;
 }
 
-static int clariion_activate(struct scsi_device *sdev)
+static int clariion_activate(struct scsi_device *sdev,
+				activate_complete fn, void *data)
 {
 	struct clariion_dh_data *csdev = get_clariion_data(sdev);
 	int result;
@@ -559,7 +560,9 @@
 		    csdev->port, lun_state[csdev->lun_state],
 		    csdev->default_sp + 'A');
 
-	return result;
+	if (fn)
+		fn(data, result);
+	return 0;
 }
 /*
  * params - parameters in the following format
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_hp_sw.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_hp_sw.c +5 -2
@@ -268,7 +268,8 @@
  * activate the passive path (and deactivate the
  * previously active one).
  */
-static int hp_sw_activate(struct scsi_device *sdev)
+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);
@@ -283,7 +284,9 @@
 				    HP_SW_NAME);
 	}
 
-	return ret;
+	if (fn)
+		fn(data, ret);
+	return 0;
 }
 
 static const struct scsi_dh_devlist hp_sw_dh_data_list[] = {
Index: linux-2.6.32-rc5/include/scsi/scsi_device.h
===================================================================
--- linux-2.6.32-rc5.orig/include/scsi/scsi_device.h 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/include/scsi/scsi_device.h 2009-10-20
18:26:14.000000000 -0700
include/scsi/scsi_device.h +2 -1
@@ -174,6 +174,7 @@
 	char *model;
 };
 
+typedef void (*activate_complete)(void *, int);
 struct scsi_device_handler {
 	/* Used by the infrastructure */
 	struct list_head list; /* list of scsi_device_handlers */
@@ -185,7 +186,7 @@
 	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 *);
+	int (*activate)(struct scsi_device *, activate_complete, void *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
};
Index: linux-2.6.32-rc5/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/md/dm-mpath.c 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/md/dm-mpath.c 2009-10-20
18:26:14.000000000 -0700
drivers/md/dm-mpath.c +4 -5
@@ -1116,8 +1116,9 @@
 	return limit_reached;
 }
 
-static void pg_init_done(struct dm_path *path, int errors)
+static void pg_init_done(void *data, int errors)
 {
+	struct dm_path *path = data;
 	struct pgpath *pgpath = path_to_pgpath(path);
 	struct priority_group *pg = pgpath->pg;
 	struct multipath *m = pg->m;
@@ -1183,12 +1184,11 @@
 
 static void activate_path(struct work_struct *work)
 {
-	int ret;
 	struct pgpath *pgpath =
 		container_of(work, struct pgpath, activate_path);
 
-	ret = scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev));
-	pg_init_done(&pgpath->path, ret);
+	scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev),
+				pg_init_done, &pgpath->path);
 }
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Moger, Babu

Tested-by on v1 1/4

2009-10-22 23:29 UTC #3 lore raw

RE: [PATCH 1/4] scsi_dh: Change the scsidh_activate interface to be asynchronous

In reply to Chandra Seetharaman

Correcting the type in my earlier response.. I have tested this patch on LSI rdac storage. It works pretty well..

Quoted 11 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Wednesday, October 21, 2009 11:23 AM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 1/4] scsi_dh: Change the scsidh_activate interface to be asynchronous

Quoted 7 lines

Make scsi_dh_activate() function asynchronous, by taking in two additional
parameters, one is the callback function and the other is the data to call
the callback function with.
Signed-off-by Chandra Seetharaman <[email protected]>

Tested-by Babu Moger <[email protected]>

Quoted 257 lines

--------------
 drivers/md/dm-mpath.c                       |    8 ++++----
 drivers/scsi/device_handler/scsi_dh.c       |   17 ++++++++++++-----
 drivers/scsi/device_handler/scsi_dh_alua.c  |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_emc.c   |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |    7 +++++--
 drivers/scsi/device_handler/scsi_dh_rdac.c  |    7 +++++--
 include/scsi/scsi_device.h                  |    3 ++-
 include/scsi/scsi_dh.h                      |    6 ++++--
 8 files changed, 42 insertions(+), 20 deletions(-)
Index: linux-2.6.32-rc5/include/scsi/scsi_dh.h
===================================================================
--- linux-2.6.32-rc5.orig/include/scsi/scsi_dh.h 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/include/scsi/scsi_dh.h 2009-10-20
18:26:14.000000000 -0700
include/scsi/scsi_dh.h +4 -2
@@ -56,14 +56,16 @@
 	SCSI_DH_DRIVER_MAX,
 };
 #if defined(CONFIG_SCSI_DH) || defined(CONFIG_SCSI_DH_MODULE)
-extern int scsi_dh_activate(struct request_queue *);
+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 int scsi_dh_set_params(struct request_queue *, const char *);
 #else
-static inline int scsi_dh_activate(struct request_queue *req)
+static inline int scsi_dh_activate(struct request_queue *req,
+					activate_complete fn, void *data)
 {
+	fn(data, 0);
 	return 0;
 }
static inline int scsi_dh_handler_exist(const char *name)
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh.c 2009-
10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh.c 2009-10-20
18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh.c +1 -1
@@ -226,7 +226,7 @@
 			 * Activate a device handler
 			 */
 			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev);
+				err = scsi_dh->activate(sdev, NULL, NULL);
 			else
 				err = 0;
 		}
@@ -423,10 +423,17 @@
 /*
  * scsi_dh_activate - activate the path associated with the
scsi_device
* corresponding to the given request queue.
- * @q - Request queue that is associated with the scsi_device to be
- * activated.
+ * Returns immediately without waiting for activation to be
completed.
+ * @q - Request queue that is associated with the scsi_device to be
+ * activated.
+ * @fn - Function to be called upon completion of the activation.
+ * Function fn is called with data (below) and the error code.
+ * Function fn may be called from the same calling context.
So,
+ * do not hold the lock in the caller which may be needed in
fn.
+ * @data - data passed to the function fn upon completion.
+ *
*/
(quoted diff) +1 -1
-int scsi_dh_activate(struct request_queue *q)
+int scsi_dh_activate(struct request_queue *q, activate_complete fn,
void *data)
{
drivers/scsi/device_handler/scsi_dh.c +1 -1
 	int err = 0;
 	unsigned long flags;
@@ -445,7 +452,7 @@
 		return err;
 
 	if (scsi_dh->activate)
-		err = scsi_dh->activate(sdev);
+		err = scsi_dh->activate(sdev, fn, data);
 	put_device(&sdev->sdev_gendev);
 	return err;
 }
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_rdac.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_rdac.c +5 -2
@@ -568,7 +568,8 @@
 	return err;
 }
 
-static int rdac_activate(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);
 	int err = SCSI_DH_OK;
@@ -580,7 +581,9 @@
 	if (h->lun_state == RDAC_LUN_UNOWNED)
 		err = send_mode_select(sdev, h);
 done:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 static int rdac_prep_fn(struct scsi_device *sdev, struct request *req)
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_alua.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_alua.c +5 -2
@@ -652,7 +652,8 @@
  * based on a certain policy. But until we actually encounter them it
  * should be okay.
  */
-static int alua_activate(struct scsi_device *sdev)
+static int alua_activate(struct scsi_device *sdev,
+			activate_complete fn, void *data)
 {
 	struct alua_dh_data *h = get_alua_data(sdev);
 	int err = SCSI_DH_OK;
@@ -667,7 +668,9 @@
 		err = alua_stpg(sdev, TPGS_STATE_OPTIMIZED, h);
 
 out:
-	return err;
+	if (fn)
+		fn(data, err);
+	return 0;
 }
 
 /*
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_emc.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_emc.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_emc.c +5 -2
@@ -528,7 +528,8 @@
 	return err;
 }
 
-static int clariion_activate(struct scsi_device *sdev)
+static int clariion_activate(struct scsi_device *sdev,
+				activate_complete fn, void *data)
 {
 	struct clariion_dh_data *csdev = get_clariion_data(sdev);
 	int result;
@@ -559,7 +560,9 @@
 		    csdev->port, lun_state[csdev->lun_state],
 		    csdev->default_sp + 'A');
 
-	return result;
+	if (fn)
+		fn(data, result);
+	return 0;
 }
 /*
  * params - parameters in the following format
Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_hp_sw.c
	2009-10-20 18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c 2009-
10-20 18:26:14.000000000 -0700
drivers/scsi/device_handler/scsi_dh_hp_sw.c +5 -2
@@ -268,7 +268,8 @@
  * activate the passive path (and deactivate the
  * previously active one).
  */
-static int hp_sw_activate(struct scsi_device *sdev)
+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);
@@ -283,7 +284,9 @@
 				    HP_SW_NAME);
 	}
 
-	return ret;
+	if (fn)
+		fn(data, ret);
+	return 0;
 }
 
 static const struct scsi_dh_devlist hp_sw_dh_data_list[] = {
Index: linux-2.6.32-rc5/include/scsi/scsi_device.h
===================================================================
--- linux-2.6.32-rc5.orig/include/scsi/scsi_device.h 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/include/scsi/scsi_device.h 2009-10-20
18:26:14.000000000 -0700
include/scsi/scsi_device.h +2 -1
@@ -174,6 +174,7 @@
 	char *model;
 };
 
+typedef void (*activate_complete)(void *, int);
 struct scsi_device_handler {
 	/* Used by the infrastructure */
 	struct list_head list; /* list of scsi_device_handlers */
@@ -185,7 +186,7 @@
 	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 *);
+	int (*activate)(struct scsi_device *, activate_complete, void *);
 	int (*prep_fn)(struct scsi_device *, struct request *);
 	int (*set_params)(struct scsi_device *, const char *);
};
Index: linux-2.6.32-rc5/drivers/md/dm-mpath.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/md/dm-mpath.c 2009-10-20
18:26:03.000000000 -0700
+++ linux-2.6.32-rc5/drivers/md/dm-mpath.c 2009-10-20
18:26:14.000000000 -0700
drivers/md/dm-mpath.c +4 -5
@@ -1116,8 +1116,9 @@
 	return limit_reached;
 }
 
-static void pg_init_done(struct dm_path *path, int errors)
+static void pg_init_done(void *data, int errors)
 {
+	struct dm_path *path = data;
 	struct pgpath *pgpath = path_to_pgpath(path);
 	struct priority_group *pg = pgpath->pg;
 	struct multipath *m = pg->m;
@@ -1183,12 +1184,11 @@
 
 static void activate_path(struct work_struct *work)
 {
-	int ret;
 	struct pgpath *pgpath =
 		container_of(work, struct pgpath, activate_path);
 
-	ret = scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev));
-	pg_init_done(&pgpath->path, ret);
+	scsi_dh_activate(bdev_get_queue(pgpath->path.dev->bdev),
+				pg_init_done, &pgpath->path);
 }
 
 /*
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Chandra Seetharaman

author

2009-10-21 16:22 UTC #4 lore raw

PATCH 2/4 scsi_dh: Make rdac hardware handler's activate() async

In reply to Chandra Seetharaman

Batch up MODE_SELECT in rdac device handler.

LSI RDAC storage has the capability of handling mode selects for multiple luns in a same command. Make use of that ability to send as few MODE SELECTs as possible to the storage controller as possible.

This patch creates a work queue and queues up activate requests when a MODE SELECT is sent down the wire. When that MODE SELECT completes, it compiles queued up activate requests for multiple luns into a single MODE SELECT.

This reduces the time to do failover/failback of large number of LUNS.

Signed-off-by Babu Moger <[email protected]>

Signed-off-by Chandra Seetharaman <[email protected]>

---
 drivers/scsi/device_handler/scsi_dh_rdac.c |  108 +++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 8 deletions(-)

drivers/scsi/device_handler/scsi_dh_rdac.c +100 -8

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
@@ -22,6 +22,7 @@
 #include <scsi/scsi.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_dh.h>
+#include <linux/workqueue.h>
 
 #define RDAC_NAME "rdac"
 #define RDAC_RETRY_COUNT 5

@@ -138,7 +139,13 @@
 	} mode_select;
 	u8	index;
 	u8	array_name[ARRAY_LABEL_LEN];
+	spinlock_t		ms_lock;
+	int			ms_queued;
+	struct work_struct	ms_work;
+	struct scsi_device	*ms_sdev;
+	struct list_head	ms_head;
 };
+
 struct c8_inquiry {
 	u8	peripheral_info;
 	u8	page_code; /* 0xC8 */

@@ -198,8 +205,17 @@
 	"owned (AVT mode)",
 };
 
+struct rdac_queue_data {
+	struct list_head	entry;
+	struct rdac_dh_data	*h;
+	activate_complete	callback_fn;
+	void			*callback_data;
+};
+
 static LIST_HEAD(ctlr_list);
 static DEFINE_SPINLOCK(list_lock);
+static struct workqueue_struct *kmpath_rdacd;
+static void send_mode_select(struct work_struct *work);
 
 /*
  * module parameter to enable rdac debug logging.

@@ -281,7 +297,6 @@
 		rdac_pg->subpage_code = 0x1;
 		rdac_pg->page_len[0] = 0x01;
 		rdac_pg->page_len[1] = 0x28;
-		rdac_pg->lun_table[h->lun] = 0x81;
 	} else {
 		struct rdac_pg_legacy *rdac_pg;

@@ -291,7 +306,6 @@
 		common = &rdac_pg->common;
 		rdac_pg->page_code = RDAC_PAGE_CODE_REDUNDANT_CONTROLLER;
 		rdac_pg->page_len = 0x68;
-		rdac_pg->lun_table[h->lun] = 0x81;
 	}
 	common->rdac_mode[1] = RDAC_MODE_TRANSFER_SPECIFIED_LUNS;
 	common->quiescence_timeout = RDAC_QUIESCENCE_TIME;

@@ -325,6 +339,7 @@
 	struct rdac_controller *ctlr;
 	ctlr = container_of(kref, struct rdac_controller, kref);
 
+	flush_workqueue(kmpath_rdacd);
 	spin_lock(&list_lock);
 	list_del(&ctlr->node);
 	spin_unlock(&list_lock);

@@ -363,6 +378,11 @@
 
 	kref_init(&ctlr->kref);
 	ctlr->use_ms10 = -1;
+	ctlr->ms_queued = 0;
+	ctlr->ms_sdev = NULL;
+	spin_lock_init(&ctlr->ms_lock);
+	INIT_WORK(&ctlr->ms_work, send_mode_select);
+	INIT_LIST_HEAD(&ctlr->ms_head);
 	list_add(&ctlr->node, &ctlr_list);
 done:
 	spin_unlock(&list_lock);

@@ -490,7 +510,7 @@
 }
 
 static int mode_select_handle_sense(struct scsi_device *sdev,
-				    unsigned char *sensebuf)
+					unsigned char *sensebuf)
 {
 	struct scsi_sense_hdr sense_hdr;
 	int err = SCSI_DH_IO, ret;

@@ -533,11 +553,29 @@
 	return err;
 }
 
-static int send_mode_select(struct scsi_device *sdev, struct rdac_dh_data *h)
+static void send_mode_select(struct work_struct *work)
 {
+	struct rdac_controller *ctlr =
+		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 request_queue *q = sdev->request_queue;
 	int err, retry_cnt = RDAC_RETRY_COUNT;
+	struct rdac_queue_data *tmp, *qdata;
+	LIST_HEAD(list);
+	u8 *lun_table;
+
+	spin_lock(&ctlr->ms_lock);
+	list_splice_init(&ctlr->ms_head, &list);
+	ctlr->ms_queued = 0;
+	ctlr->ms_sdev = NULL;
+	spin_unlock(&ctlr->ms_lock);
+
+	if (ctlr->use_ms10)
+		lun_table = ctlr->mode_select.expanded.lun_table;
+	else
+		lun_table = ctlr->mode_select.legacy.lun_table;
 
 retry:
 	err = SCSI_DH_RES_TEMP_UNAVAIL;

@@ -545,6 +583,10 @@
 	if (!rq)
 		goto done;
 
+	list_for_each_entry(qdata, &list, entry) {
+		lun_table[qdata->h->lun] = 0x81;
+	}
+
 	RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d, "
 		"%s MODE_SELECT command",
 		(char *) h->ctlr->array_name, h->ctlr->index,

@@ -565,7 +607,41 @@
 	}
 
 done:
-	return err;
+	list_for_each_entry_safe(qdata, tmp, &list, entry) {
+		list_del(&qdata->entry);
+		if (err == SCSI_DH_OK)
+			qdata->h->state = RDAC_STATE_ACTIVE;
+		if (qdata->callback_fn)
+			qdata->callback_fn(qdata->callback_data, err);
+		kfree(qdata);
+	}
+	return;
+}
+
+static int queue_mode_select(struct scsi_device *sdev,
+				activate_complete fn, void *data)
+{
+	struct rdac_queue_data *qdata;
+	struct rdac_controller *ctlr;
+
+	qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
+	if (!qdata)
+		return SCSI_DH_RETRY;
+
+	qdata->h = get_rdac_data(sdev);
+	qdata->callback_fn = fn;
+	qdata->callback_data = data;
+
+	ctlr = qdata->h->ctlr;
+	spin_lock(&ctlr->ms_lock);
+	list_add_tail(&qdata->entry, &ctlr->ms_head);
+	if (!ctlr->ms_queued) {
+		ctlr->ms_queued = 1;
+		ctlr->ms_sdev = sdev;
+		queue_work(kmpath_rdacd, &ctlr->ms_work);
+	}
+	spin_unlock(&ctlr->ms_lock);
+	return SCSI_DH_OK;
 }
 
 static int rdac_activate(struct scsi_device *sdev,

@@ -578,8 +654,11 @@
 	if (err != SCSI_DH_OK)
 		goto done;
 
-	if (h->lun_state == RDAC_LUN_UNOWNED)
-		err = send_mode_select(sdev, h);
+	if (h->lun_state == RDAC_LUN_UNOWNED) {
+		err = queue_mode_select(sdev, fn, data);
+		if (err == SCSI_DH_OK)
+			return 0;
+	}
 done:
 	if (fn)
 		fn(data, err);

@@ -793,13 +872,26 @@
 	int r;
 
 	r = scsi_register_device_handler(&rdac_dh);
-	if (r != 0)
+	if (r != 0) {
 		printk(KERN_ERR "Failed to register scsi device handler.");
+		goto done;
+	}
+
+	/*
+	 * Create workqueue to handle mode selects for rdac
+	 */
+	kmpath_rdacd = create_singlethread_workqueue("kmpath_rdacd");
+	if (!kmpath_rdacd) {
+		scsi_unregister_device_handler(&rdac_dh);
+		printk(KERN_ERR "kmpath_rdacd creation failed.\n");
+	}
+done:
 	return r;
 }
 
 static void __exit rdac_exit(void)
 {
+	destroy_workqueue(kmpath_rdacd);
 	scsi_unregister_device_handler(&rdac_dh);
 }

Moger, Babu

Tested-by on v1 2/4

2009-10-22 23:30 UTC #5 lore raw

RE: [PATCH 2/4] scsi_dh: Make rdac hardware handler's activate() async

I have tested this patch on LSI rdac storage. It works pretty well..

Quoted 22 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Wednesday, October 21, 2009 11:23 AM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 2/4] scsi_dh: Make rdac hardware handler's activate() async
Batch up MODE_SELECT in rdac device handler.
LSI RDAC storage has the capability of handling mode selects for multiple luns in a same command. Make use of that ability to send as few MODE SELECTs as possible to the storage controller as possible.
This patch creates a work queue and queues up activate requests when a MODE SELECT is sent down the wire. When that MODE SELECT completes, it compiles queued up activate requests for multiple luns into a single MODE SELECT.

Quoted 4 lines

This reduces the time to do failover/failback of large number of LUNS.
Signed-off-by Babu Moger <[email protected]>
Signed-off-by Chandra Seetharaman <[email protected]>

Tested-by Babu Moger <[email protected]>

Quoted 230 lines

---
 drivers/scsi/device_handler/scsi_dh_rdac.c |  108
+++++++++++++++++++++++++++--
 1 file changed, 100 insertions(+), 8 deletions(-)

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================
--- linux-2.6.32-rc5.orig/drivers/scsi/device_handler/scsi_dh_rdac.c

	2009-10-20 18:26:14.000000000 -0700

+++ linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_rdac.c 2009-
10-20 18:26:29.000000000 -0700

drivers/scsi/device_handler/scsi_dh_rdac.c +42 -4

@@ -22,6 +22,7 @@
 #include <scsi/scsi.h>
 #include <scsi/scsi_eh.h>
 #include <scsi/scsi_dh.h>
+#include <linux/workqueue.h>
 
 #define RDAC_NAME "rdac"
 #define RDAC_RETRY_COUNT 5

@@ -138,7 +139,13 @@
 	} mode_select;
 	u8	index;
 	u8	array_name[ARRAY_LABEL_LEN];
+	spinlock_t		ms_lock;
+	int			ms_queued;
+	struct work_struct	ms_work;
+	struct scsi_device	*ms_sdev;
+	struct list_head	ms_head;
 };
+
 struct c8_inquiry {
 	u8	peripheral_info;
 	u8	page_code; /* 0xC8 */

@@ -198,8 +205,17 @@
 	"owned (AVT mode)",
 };
 
+struct rdac_queue_data {
+	struct list_head	entry;
+	struct rdac_dh_data	*h;
+	activate_complete	callback_fn;
+	void			*callback_data;
+};
+
 static LIST_HEAD(ctlr_list);
 static DEFINE_SPINLOCK(list_lock);
+static struct workqueue_struct *kmpath_rdacd;
+static void send_mode_select(struct work_struct *work);
 
 /*
  * module parameter to enable rdac debug logging.

@@ -281,7 +297,6 @@
 		rdac_pg->subpage_code = 0x1;
 		rdac_pg->page_len[0] = 0x01;
 		rdac_pg->page_len[1] = 0x28;
-		rdac_pg->lun_table[h->lun] = 0x81;
 	} else {
 		struct rdac_pg_legacy *rdac_pg;

@@ -291,7 +306,6 @@
 		common = &rdac_pg->common;
 		rdac_pg->page_code = RDAC_PAGE_CODE_REDUNDANT_CONTROLLER;
 		rdac_pg->page_len = 0x68;
-		rdac_pg->lun_table[h->lun] = 0x81;
 	}
 	common->rdac_mode[1] = RDAC_MODE_TRANSFER_SPECIFIED_LUNS;
 	common->quiescence_timeout = RDAC_QUIESCENCE_TIME;

@@ -325,6 +339,7 @@
 	struct rdac_controller *ctlr;
 	ctlr = container_of(kref, struct rdac_controller, kref);
 
+	flush_workqueue(kmpath_rdacd);
 	spin_lock(&list_lock);
 	list_del(&ctlr->node);
 	spin_unlock(&list_lock);

@@ -363,6 +378,11 @@
 
 	kref_init(&ctlr->kref);
 	ctlr->use_ms10 = -1;
+	ctlr->ms_queued = 0;
+	ctlr->ms_sdev = NULL;
+	spin_lock_init(&ctlr->ms_lock);
+	INIT_WORK(&ctlr->ms_work, send_mode_select);
+	INIT_LIST_HEAD(&ctlr->ms_head);
 	list_add(&ctlr->node, &ctlr_list);
 done:
 	spin_unlock(&list_lock);

@@ -490,7 +510,7 @@
 }
 
 static int mode_select_handle_sense(struct scsi_device *sdev,
-				    unsigned char *sensebuf)
+					unsigned char *sensebuf)
 {
 	struct scsi_sense_hdr sense_hdr;
 	int err = SCSI_DH_IO, ret;

@@ -533,11 +553,29 @@
 	return err;
 }
 
-static int send_mode_select(struct scsi_device *sdev, struct
 rdac_dh_data *h)
+static void send_mode_select(struct work_struct *work)
 {
+	struct rdac_controller *ctlr =
+		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 request_queue *q = sdev->request_queue;
 	int err, retry_cnt = RDAC_RETRY_COUNT;
+	struct rdac_queue_data *tmp, *qdata;
+	LIST_HEAD(list);
+	u8 *lun_table;
+
+	spin_lock(&ctlr->ms_lock);
+	list_splice_init(&ctlr->ms_head, &list);
+	ctlr->ms_queued = 0;
+	ctlr->ms_sdev = NULL;
+	spin_unlock(&ctlr->ms_lock);
+
+	if (ctlr->use_ms10)
+		lun_table = ctlr->mode_select.expanded.lun_table;
+	else
+		lun_table = ctlr->mode_select.legacy.lun_table;
 
 retry:

drivers/scsi/device_handler/scsi_dh_rdac.c +58 -5

 	err = SCSI_DH_RES_TEMP_UNAVAIL;

@@ -545,6 +583,10 @@
 	if (!rq)
 		goto done;
 
+	list_for_each_entry(qdata, &list, entry) {
+		lun_table[qdata->h->lun] = 0x81;
+	}
+
 	RDAC_LOG(RDAC_LOG_FAILOVER, sdev, "array %s, ctlr %d, "
 		"%s MODE_SELECT command",
 		(char *) h->ctlr->array_name, h->ctlr->index,

@@ -565,7 +607,41 @@
 	}
 
 done:
-	return err;
+	list_for_each_entry_safe(qdata, tmp, &list, entry) {
+		list_del(&qdata->entry);
+		if (err == SCSI_DH_OK)
+			qdata->h->state = RDAC_STATE_ACTIVE;
+		if (qdata->callback_fn)
+			qdata->callback_fn(qdata->callback_data, err);
+		kfree(qdata);
+	}
+	return;
+}
+
+static int queue_mode_select(struct scsi_device *sdev,
+				activate_complete fn, void *data)
+{
+	struct rdac_queue_data *qdata;
+	struct rdac_controller *ctlr;
+
+	qdata = kzalloc(sizeof(*qdata), GFP_KERNEL);
+	if (!qdata)
+		return SCSI_DH_RETRY;
+
+	qdata->h = get_rdac_data(sdev);
+	qdata->callback_fn = fn;
+	qdata->callback_data = data;
+
+	ctlr = qdata->h->ctlr;
+	spin_lock(&ctlr->ms_lock);
+	list_add_tail(&qdata->entry, &ctlr->ms_head);
+	if (!ctlr->ms_queued) {
+		ctlr->ms_queued = 1;
+		ctlr->ms_sdev = sdev;
+		queue_work(kmpath_rdacd, &ctlr->ms_work);
+	}
+	spin_unlock(&ctlr->ms_lock);
+	return SCSI_DH_OK;
 }
 
 static int rdac_activate(struct scsi_device *sdev,

@@ -578,8 +654,11 @@
 	if (err != SCSI_DH_OK)
 		goto done;
 
-	if (h->lun_state == RDAC_LUN_UNOWNED)
-		err = send_mode_select(sdev, h);
+	if (h->lun_state == RDAC_LUN_UNOWNED) {
+		err = queue_mode_select(sdev, fn, data);
+		if (err == SCSI_DH_OK)
+			return 0;
+	}
 done:
 	if (fn)
 		fn(data, err);

@@ -793,13 +872,26 @@
 	int r;
 
 	r = scsi_register_device_handler(&rdac_dh);
-	if (r != 0)
+	if (r != 0) {
 		printk(KERN_ERR "Failed to register scsi device handler.");
+		goto done;
+	}
+
+	/*
+	 * Create workqueue to handle mode selects for rdac
+	 */
+	kmpath_rdacd = create_singlethread_workqueue("kmpath_rdacd");
+	if (!kmpath_rdacd) {
+		scsi_unregister_device_handler(&rdac_dh);
+		printk(KERN_ERR "kmpath_rdacd creation failed.\n");
+	}
+done:
 	return r;
 }
 
 static void __exit rdac_exit(void)
 {
+	destroy_workqueue(kmpath_rdacd);
 	scsi_unregister_device_handler(&rdac_dh);
 }
 
--

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

Chandra Seetharaman

author

2009-10-21 16:22 UTC #6 lore raw

PATCH 3/4 scsi_dh: Make hp hardware handler's activate() async

In reply to Chandra Seetharaman

Make the activate function asynchronous by using blk_execute_rq_nowait()

Signed-off-by Chandra Seetharaman <[email protected]>

---
 drivers/scsi/device_handler/scsi_dh_hp_sw.c |   87 +++++++++++++++++-----------
 1 file changed, 54 insertions(+), 33 deletions(-)

drivers/scsi/device_handler/scsi_dh_hp_sw.c +54 ~~-33~~

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================
@@ -39,8 +39,14 @@
 	unsigned char sense[SCSI_SENSE_BUFFERSIZE];
 	int path_state;
 	int retries;
+	int retry_cnt;
+	struct scsi_device *sdev;
+	activate_complete	callback_fn;
+	void			*callback_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)
 {
 	struct scsi_dh_data *scsi_dh_data = sdev->scsi_dh_data;

@@ -191,19 +197,53 @@
 	return rc;
 }
 
+static void start_stop_endio(struct request *req, int error)
+{
+	struct hp_sw_dh_data *h = req->end_io_data;
+	unsigned err = SCSI_DH_OK;
+
+	if (error || host_byte(req->errors) != DID_OK ||
+			msg_byte(req->errors) != COMMAND_COMPLETE) {
+		sdev_printk(KERN_WARNING, h->sdev,
+			    "%s: sending start_stop_unit failed with %x\n",
+			    HP_SW_NAME, req->errors);
+		err = SCSI_DH_IO;
+		goto done;
+	}
+
+	if (req->sense_len > 0) {
+		err = start_done(h->sdev, h->sense);
+		if (err == SCSI_DH_RETRY) {
+			err = SCSI_DH_IO;
+			if (--h->retry_cnt) {
+				blk_put_request(req);
+				err = hp_sw_start_stop(h);
+				if (err == SCSI_DH_OK)
+					return;
+			}
+		}
+	}
+done:
+	blk_put_request(req);
+	if (h->callback_fn) {
+		h->callback_fn(h->callback_data, err);
+		h->callback_fn = h->callback_data = NULL;
+	}
+	return;
+
+}
+
 /*
  * hp_sw_start_stop - Send START STOP UNIT command
  * @sdev: sdev command should be sent to
  *
  * Sending START STOP UNIT activates the SP.
  */
-static int hp_sw_start_stop(struct scsi_device *sdev, struct hp_sw_dh_data *h)
+static int hp_sw_start_stop(struct hp_sw_dh_data *h)
 {
 	struct request *req;
-	int ret, retry;
 
-retry:
-	req = blk_get_request(sdev->request_queue, WRITE, GFP_NOIO);
+	req = blk_get_request(h->sdev->request_queue, WRITE, GFP_ATOMIC);
 	if (!req)
 		return SCSI_DH_RES_TEMP_UNAVAIL;

@@ -217,32 +257,10 @@
 	req->sense = h->sense;
 	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	req->sense_len = 0;
-	retry = h->retries;
+	req->end_io_data = h;
 
-	ret = blk_execute_rq(req->q, NULL, req, 1);
-	if (ret == -EIO) {
-		if (req->sense_len > 0) {
-			ret = start_done(sdev, h->sense);
-		} else {
-			sdev_printk(KERN_WARNING, sdev,
-				    "%s: sending start_stop_unit failed with %x\n",
-				    HP_SW_NAME, req->errors);
-			ret = SCSI_DH_IO;
-		}
-	} else
-		ret = SCSI_DH_OK;
-
-	if (ret == SCSI_DH_RETRY) {
-		if (--retry) {
-			blk_put_request(req);
-			goto retry;
-		}
-		ret = SCSI_DH_IO;
-	}
-
-	blk_put_request(req);
-
-	return ret;
+	blk_execute_rq_nowait(req->q, NULL, req, 1, start_stop_endio);
+	return SCSI_DH_OK;
 }
 
 static int hp_sw_prep_fn(struct scsi_device *sdev, struct request *req)

@@ -277,11 +295,13 @@
 	ret = hp_sw_tur(sdev, h);
 
 	if (ret == SCSI_DH_OK && h->path_state == HP_SW_PATH_PASSIVE) {
-		ret = hp_sw_start_stop(sdev, h);
+		h->retry_cnt = h->retries;
+		h->callback_fn = fn;
+		h->callback_data = data;
+		ret = hp_sw_start_stop(h);
 		if (ret == SCSI_DH_OK)
-			sdev_printk(KERN_INFO, sdev,
-				    "%s: activated path\n",
-				    HP_SW_NAME);
+			return 0;
+		h->callback_fn = h->callback_data = NULL;
 	}
 
 	if (fn)

@@ -329,6 +349,7 @@
 	h = (struct hp_sw_dh_data *) scsi_dh_data->buf;
 	h->path_state = HP_SW_PATH_UNINITIALIZED;
 	h->retries = HP_SW_RETRIES;
+	h->sdev = sdev;
 
 	ret = hp_sw_tur(sdev, h);
 	if (ret != SCSI_DH_OK || h->path_state == HP_SW_PATH_UNINITIALIZED)

Chandra Seetharaman

author

2009-10-21 16:23 UTC #7 lore raw

PATCH 4/4 scsi_dh: Make alua hardware handler's activate() async

In reply to Chandra Seetharaman

Make the activate function asynchronous by using blk_execute_rq_nowait()

Signed-off-by Chandra Seetharaman <[email protected]>

---
 drivers/scsi/device_handler/scsi_dh_alua.c |  132 ++++++++++++++++-------------
 1 file changed, 73 insertions(+), 59 deletions(-)

drivers/scsi/device_handler/scsi_dh_alua.c +73 ~~-59~~

Index: linux-2.6.32-rc5/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================
@@ -60,11 +60,17 @@
 	int			bufflen;
 	unsigned char		sense[SCSI_SENSE_BUFFERSIZE];
 	int			senselen;
+	struct scsi_device	*sdev;
+	activate_complete	callback_fn;
+	void			*callback_data;
 };
 
 #define ALUA_POLICY_SWITCH_CURRENT	0
 #define ALUA_POLICY_SWITCH_ALL		1
 
+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)
 {
 	struct scsi_dh_data *scsi_dh_data = sdev->scsi_dh_data;

@@ -231,18 +237,71 @@
 }
 
 /*
+ * alua_stpg - Evaluate SET TARGET GROUP STATES
+ * @sdev: the device to be evaluated
+ * @state: the new target group state
+ *
+ * Send a SET TARGET GROUP STATES command to the device.
+ * We only have to test here if we should resubmit the command;
+ * any other error is assumed as a failure.
+ */
+static void stpg_endio(struct request *req, int error)
+{
+	struct alua_dh_data *h = req->end_io_data;
+	struct scsi_sense_hdr sense_hdr;
+	unsigned err = SCSI_DH_IO;
+
+	if (error || host_byte(req->errors) != DID_OK ||
+			msg_byte(req->errors) != COMMAND_COMPLETE)
+		goto done;
+
+	if (err == SCSI_DH_IO && h->senselen > 0) {
+		err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
+					   &sense_hdr);
+		if (!err) {
+			err = SCSI_DH_IO;
+			goto done;
+		}
+		err = alua_check_sense(h->sdev, &sense_hdr);
+		if (err == ADD_TO_MLQUEUE) {
+			err = SCSI_DH_RETRY;
+			goto done;
+		}
+		sdev_printk(KERN_INFO, h->sdev,
+			    "%s: stpg sense code: %02x/%02x/%02x\n",
+			    ALUA_DH_NAME, sense_hdr.sense_key,
+			    sense_hdr.asc, sense_hdr.ascq);
+		err = SCSI_DH_IO;
+	}
+	if (err == SCSI_DH_OK) {
+		h->state = TPGS_STATE_OPTIMIZED;
+		sdev_printk(KERN_INFO, h->sdev,
+			    "%s: port group %02x switched to state %c\n",
+			    ALUA_DH_NAME, h->group_id,
+			    print_alua_state(h->state));
+	}
+done:
+	blk_put_request(req);
+	if (h->callback_fn) {
+		h->callback_fn(h->callback_data, err);
+		h->callback_fn = h->callback_data = NULL;
+	}
+	return;
+}
+
+/*
  * submit_stpg - Issue a SET TARGET GROUP STATES command
- * @sdev: sdev the command should be sent to
  *
  * Currently we're only setting the current target port group state
  * to 'active/optimized' and let the array firmware figure out
  * the states of the remaining groups.
  */
-static unsigned submit_stpg(struct scsi_device *sdev, struct alua_dh_data *h)
+static unsigned submit_stpg(struct alua_dh_data *h)
 {
 	struct request *rq;
 	int err = SCSI_DH_RES_TEMP_UNAVAIL;
 	int stpg_len = 8;
+	struct scsi_device *sdev = h->sdev;
 
 	/* Prepare the data buffer */
 	memset(h->buff, 0, stpg_len);

@@ -252,7 +311,7 @@
 
 	rq = get_alua_req(sdev, h->buff, stpg_len, WRITE);
 	if (!rq)
-		goto done;
+		return SCSI_DH_RES_TEMP_UNAVAIL;
 
 	/* Prepare the command. */
 	rq->cmd[0] = MAINTENANCE_OUT;

@@ -266,17 +325,9 @@
 	rq->sense = h->sense;
 	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
 	rq->sense_len = h->senselen = 0;
+	rq->end_io_data = h;
 
-	err = blk_execute_rq(rq->q, NULL, rq, 1);
-	if (err == -EIO) {
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: stpg failed with %x\n",
-			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
-		err = SCSI_DH_IO;
-	}
-	blk_put_request(rq);
-done:
+	blk_execute_rq_nowait(rq->q, NULL, rq, 1, stpg_endio);
 	return err;
 }

@@ -477,50 +528,6 @@
 }
 
 /*
- * alua_stpg - Evaluate SET TARGET GROUP STATES
- * @sdev: the device to be evaluated
- * @state: the new target group state
- *
- * Send a SET TARGET GROUP STATES command to the device.
- * We only have to test here if we should resubmit the command;
- * any other error is assumed as a failure.
- */
-static int alua_stpg(struct scsi_device *sdev, int state,
-		     struct alua_dh_data *h)
-{
-	struct scsi_sense_hdr sense_hdr;
-	unsigned err;
-	int retry = ALUA_FAILOVER_RETRIES;
-
- retry:
-	err = submit_stpg(sdev, h);
-	if (err == SCSI_DH_IO && h->senselen > 0) {
-		err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
-					   &sense_hdr);
-		if (!err)
-			return SCSI_DH_IO;
-		err = alua_check_sense(sdev, &sense_hdr);
-		if (retry > 0 && err == ADD_TO_MLQUEUE) {
-			retry--;
-			goto retry;
-		}
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: stpg sense code: %02x/%02x/%02x\n",
-			    ALUA_DH_NAME, sense_hdr.sense_key,
-			    sense_hdr.asc, sense_hdr.ascq);
-		err = SCSI_DH_IO;
-	}
-	if (err == SCSI_DH_OK) {
-		h->state = state;
-		sdev_printk(KERN_INFO, sdev,
-			    "%s: port group %02x switched to state %c\n",
-			    ALUA_DH_NAME, h->group_id,
-			    print_alua_state(h->state) );
-	}
-	return err;
-}
-
-/*
  * alua_rtpg - Evaluate REPORT TARGET GROUP STATES
  * @sdev: the device to be evaluated.
  *

@@ -664,8 +671,14 @@
 			goto out;
 	}
 
-	if (h->tpgs & TPGS_MODE_EXPLICIT && h->state != TPGS_STATE_OPTIMIZED)
-		err = alua_stpg(sdev, TPGS_STATE_OPTIMIZED, h);
+	if (h->tpgs & TPGS_MODE_EXPLICIT && h->state != TPGS_STATE_OPTIMIZED) {
+		h->callback_fn = fn;
+		h->callback_data = data;
+		err = submit_stpg(h);
+		if (err == SCSI_DH_OK)
+			return 0;
+		h->callback_fn = h->callback_data = NULL;
+	}
 
 out:
 	if (fn)

@@ -748,6 +761,7 @@
 	h->rel_port = -1;
 	h->buff = h->inq;
 	h->bufflen = ALUA_INQUIRY_SIZE;
+	h->sdev = sdev;
 
 	err = alua_initialize(sdev, h);
 	if (err != SCSI_DH_OK)

Christoph Hellwig

2009-10-22 01:06 UTC #8 lore raw

In reply to Chandra Seetharaman

Quoted 6 lines · Chandra Seetharaman

Hello All,
Currently, device handlers process path activation in series. This leads to a lot of time delay when more than 100 luns are involved. For example, with lsi rdac 100+ luns take about 12-15 minutes. This was found by Moger Babu of LSI.

So what happened to the suggestion to just switch over per target
instead of per lun? It's the obviously much better way to do, and
unless there are bugs in the hardware preventing it from working it should be the way to go.

Chandra Seetharaman

2009-10-22 16:06 UTC #9 lore raw

Hi Christoph,

Thanks for looking into this.

Babu (LSI) and I did some testing and found couple of problems. Major one was that moving all luns blindly causes luns to be moved to their non-preferred path against the user's setting, which is a no-no.

Here is the email Babu sent on this:
http://marc.info/?l=linux-scsi&m=125495712310371&w=2

chandra

Quoted 17 lines · Christoph Hellwig

Quoted 6 lines · Chandra Seetharaman
Hello All,
Currently, device handlers process path activation in series. This leads to a lot of time delay when more than 100 luns are involved. For example, with lsi rdac 100+ luns take about 12-15 minutes. This was found by Moger Babu of LSI.
So what happened to the suggestion to just switch over per target
instead of per lun? It's the obviously much better way to do, and
unless there are bugs in the hardware preventing it from working it should be the way to go.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html