scsi_dh: Make scsi_dh_activate asynchronous

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Tuesday, September 29, 2009 9:08 PM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 0/4] 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

This set of patches applies cleanly on 2.6.31 and I tested the rdac 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

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

Hi Chandra,
Thanks for the patches. Patches applied cleanly on 2.6.31. I am testing the patches right now. Will post the with test results tomorrow.
These are my initial comments.
1. Patches do not apply on 2.6.32. You may have to regenerate it on 2.6.32.
2. Also, I noticed few errors and warnings while running checkpatch.pl script.
3. Subject line for patch 3 and 4 may need to be modified. It still says "rdac handler". It should be hp and alua handler.

Thanks..

Quoted 54 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Tuesday, September 29, 2009 9:08 PM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 0/4] 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

This set of patches applies cleanly on 2.6.31 and I tested the rdac 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

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

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Tuesday, September 29, 2009 9:08 PM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 2/4] scsi_dh: rdac handler: Make rdac hardware handler 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.

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 |  117
++++++++++++++++++++++++++---
 1 file changed, 104 insertions(+), 13 deletions(-)

Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================

drivers/scsi/device_handler/scsi_dh_rdac.c +15 -0

@@ -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

@@ -135,6 +136,11 @@ struct rdac_controller {
 		struct rdac_pg_legacy legacy;
 		struct rdac_pg_expanded expanded;
 	} mode_select;
+	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;

@@ -195,8 +201,17 @@ static const char *lun_state[] =
 	"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);
 
 static inline struct rdac_dh_data *get_rdac_data(struct scsi_device
 *sdev)

{

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

@@ -253,7 +268,6 @@ static struct request *rdac_failover_get
 		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;
 

@@ -263,7 +277,6 @@ static struct request *rdac_failover_get
 		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;

@@ -297,6 +310,7 @@ static void release_controller(struct kr
 	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);

@@ -326,6 +340,11 @@ static struct rdac_controller *get_contr
 	memcpy(ctlr->slot_id, slot_id, SLOT_ID_LEN);
 	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);

@@ -445,8 +464,7 @@ static int set_mode_select(struct scsi_d
 	return err;
 }
 
-static int mode_select_handle_sense(struct scsi_device *sdev,
-				    unsigned char *sensebuf)
+static int mode_select_handle_sense(unsigned char *sensebuf)
 {
 	struct scsi_sense_hdr sense_hdr;
 	int err = SCSI_DH_IO, ret;

@@ -478,7 +496,7 @@ static int mode_select_handle_sense(stru
 			err = SCSI_DH_RETRY;
 		break;
 	default:
-		sdev_printk(KERN_INFO, sdev,
+		printk(KERN_INFO
 			    "MODE_SELECT failed with sense
 %02x/%02x/%02x.\n",
 			    sense_hdr.sense_key, sense_hdr.asc,

sense_hdr.ascq);

drivers/scsi/device_handler/scsi_dh_rdac.c +19 -1

 	}

@@ -487,11 +505,29 @@ done:
 	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 +43 -4

 	err = SCSI_DH_RES_TEMP_UNAVAIL;

@@ -499,13 +535,17 @@ retry:
 	if (!rq)
 		goto done;
 
-	sdev_printk(KERN_INFO, sdev, "%s MODE_SELECT command.\n",
+ 	list_for_each_entry(qdata, &list, entry) {
+		lun_table[qdata->h->lun] = 0x81;
+	}
+
+	printk(KERN_INFO "%s MODE_SELECT command.\n",
 		(retry_cnt == RDAC_RETRY_COUNT) ? "queueing" : "retrying");
 
 	err = blk_execute_rq(q, NULL, rq, 1);
 	blk_put_request(rq);
 	if (err != SCSI_DH_OK) {
-		err = mode_select_handle_sense(sdev, h->sense);
+		err = mode_select_handle_sense(h->sense);
 		if (err == SCSI_DH_RETRY && retry_cnt--)
 			goto retry;
 	}

@@ -513,10 +553,45 @@ retry:
 		h->state = RDAC_STATE_ACTIVE;
 
 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 rdac_activate(struct scsi_device *sdev, activate_complete
 fn, void *data)
+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,
+				activate_complete fn, void *data)
 {
 	struct rdac_dh_data *h = get_rdac_data(sdev);

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

 	int err = SCSI_DH_OK;

@@ -536,8 +611,11 @@ static int rdac_activate(struct scsi_dev
 		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);

@@ -726,13 +804,26 @@ static int __init rdac_init(void)
 	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

Hi Chandra,
I have tested these patches on 2.6.31 kernel with LSI storage.
My configuration had 234 Luns with two paths to each lun(one active, one passive) and luns were evenly distributed. Here are the results.

Results with the patches
Failover takes about 1 minute to complete 117 mode selects. Failovers are completed in two sets(1 mode select first and then 116 mode selects together). Failback takes about 2 and Half minutes..

Results without the patches on same configuration failover takes about 12 minutes to complete 117 mode selects. Failback takes about 15 minutes.

Thanks
Babu Moger

Quoted 280 lines

-----Original Message-----
From: [email protected] [mailto:linux-scsi- [email protected]] On Behalf Of Chandra Seetharaman
Sent: Tuesday, September 29, 2009 9:08 PM
To: [email protected]
Cc: [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]; Chandra Seetharaman
Subject: [PATCH 2/4] scsi_dh: rdac handler: Make rdac hardware handler 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.

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 |  117
++++++++++++++++++++++++++---
 1 file changed, 104 insertions(+), 13 deletions(-)

Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================

drivers/scsi/device_handler/scsi_dh_rdac.c +15 -0

@@ -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

@@ -135,6 +136,11 @@ struct rdac_controller {
 		struct rdac_pg_legacy legacy;
 		struct rdac_pg_expanded expanded;
 	} mode_select;
+	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;

@@ -195,8 +201,17 @@ static const char *lun_state[] =
 	"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);
 
 static inline struct rdac_dh_data *get_rdac_data(struct scsi_device
 *sdev)

{

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

@@ -253,7 +268,6 @@ static struct request *rdac_failover_get
 		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;
 

@@ -263,7 +277,6 @@ static struct request *rdac_failover_get
 		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;

@@ -297,6 +310,7 @@ static void release_controller(struct kr
 	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);

@@ -326,6 +340,11 @@ static struct rdac_controller *get_contr
 	memcpy(ctlr->slot_id, slot_id, SLOT_ID_LEN);
 	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);

@@ -445,8 +464,7 @@ static int set_mode_select(struct scsi_d
 	return err;
 }
 
-static int mode_select_handle_sense(struct scsi_device *sdev,
-				    unsigned char *sensebuf)
+static int mode_select_handle_sense(unsigned char *sensebuf)
 {
 	struct scsi_sense_hdr sense_hdr;
 	int err = SCSI_DH_IO, ret;

@@ -478,7 +496,7 @@ static int mode_select_handle_sense(stru
 			err = SCSI_DH_RETRY;
 		break;
 	default:
-		sdev_printk(KERN_INFO, sdev,
+		printk(KERN_INFO
 			    "MODE_SELECT failed with sense
 %02x/%02x/%02x.\n",
 			    sense_hdr.sense_key, sense_hdr.asc,

sense_hdr.ascq);

drivers/scsi/device_handler/scsi_dh_rdac.c +19 -1

 	}

@@ -487,11 +505,29 @@ done:
 	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 +43 -4

 	err = SCSI_DH_RES_TEMP_UNAVAIL;

@@ -499,13 +535,17 @@ retry:
 	if (!rq)
 		goto done;
 
-	sdev_printk(KERN_INFO, sdev, "%s MODE_SELECT command.\n",
+ 	list_for_each_entry(qdata, &list, entry) {
+		lun_table[qdata->h->lun] = 0x81;
+	}
+
+	printk(KERN_INFO "%s MODE_SELECT command.\n",
 		(retry_cnt == RDAC_RETRY_COUNT) ? "queueing" : "retrying");
 
 	err = blk_execute_rq(q, NULL, rq, 1);
 	blk_put_request(rq);
 	if (err != SCSI_DH_OK) {
-		err = mode_select_handle_sense(sdev, h->sense);
+		err = mode_select_handle_sense(h->sense);
 		if (err == SCSI_DH_RETRY && retry_cnt--)
 			goto retry;
 	}

@@ -513,10 +553,45 @@ retry:
 		h->state = RDAC_STATE_ACTIVE;
 
 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 rdac_activate(struct scsi_device *sdev, activate_complete
 fn, void *data)
+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,
+				activate_complete fn, void *data)
 {
 	struct rdac_dh_data *h = get_rdac_data(sdev);

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

 	int err = SCSI_DH_OK;

@@ -536,8 +611,11 @@ static int rdac_activate(struct scsi_dev
 		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);

@@ -726,13 +804,26 @@ static int __init rdac_init(void)
 	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

drivers/scsi/device_handler/scsi_dh.c +0 -0

 			else
 				err = 0;
 		}

@@ -411,10 +411,17 @@ EXPORT_SYMBOL_GPL(scsi_unregister_device
 /*
  * 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.
+ *
*/

drivers/scsi/device_handler/scsi_dh.c +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;

@@ -433,7 +440,7 @@ int scsi_dh_activate(struct request_queu
 		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.31/drivers/scsi/device_handler/scsi_dh_rdac.c
===================================================================

drivers/scsi/device_handler/scsi_dh_rdac.c +1 -1

@@ -516,7 +516,7 @@ done:
 	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);

drivers/scsi/device_handler/scsi_dh_rdac.c +3 -1

 	int err = SCSI_DH_OK;

@@ -539,7 +539,9 @@ static int rdac_activate(struct scsi_dev
 	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.31/drivers/scsi/device_handler/scsi_dh_alua.c
===================================================================

drivers/scsi/device_handler/scsi_dh_alua.c +1 -1

@@ -652,7 +652,7 @@ out:
  * 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);

drivers/scsi/device_handler/scsi_dh_alua.c +3 -1

 	int err = SCSI_DH_OK;

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

Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh_emc.c
===================================================================

drivers/scsi/device_handler/scsi_dh_emc.c +1 -1

@@ -528,7 +528,7 @@ retry:
 	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);

drivers/scsi/device_handler/scsi_dh_emc.c +3 -1

 	int result;

@@ -559,7 +559,9 @@ done:
 		    csdev->port, lun_state[csdev->lun_state],
 		    csdev->default_sp + 'A');
 
-	return result;
+	if (fn)
+		fn(data, result);
+	return 0;
 }
 
 static const struct scsi_dh_devlist clariion_dev_list[] = {

Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh_hp_sw.c
===================================================================

drivers/scsi/device_handler/scsi_dh_hp_sw.c +1 -1

@@ -268,7 +268,7 @@ static int hp_sw_prep_fn(struct scsi_dev
  * 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;

drivers/scsi/device_handler/scsi_dh_hp_sw.c +3 -1

 	struct hp_sw_dh_data *h = get_hp_sw_data(sdev);

@@ -283,7 +283,9 @@ static int hp_sw_activate(struct scsi_de
 				    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.31/include/scsi/scsi_device.h
===================================================================

include/scsi/scsi_device.h +2 -1

@@ -174,6 +174,7 @@ struct scsi_dh_devlist {
 	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 @@ struct scsi_device_handler {
 	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 *);
 };

Index: linux-2.6.31/drivers/md/dm-mpath.c
===================================================================

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

@@ -1086,8 +1086,9 @@ static int pg_init_limit_reached(struct
 	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;

@@ -1153,12 +1154,11 @@ static void pg_init_done(struct dm_path
 
 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, I have one comment on this patch..

drivers/scsi/device_handler/scsi_dh.c store_dh_state(struct device *dev, struc · line 217

Quoted 11 lines

static inline int scsi_dh_handler_exist(const char *name)
Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh.c
===================================================================

drivers/scsi/device_handler/scsi_dh.c +1 -1

@@ -214,7 +214,7 @@ store_dh_state(struct device *dev, struc
 			 * Activate a device handler
 			 */
 			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev);
+				err = scsi_dh->activate(sdev, NULL, NULL);

This might cause problems in handlers. We don’t do NULL check in attach routine.

drivers/scsi/device_handler/scsi_dh.c store_dh_state(struct device *dev, struc · line 220

Quoted 5 lines

drivers/scsi/device_handler/scsi_dh.c +0 -0

 			else
 				err = 0;
 		}

Quoted 16 lines

Chandra, I have one comment on this patch..

drivers/scsi/device_handler/scsi_dh.c store_dh_state(struct device *dev, struc · line 217

Quoted 11 lines

static inline int scsi_dh_handler_exist(const char *name)
Index: linux-2.6.31/drivers/scsi/device_handler/scsi_dh.c
===================================================================

drivers/scsi/device_handler/scsi_dh.c +1 -1

@@ -214,7 +214,7 @@ store_dh_state(struct device *dev, struc
 			 * Activate a device handler
 			 */
 			if (scsi_dh->activate)
-				err = scsi_dh->activate(sdev);
+				err = scsi_dh->activate(sdev, NULL, NULL);

This might cause problems in handlers. We don’t do NULL check in

attach routine.

I assume you meant activate(), when you mention "attach" above.

At every place where the callback function is called, we first check that the callback function is not NULL.

Can you point me if I missed some place.

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

This set of patches applies cleanly on 2.6.31 and I tested the rdac handler on the same.

Please review and provide comments.

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

However: The main reason why we're getting flooded with MODE SELECT commands is that the RDAC handler switches _each LUN_, not the entire controller. Seeing that the controller simply cannot cope with the resulting MODE SELECT flood wouldn't it be more sensible to switch the entire controller here?

After all, we're trying to address a communication failure between the HBA and the controller, not a failure between the controller and the LUN. And by that reasoning switching individual LUNs is quite pointless as we have to switch _all_ LUNs handled by this controller eventually.

So I would suggest to first issue a MODE SENSE command to check which LUNs are currently handled by this controller and then switch those LUNs in one go. This way we would be sending quite a few MODE SENSE commands, but I was under the impression that those do not have any restriction.

I will see to draw up a patch.

Hmm. IIRC we added the synchronous mode by request of LSI/IBM, as the RDAC handler could only support on MODE SELECT command at a time. If LSI checked this, okay, no objections.

However: The main reason why we're getting flooded with MODE SELECT commands is that the RDAC handler switches _each LUN_, not the entire controller. Seeing that the controller simply cannot cope with the resulting MODE SELECT flood wouldn't it be more sensible to switch the entire controller here?

After all, we're trying to address a communication failure between the HBA and the controller, not a failure between the controller and the LUN. And by that reasoning switching individual LUNs is quite pointless as we have to switch _all_ LUNs handled by this controller eventually.

So I would suggest to first issue a MODE SENSE command to check which LUNs are currently handled by this controller and then switch those LUNs in one go. This way we would be sending quite a few MODE SENSE commands, but I was under the impression that those do not have any restriction.

I will see to draw up a patch.

Cheers,

Hannes

On Mon, 2009-10-05 at 15:01 +0200, Hannes Reinecke wrote:

Thanks for the comment and the patch. I will try the patch.

Quoted 3 lines

Hmm. IIRC we added the synchronous mode by request of LSI/IBM, as the RDAC handler could only support on MODE SELECT command at a time. If LSI checked this, okay, no objections.

The original patch (for rdac) came from LSI (Moger Babu), I made changes to the infrastructure and to the code to fit them together.

Quoted 4 lines

However: The main reason why we're getting flooded with MODE SELECT commands is that the RDAC handler switches _each LUN_, not the entire controller. Seeing that the controller simply cannot cope with the resulting MODE SELECT flood wouldn't it be more sensible to switch the entire controller here?

I see your point of view.... but here is the rationale...

When we originally added the rdac support (as dm_rdac), we decided this way consciously for the following reasons:
1. we do not know which link is broken (hba-ctlr or ctlr-lun). The way it is currently implemented, both these cases are handled.

2. multipath layer to decide what to do and this module to just do what it was told.

3. since multipath sends pg_init only if there is any IO sent to the lun, (with the current implementation) we don't have to change ownership (back and forth) of all the luns if the user is using only a handful.

4. to be consistent with LSI's original driver (which does one lun at a

    time).

-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 3:08 AM
To: [email protected]
Cc: [email protected]; [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0/4] scsi_dh: Make scsi_dh_activate asynchronous

Quoted 5 lines · Chandra Seetharaman

On Mon, 2009-10-05 at 15:01 +0200, Hannes Reinecke wrote:

Thanks for the comment and the patch. I will try the patch.

Quoted 1 line

Hmm. IIRC we added the synchronous mode by request of LSI/IBM, as

the RDAC

Quoted 4 lines

Quoted 2 lines

handler could only support on MODE SELECT command at a time. If LSI checked this, okay, no objections.

The original patch (for rdac) came from LSI (Moger Babu), I made

changes

Quoted 3 lines

to the infrastructure and to the code to fit them together.

Quoted 1 line

However: The main reason why we're getting flooded with MODE SELECT

commands

Quoted 1 line

Quoted 1 line

is that the RDAC handler switches _each LUN_, not the entire

controller.

Quoted 1 line

Quoted 1 line

Seeing that the controller simply cannot cope with the resulting

MODE SELECT

Quoted 1 line

Quoted 1 line

flood wouldn't it be more sensible to switch the entire controller

here?

Quoted 4 lines

I see your point of view.... but here is the rationale...

When we originally added the rdac support (as dm_rdac), we decided

this

Quoted 3 lines

way consciously for the following reasons:
1. we do not know which link is broken (hba-ctlr or ctlr-lun). The way it is currently implemented, both these cases are

handled.
Quite. But if the ctlr-lun link is broken, we really should receive and appropriate error code, which then could be handled in dm-rdac appropriately. After all, the controller is still accessible and so we don't have to guess what happened (which is all what multipath is about). So I doubt we need to worry about this.

Quoted 2 lines

2. multipath layer to decide what to do and this module to just do what it was told.

Yep.

Quoted 3 lines

3. since multipath sends pg_init only if there is any IO sent to the lun, (with the current implementation) we don't have to change ownership (back and forth) of all the luns if the user is using

only

Quoted 1 line

a handful.

Well, yes. But if the implementation is such that changing ownership for all LUNs is about as efficient as changing an individual LUN (or, as the case might be, as _inefficient_ :-), surely it's better to save us some coding here.

Quoted 1 line

4. to be consistent with LSI's original driver (which does one lun

at a

Quoted 1 line

    time).

:-/ That's unfair.
But it still has the drawback that it doesn't scale; given enough LUNs and
access patterns you _inevitably_ have to send MODE SELECT commands for each LUN, ie you only delay this issue.
Only by switching all LUNs you can avoid this.

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Moger, Babu
Sent: Tuesday, October 06, 2009 2:46 PM
To: Hannes Reinecke; [email protected]
Cc: [email protected]; Stankey, Robert; [email protected]; Dachepalli, Sudhir; [email protected]; Chauhan, Vijay; [email protected]; Qi, Yanling; [email protected]
Subject: [dm-devel] RE: [PATCH 0/4] scsi_dh: Make scsi_dh_activate asynchronous

Thanks Hannes and Chandra for your patches and feedback. Right now we are internally discussing the pros and cons of both these approaches. We will respond to all your question once we arrive at the conclusion.

Also adding our Team (Bob Stankey, Sudhir, Yanling, Vijay ) in the response.

Thanks
Babu Moger

Quoted 14 lines

-----Original Message-----
From: Hannes Reinecke [mailto:[email protected]]
Sent: Tuesday, October 06, 2009 3:08 AM
To: [email protected]
Cc: [email protected]; [email protected]; Moger, Babu; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH 0/4] scsi_dh: Make scsi_dh_activate asynchronous

Quoted 4 lines · Chandra Seetharaman

On Mon, 2009-10-05 at 15:01 +0200, Hannes Reinecke wrote:

Thanks for the comment and the patch. I will try the patch.

Quoted 10 lines

Quoted 1 line

Quoted 1 line

Hmm. IIRC we added the synchronous mode by request of LSI/IBM, as

the RDAC

Quoted 4 lines

Quoted 2 lines

handler could only support on MODE SELECT command at a time. If LSI checked this, okay, no objections.

The original patch (for rdac) came from LSI (Moger Babu), I made

changes

Quoted 3 lines

to the infrastructure and to the code to fit them together.

Quoted 1 line

However: The main reason why we're getting flooded with MODE

SELECT

Quoted 22 lines

commands

Quoted 1 line

Quoted 1 line

is that the RDAC handler switches _each LUN_, not the entire

controller.

Quoted 1 line

Quoted 1 line

Seeing that the controller simply cannot cope with the resulting

MODE SELECT

Quoted 1 line

Quoted 1 line

flood wouldn't it be more sensible to switch the entire controller

here?

Quoted 4 lines

I see your point of view.... but here is the rationale...

When we originally added the rdac support (as dm_rdac), we decided

this

Quoted 3 lines

way consciously for the following reasons:
1. we do not know which link is broken (hba-ctlr or ctlr-lun). The way it is currently implemented, both these cases are

handled.
Quite. But if the ctlr-lun link is broken, we really should receive and appropriate error code, which then could be handled in dm-rdac appropriately. After all, the controller is still accessible and so we don't have to guess what happened (which is all what multipath is about). So I doubt we need to worry about this.

Quoted 5 lines

Quoted 2 lines

2. multipath layer to decide what to do and this module to just do what it was told.

Yep.

Quoted 1 line

3. since multipath sends pg_init only if there is any IO sent to

the

Quoted 9 lines

Quoted 2 lines

lun, (with the current implementation) we don't have to change ownership (back and forth) of all the luns if the user is using

only

Quoted 1 line

a handful.

Well, yes. But if the implementation is such that changing ownership for all LUNs is about as efficient as changing an individual LUN (or, as the case might be, as _inefficient_ :-), surely it's better to save us some coding here.

Quoted 5 lines

Quoted 1 line

4. to be consistent with LSI's original driver (which does one lun

at a

Quoted 1 line

    time).

:-/ That's unfair.
But it still has the drawback that it doesn't scale; given enough

LUNs

Quoted 2 lines

and
access patterns you _inevitably_ have to send MODE SELECT commands

for

Quoted 11 lines

each LUN, ie you only delay this issue.
Only by switching all LUNs you can avoid this.

Cheers,

Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

--
dm-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/dm-devel

Hi Hannes,
I have tested the patch you had sent. Failover works fine.

But, we are seeing problems during the failback. It is causing continues mode-select thrashing(ping pong).

Reason for this is, handler does not know if the mode select is coming for failover or failback. Every mode select will cause movement of all the Luns. It does not matter if the LUNs are on preferred path or not. On next polling interval, multipathd will find some Luns are not on preferred path and will initiate another failback. This will result in continues ping pong. I have explained this with an EXAMPLE 1 below.

For failback to work properly, we have to have selective Lun level failover.

There is also one more Cluster scenario where we could get into thrashing with Controller Level failover. Please see the EXAMPLE 2 below.

We have been testing LUN level failover with device mapper for a while now. It is working well for us and only problem we have is slower failovers with big configurations(failover was taking about 12 minutes with 234 luns). LSI and IBM(Chandra) has been working on asynchronous behavior for the past 3-4 months. I have tested all the patches Chandra has posted and we have seen very good results(Failover takes only 1 minute with 234 luns).

Also, these patches give the opportunity for other handlers to move to asynchronous behavior if they wish to. We need your(and Linux community) help to review the patches and move forward on this issue.

Thanks
Babu Moger
LSI Corporation

Following are the two example where we could see mode-select thrashing..

EXAMPLE 1 (mode select thrashing with 2 Luns in single host)
=======================================================
Let's take a very simple example.

I have 2 Luns on my host. Host is seeing both the controllers with one path to each controller.

Lun 0 is owned by controller A and preferred owner is A. Lun 1 is owned by controller B and preferred owned is B

Here is multipath -ll output..

mpath237 (3600a0b80000f519c0000cc8a48fc7d0b) dm-4 LSI,INF-01-00 [size=2.0G][features=1 queue_if_no_path][hwhandler=1 rdac][rw] \_ round-robin 0 [prio=100][active]
\_ 1:0:0:0 sde 8:64 [active][ready] (controller A)
\_ round-robin 0 [prio=0][enabled]
\_ 2:0:0:0 sdi 8:128 [active][ghost] (controller B)

mpath180 (3600a0b80000f519c0000cc9048fc7d7b) dm-5 LSI,INF-01-00 [size=2.0G][features=1 queue_if_no_path][hwhandler=1 rdac][rw] \_ round-robin 0 [prio=100][active]
\_ 2:0:0:1 sdj 8:144 [active][ready] (controller B)
\_ round-robin 0 [prio=0][enabled]
\_ 1:0:0:1 sdf 8:80 [active][ghost] (controller A)

1. Run I/O on both these Luns
2. Pull the cable connected to controller A.
3. Failover will happen and Lun 0 will move to controller B. Now both the Luns are on controller B.
4. Connect the cable back on controller A.
5. multipath tool will detect the physical Luns on controller A and run the priority test.
6. It will find that Lun 0 is not on preferred path and will initiate a failback. Because it is a controller level failover it will move the Lun 1 also to controller A. Now both the Luns are on controller A.
7. Multipath tool will come again and find Lun 1 not on preferred path and initiate failback. This will both the Luns to controller B.
This will continue forever.

EXAMPLE 2: (mode select thrashing in cluster setup)
============================================================================
Let's take two node cluster environment where luns are visible across multiple nodes, although any given lun would only be accessible via one node at a time. If a cluster configuration were to get into a state where one node only has visibility to one controller while another node only has visibility to the alternate, a “thrashing” condition could happen. Take this example:

• 32 luns have been mapped from the storage to all nodes.
• Luns 0-15 are owned by the ‘A’ controller and being accessed by node #1; luns 16-31 are owned by ‘B’ and mapped to node #2.
• Node #1 only has access to the ‘A’ controller; node #2 only has access to the ‘B’ controller.

Let’s say Node #1 decides to access lun 16. Because it does not have visibility to the ‘B’ controller it must issue a volume transfer request. With Controller failover solution the volume transfer request would also move luns 17-31. If node #2 were accessing those luns they would receive ownership errors, causing a volume transfer request to move them back. However, this also moves lun 16 from ‘A’ back to ‘B’, causing node #1 to do the volume transfer request again…..etc.

Quoted 3 lines · Babu

1. Run I/O on both these Luns
2. Pull the cable connected to controller A.
3. Failover will happen and Lun 0 will move to controller B. Now both

the Luns are on controller B.

Quoted 2 lines

4. Connect the cable back on controller A.
5. multipath tool will detect the physical Luns on controller A and

run the priority test.

Quoted 1 line

6. It will find that Lun 0 is not on preferred path and will initiate

a failback.

Quoted 1 line

Because it is a controller level failover it will move the Lun 1

also to controller A.

Quoted 2 lines

Now both the Luns are on controller A.
7. Multipath tool will come again and find Lun 1 not on preferred

path and initiate failback.

Quoted 3 lines

This will both the Luns to controller B. This will continue forever.

Hmm. Yes, correct.
After all, the patch I sent was meant to be a proof of concept, not a fully fledged
solution. (In fact, I'm quite surprised it worked so well :-)

What about modifying the LUN select code to switch all _visible_ LUNs (ie all LUNs which
are _not_ on the preferred path) in one go?
That way we wouldn't run into this issue.

Hannes, Thanks again for the response.. Please see the comments below..

Quoted 23 lines

Quoted 3 lines · Babu

1. Run I/O on both these Luns
2. Pull the cable connected to controller A.
3. Failover will happen and Lun 0 will move to controller B. Now both

the Luns are on controller B.

Quoted 2 lines

4. Connect the cable back on controller A.
5. multipath tool will detect the physical Luns on controller A and

run the priority test.

Quoted 1 line

6. It will find that Lun 0 is not on preferred path and will initiate

a failback.

Quoted 1 line

Because it is a controller level failover it will move the Lun 1

also to controller A.

Quoted 2 lines

Now both the Luns are on controller A.
7. Multipath tool will come again and find Lun 1 not on preferred

path and initiate failback.

Quoted 3 lines

This will both the Luns to controller B. This will continue forever.

Hmm. Yes, correct.
After all, the patch I sent was meant to be a proof of concept, not a fully fledged
solution. (In fact, I'm quite surprised it worked so well :-)

Quoted 4 lines

What about modifying the LUN select code to switch all _visible_ LUNs (ie all LUNs which
are _not_ on the preferred path) in one go?
That way we wouldn't run into this issue.

In this case handler should be able to do two tasks.
1. move all the Luns when one of the path failed(failover)
2. move only the luns which are not there on preferred path(failback).

With the current code rdac_activate(struct scsi_device *sdev) we cannot make that decision.
There is no information passed from dm layer to know if this command is for task 1(failover) or task 2(failback).