[PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

This patch series makes the ib_srp driver better suited for use in a
H.A. setup because:
- multipathd is notified faster about transport layer failures.
- Transport layer failures reliably result in SCSI host removal.
- Switchover can be triggered explicitly by deleting an initiator
   device.
- Disconnecting from a target without unloading ib_srp is now possible.

This patch series is also available here:
http://github.com/bvanassche/linux/commits/srp-ha
And a backport of this patch series to kernels 3.6 and before is 
available here: http://github.com/bvanassche/ib_srp-backport.

Changes since v4:
- Added a patch for removing SCSI devices upon a port down event

Changes since v3:
- Restored the dev_loss_tmo and fast_io_fail_tmo sysfs attributes.
- Included a patch to fix an ib_srp crash that could be triggered by
   cable pulling.

Changes since v2:
- Addressed the v2 review comments.
- Dropped the patches that have already been merged.
- Dropped the patches for integration with multipathd.
- Dropped the micro-optimization of the IB completion handlers.

The individual patches are:
0001-ib_srp-Enlarge-block-layer-timeout.patch
0002-ib_srp-Eliminate-state-SRP_TARGET_CONNECTING.patch
0003-ib_srp-Introduce-srp_handle_qp_err.patch
0004-ib_srp-Suppress-superfluous-error-messages.patch
0005-ib_srp-Avoid-that-SCSI-error-handling-causes-trouble.patch
0006-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch
0007-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch
0008-ib_srp-Keep-processing-commands-during-host-removal.patch
0009-ib_srp-Make-srp_disconnect_target-wait-for-IB-comple.patch
0010-ib_srp-Document-sysfs-attributes.patch
0011-srp_transport-Fix-atttribute-registration.patch
0012-srp_transport-Simplify-attribute-initialization-code.patch
0013-srp_transport-Document-sysfs-attributes.patch
0014-ib_srp-Allow-SRP-disconnect-through-sysfs.patch
0015-ib_srp-Maintain-a-single-connection-per-I_T-nexus.patch
0016-srp_transport-Add-transport-layer-error-handling.patch
0017-ib_srp-Add-dev_loss_tmo-support.patch
0018-ib_srp-Remove-SCSI-devices-upon-port-down-event.patch
0019-scsi_transport_srp-Fail-I-O-faster.patch

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 01/19] ib_srp: Enlarge block layer timeout

[Bart Van Assche]

Enlarge the block layer timeout for disks such that it is above
the InfiniBand transport layer timeout.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   45 +++++++++++++++++++++++++++++++++++
 drivers/infiniband/ulp/srp/ib_srp.h |    2 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 922d845..6c0cd66 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1419,6 +1419,33 @@ err:
 	return -ENOMEM;
 }
 
+static uint32_t srp_compute_rq_tmo(struct ib_qp_attr *qp_attr, int attr_mask)
+{
+	uint64_t T_tr_ns, max_compl_time_ms;
+	uint32_t rq_tmo_jiffies;
+
+	/*
+	 * According to section 11.2.4.2 in the IBTA spec (Modify Queue Pair,
+	 * table 91), both the QP timeout and the retry count have to be set
+	 * for RC QP's during the RTR to RTS transition.
+	 */
+	WARN_ON((attr_mask & (IB_QP_TIMEOUT | IB_QP_RETRY_CNT)) !=
+		(IB_QP_TIMEOUT | IB_QP_RETRY_CNT));
+
+	/*
+	 * Set target->rq_tmo_jiffies to one second more than the largest time
+	 * it can take before an error completion is generated. See also
+	 * C9-140..142 in the IBTA spec for more information about how to
+	 * convert the QP Local ACK Timeout value to nanoseconds.
+	 */
+	T_tr_ns = 4096 * (1ULL << qp_attr->timeout);
+	max_compl_time_ms = qp_attr->retry_cnt * 4 * T_tr_ns;
+	do_div(max_compl_time_ms, NSEC_PER_MSEC);
+	rq_tmo_jiffies = msecs_to_jiffies(max_compl_time_ms + 1000);
+
+	return rq_tmo_jiffies;
+}
+
 static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 			       struct srp_login_rsp *lrsp,
 			       struct srp_target_port *target)
@@ -1478,6 +1505,8 @@ static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 	if (ret)
 		goto error_free;
 
+	target->rq_tmo_jiffies = srp_compute_rq_tmo(qp_attr, attr_mask);
+
 	ret = ib_modify_qp(target->qp, qp_attr, attr_mask);
 	if (ret)
 		goto error_free;
@@ -1729,6 +1758,21 @@ static int srp_reset_host(struct scsi_cmnd *scmnd)
 	return ret;
 }
 
+static int srp_slave_configure(struct scsi_device *sdev)
+{
+	struct Scsi_Host *shost = sdev->host;
+	struct srp_target_port *target = host_to_target(shost);
+	struct request_queue *q = sdev->request_queue;
+	unsigned long timeout;
+
+	if (sdev->type == TYPE_DISK) {
+		timeout = max_t(unsigned, 30 * HZ, target->rq_tmo_jiffies);
+		blk_queue_rq_timeout(q, timeout);
+	}
+
+	return 0;
+}
+
 static ssize_t show_id_ext(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -1861,6 +1905,7 @@ static struct scsi_host_template srp_template = {
 	.module				= THIS_MODULE,
 	.name				= "InfiniBand SRP initiator",
 	.proc_name			= DRV_NAME,
+	.slave_configure		= srp_slave_configure,
 	.info				= srp_target_info,
 	.queuecommand			= srp_queuecommand,
 	.eh_abort_handler		= srp_abort,
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 020caf0..e3a6304 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -163,6 +163,8 @@ struct srp_target_port {
 	struct ib_sa_query     *path_query;
 	int			path_query_id;
 
+	u32			rq_tmo_jiffies;
+
 	struct ib_cm_id	       *cm_id;
 
 	int			max_ti_iu_len;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 02/19] ib_srp: Eliminate state SRP_TARGET_CONNECTING

[Bart Van Assche]

Block the SCSI host while reconnecting instead of representing
the reconnection activity as a distinct SRP target state. This
allows to eliminate the target state SRP_TARGET_CONNECTING.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   18 ++++++++++--------
 drivers/infiniband/ulp/srp/ib_srp.h |    1 -
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 6c0cd66..ed89370 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -646,13 +646,16 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *re
 
 static int srp_reconnect_target(struct srp_target_port *target)
 {
+	struct Scsi_Host *shost = target->scsi_host;
 	struct ib_qp_attr qp_attr;
 	struct ib_wc wc;
 	int i, ret;
 
-	if (!srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_CONNECTING))
+	if (target->state != SRP_TARGET_LIVE)
 		return -EAGAIN;
 
+	scsi_target_block(&shost->shost_gendev);
+
 	srp_disconnect_target(target);
 	/*
 	 * Now get a new local CM ID so that we avoid confusing the
@@ -691,12 +694,15 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	if (ret)
 		goto err;
 
-	if (!srp_change_state(target, SRP_TARGET_CONNECTING, SRP_TARGET_LIVE))
-		ret = -EAGAIN;
+	scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
+
+	shost_printk(KERN_INFO, target->scsi_host, PFX "reconnect succeeded\n");
 
 	return ret;
 
 err:
+	scsi_target_unblock(&shost->shost_gendev, SDEV_TRANSPORT_OFFLINE);
+
 	shost_printk(KERN_ERR, target->scsi_host,
 		     PFX "reconnect failed (%d), removing target port.\n", ret);
 
@@ -710,7 +716,7 @@ err:
 	 * the flush_scheduled_work() in srp_remove_one().
 	 */
 	spin_lock_irq(&target->lock);
-	if (target->state == SRP_TARGET_CONNECTING) {
+	if (target->state == SRP_TARGET_LIVE) {
 		target->state = SRP_TARGET_DEAD;
 		INIT_WORK(&target->work, srp_remove_work);
 		queue_work(ib_wq, &target->work);
@@ -1311,9 +1317,6 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	unsigned long flags;
 	int len;
 
-	if (target->state == SRP_TARGET_CONNECTING)
-		goto err;
-
 	if (target->state == SRP_TARGET_DEAD ||
 	    target->state == SRP_TARGET_REMOVED) {
 		scmnd->result = DID_BAD_TARGET << 16;
@@ -1377,7 +1380,6 @@ err_iu:
 err_unlock:
 	spin_unlock_irqrestore(&target->lock, flags);
 
-err:
 	return SCSI_MLQUEUE_HOST_BUSY;
 }
 
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index e3a6304..8b436ce 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -80,7 +80,6 @@ enum {
 
 enum srp_target_state {
 	SRP_TARGET_LIVE,
-	SRP_TARGET_CONNECTING,
 	SRP_TARGET_DEAD,
 	SRP_TARGET_REMOVED
 };
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 03/19] ib_srp: Introduce srp_handle_qp_err()

[Bart Van Assche]

Introduce the function srp_handle_qp_err(), change the type of
qp_in_error from int into bool and move the initialization of that
variable from srp_reconnect_target() to srp_connect_target().

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   37 +++++++++++++++++++----------------
 drivers/infiniband/ulp/srp/ib_srp.h |    2 +-
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index ed89370..87f0bfd 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -515,6 +515,8 @@ static int srp_connect_target(struct srp_target_port *target)
 	int retries = 3;
 	int ret;
 
+	target->qp_in_error = false;
+
 	ret = srp_lookup_path(target);
 	if (ret)
 		return ret;
@@ -689,7 +691,6 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	for (i = 0; i < SRP_SQ_SIZE; ++i)
 		list_add(&target->tx_ring[i]->list, &target->free_tx);
 
-	target->qp_in_error = 0;
 	ret = srp_connect_target(target);
 	if (ret)
 		goto err;
@@ -1268,6 +1269,15 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
 			     PFX "Recv failed with error code %d\n", res);
 }
 
+static void srp_handle_qp_err(enum ib_wc_status wc_status,
+			      enum ib_wc_opcode wc_opcode,
+			      struct srp_target_port *target)
+{
+	shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n",
+		     wc_opcode & IB_WC_RECV ? "receive" : "send", wc_status);
+	target->qp_in_error = true;
+}
+
 static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
 {
 	struct srp_target_port *target = target_ptr;
@@ -1275,15 +1285,12 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
 
 	ib_req_notify_cq(cq, IB_CQ_NEXT_COMP);
 	while (ib_poll_cq(cq, 1, &wc) > 0) {
-		if (wc.status) {
-			shost_printk(KERN_ERR, target->scsi_host,
-				     PFX "failed receive status %d\n",
-				     wc.status);
-			target->qp_in_error = 1;
+		if (likely(wc.status == IB_WC_SUCCESS)) {
+			srp_handle_recv(target, &wc);
+		} else {
+			srp_handle_qp_err(wc.status, wc.opcode, target);
 			break;
 		}
-
-		srp_handle_recv(target, &wc);
 	}
 }
 
@@ -1294,16 +1301,13 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
 	struct srp_iu *iu;
 
 	while (ib_poll_cq(cq, 1, &wc) > 0) {
-		if (wc.status) {
-			shost_printk(KERN_ERR, target->scsi_host,
-				     PFX "failed send status %d\n",
-				     wc.status);
-			target->qp_in_error = 1;
+		if (likely(wc.status == IB_WC_SUCCESS)) {
+			iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
+			list_add(&iu->list, &target->free_tx);
+		} else {
+			srp_handle_qp_err(wc.status, wc.opcode, target);
 			break;
 		}
-
-		iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
-		list_add(&iu->list, &target->free_tx);
 	}
 }
 
@@ -2279,7 +2283,6 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_ib;
 
-	target->qp_in_error = 0;
 	ret = srp_connect_target(target);
 	if (ret) {
 		shost_printk(KERN_ERR, target->scsi_host,
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 8b436ce..02dc3ac 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -179,7 +179,7 @@ struct srp_target_port {
 	struct list_head	list;
 	struct completion	done;
 	int			status;
-	int			qp_in_error;
+	bool			qp_in_error;
 
 	struct completion	tsk_mgmt_done;
 	u8			tsk_mgmt_status;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 04/19] ib_srp: Suppress superfluous error messages

[Bart Van Assche]

Keep track of the connection state. Only report QP errors while
connected. Only invoke ib_send_cm_dreq() when connected such that
invoking srp_disconnect_target() after having received a DREQ
does not cause an error message to be printed.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   43 +++++++++++++++++++++++++++--------
 drivers/infiniband/ulp/srp/ib_srp.h |    1 +
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 87f0bfd..01b6cd7 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -428,17 +428,34 @@ static int srp_send_req(struct srp_target_port *target)
 	return status;
 }
 
+static bool srp_change_conn_state(struct srp_target_port *target,
+				  bool connected)
+{
+	bool changed = false;
+
+	spin_lock_irq(&target->lock);
+	if (target->connected != connected) {
+		target->connected = connected;
+		changed = true;
+	}
+	spin_unlock_irq(&target->lock);
+
+	return changed;
+}
+
 static void srp_disconnect_target(struct srp_target_port *target)
 {
-	/* XXX should send SRP_I_LOGOUT request */
+	if (srp_change_conn_state(target, false)) {
+		/* XXX should send SRP_I_LOGOUT request */
 
-	init_completion(&target->done);
-	if (ib_send_cm_dreq(target->cm_id, NULL, 0)) {
-		shost_printk(KERN_DEBUG, target->scsi_host,
-			     PFX "Sending CM DREQ failed\n");
-		return;
+		init_completion(&target->done);
+		if (ib_send_cm_dreq(target->cm_id, NULL, 0)) {
+			shost_printk(KERN_DEBUG, target->scsi_host,
+				     PFX "Sending CM DREQ failed\n");
+		} else {
+			wait_for_completion(&target->done);
+		}
 	}
-	wait_for_completion(&target->done);
 }
 
 static bool srp_change_state(struct srp_target_port *target,
@@ -515,6 +532,8 @@ static int srp_connect_target(struct srp_target_port *target)
 	int retries = 3;
 	int ret;
 
+	WARN_ON(target->connected);
+
 	target->qp_in_error = false;
 
 	ret = srp_lookup_path(target);
@@ -536,6 +555,7 @@ static int srp_connect_target(struct srp_target_port *target)
 		 */
 		switch (target->status) {
 		case 0:
+			srp_change_conn_state(target, true);
 			return 0;
 
 		case SRP_PORT_REDIRECT:
@@ -1273,8 +1293,11 @@ static void srp_handle_qp_err(enum ib_wc_status wc_status,
 			      enum ib_wc_opcode wc_opcode,
 			      struct srp_target_port *target)
 {
-	shost_printk(KERN_ERR, target->scsi_host, PFX "failed %s status %d\n",
-		     wc_opcode & IB_WC_RECV ? "receive" : "send", wc_status);
+	if (target->connected)
+		shost_printk(KERN_ERR, target->scsi_host,
+			     PFX "failed %s status %d\n",
+			     wc_opcode & IB_WC_RECV ? "receive" : "send",
+			     wc_status);
 	target->qp_in_error = true;
 }
 
@@ -1634,6 +1657,7 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 	case IB_CM_DREQ_RECEIVED:
 		shost_printk(KERN_WARNING, target->scsi_host,
 			     PFX "DREQ received - connection closed\n");
+		srp_change_conn_state(target, false);
 		if (ib_send_cm_drep(cm_id, NULL, 0))
 			shost_printk(KERN_ERR, target->scsi_host,
 				     PFX "Sending CM DREP failed\n");
@@ -1950,6 +1974,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 	spin_unlock(&host->target_lock);
 
 	target->state = SRP_TARGET_LIVE;
+	target->connected = false;
 
 	scsi_scan_target(&target->scsi_host->shost_gendev,
 			 0, target->scsi_id, SCAN_WILD_CARD, 0);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 02dc3ac..ef95fa4 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -163,6 +163,7 @@ struct srp_target_port {
 	int			path_query_id;
 
 	u32			rq_tmo_jiffies;
+	bool			connected;
 
 	struct ib_cm_id	       *cm_id;
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 05/19] ib_srp: Avoid that SCSI error handling causes trouble

[Bart Van Assche]

Sending data over a queue pair after the corresponding cm_id has
been destroyed is wrong. The HCA will send the data anyway and
the data may be sent to another system to a queue pair that is in
use. The data will get processed and a response will be sent back.
An application will receive data that it did not expect. This can
result in a kernel oops.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 01b6cd7..abcefa1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1695,8 +1695,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,
 	struct srp_iu *iu;
 	struct srp_tsk_mgmt *tsk_mgmt;
 
-	if (target->state == SRP_TARGET_DEAD ||
-	    target->state == SRP_TARGET_REMOVED)
+	if (!target->connected)
 		return -1;
 
 	init_completion(&target->tsk_mgmt_done);
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 06/19] ib_srp: Introduce the helper function srp_remove_target()

[Bart Van Assche]

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index abcefa1..be9c7fe 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -506,6 +506,17 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)
 		device_remove_file(&shost->shost_dev, *attr);
 }
 
+static void srp_remove_target(struct srp_target_port *target)
+{
+	srp_del_scsi_host_attr(target->scsi_host);
+	srp_remove_host(target->scsi_host);
+	scsi_remove_host(target->scsi_host);
+	ib_destroy_cm_id(target->cm_id);
+	srp_free_target_ib(target);
+	srp_free_req_data(target);
+	scsi_host_put(target->scsi_host);
+}
+
 static void srp_remove_work(struct work_struct *work)
 {
 	struct srp_target_port *target =
@@ -518,13 +529,7 @@ static void srp_remove_work(struct work_struct *work)
 	list_del(&target->list);
 	spin_unlock(&target->srp_host->target_lock);
 
-	srp_del_scsi_host_attr(target->scsi_host);
-	srp_remove_host(target->scsi_host);
-	scsi_remove_host(target->scsi_host);
-	ib_destroy_cm_id(target->cm_id);
-	srp_free_target_ib(target);
-	srp_free_req_data(target);
-	scsi_host_put(target->scsi_host);
+	srp_remove_target(target);
 }
 
 static int srp_connect_target(struct srp_target_port *target)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 07/19] ib_srp: Eliminate state SRP_TARGET_DEAD

[Bart Van Assche]

Only queue removal work after having changed the target state
into SRP_TARGET_REMOVED and not if that state was already equal
to SRP_TARGET_REMOVED. That allows to remove the state
SRP_TARGET_DEAD. Add a call to srp_disconnect_target() in
srp_remove_target() - due to previous changes it is now safe to
invoke that last function even if the IB connection has already
been disconnected. This change allows to replace the target
removal code in srp_remove_one() by an (indirect) call to
srp_remove_target(). Rename srp_target_port.work into
srp_target_port.remove_work to reflect its usage.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   86 +++++++++++++----------------------
 drivers/infiniband/ulp/srp/ib_srp.h |    5 +-
 2 files changed, 33 insertions(+), 58 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index be9c7fe..23612c1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -428,6 +428,23 @@ static int srp_send_req(struct srp_target_port *target)
 	return status;
 }
 
+static bool srp_queue_remove_work(struct srp_target_port *target)
+{
+	bool changed = false;
+
+	spin_lock_irq(&target->lock);
+	if (target->state != SRP_TARGET_REMOVED) {
+		target->state = SRP_TARGET_REMOVED;
+		changed = true;
+	}
+	spin_unlock_irq(&target->lock);
+
+	if (changed)
+		queue_work(system_long_wq, &target->remove_work);
+
+	return changed;
+}
+
 static bool srp_change_conn_state(struct srp_target_port *target,
 				  bool connected)
 {
@@ -458,21 +475,6 @@ static void srp_disconnect_target(struct srp_target_port *target)
 	}
 }
 
-static bool srp_change_state(struct srp_target_port *target,
-			    enum srp_target_state old,
-			    enum srp_target_state new)
-{
-	bool changed = false;
-
-	spin_lock_irq(&target->lock);
-	if (target->state == old) {
-		target->state = new;
-		changed = true;
-	}
-	spin_unlock_irq(&target->lock);
-	return changed;
-}
-
 static void srp_free_req_data(struct srp_target_port *target)
 {
 	struct ib_device *ibdev = target->srp_host->srp_dev->dev;
@@ -508,9 +510,12 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)
 
 static void srp_remove_target(struct srp_target_port *target)
 {
+	WARN_ON(target->state != SRP_TARGET_REMOVED);
+
 	srp_del_scsi_host_attr(target->scsi_host);
 	srp_remove_host(target->scsi_host);
 	scsi_remove_host(target->scsi_host);
+	srp_disconnect_target(target);
 	ib_destroy_cm_id(target->cm_id);
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
@@ -520,10 +525,9 @@ static void srp_remove_target(struct srp_target_port *target)
 static void srp_remove_work(struct work_struct *work)
 {
 	struct srp_target_port *target =
-		container_of(work, struct srp_target_port, work);
+		container_of(work, struct srp_target_port, remove_work);
 
-	if (!srp_change_state(target, SRP_TARGET_DEAD, SRP_TARGET_REMOVED))
-		return;
+	WARN_ON(target->state != SRP_TARGET_REMOVED);
 
 	spin_lock(&target->srp_host->target_lock);
 	list_del(&target->list);
@@ -737,17 +741,8 @@ err:
 	 * However, we have to defer the real removal because we
 	 * are in the context of the SCSI error handler now, which
 	 * will deadlock if we call scsi_remove_host().
-	 *
-	 * Schedule our work inside the lock to avoid a race with
-	 * the flush_scheduled_work() in srp_remove_one().
 	 */
-	spin_lock_irq(&target->lock);
-	if (target->state == SRP_TARGET_LIVE) {
-		target->state = SRP_TARGET_DEAD;
-		INIT_WORK(&target->work, srp_remove_work);
-		queue_work(ib_wq, &target->work);
-	}
-	spin_unlock_irq(&target->lock);
+	srp_queue_remove_work(target);
 
 	return ret;
 }
@@ -1349,8 +1344,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	unsigned long flags;
 	int len;
 
-	if (target->state == SRP_TARGET_DEAD ||
-	    target->state == SRP_TARGET_REMOVED) {
+	if (target->state == SRP_TARGET_REMOVED) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
 		return 0;
@@ -2268,6 +2262,7 @@ static ssize_t srp_create_target(struct device *dev,
 			     sizeof (struct srp_indirect_buf) +
 			     target->cmd_sg_cnt * sizeof (struct srp_direct_buf);
 
+	INIT_WORK(&target->remove_work, srp_remove_work);
 	spin_lock_init(&target->lock);
 	INIT_LIST_HEAD(&target->free_tx);
 	INIT_LIST_HEAD(&target->free_reqs);
@@ -2501,8 +2496,7 @@ static void srp_remove_one(struct ib_device *device)
 {
 	struct srp_device *srp_dev;
 	struct srp_host *host, *tmp_host;
-	LIST_HEAD(target_list);
-	struct srp_target_port *target, *tmp_target;
+	struct srp_target_port *target;
 
 	srp_dev = ib_get_client_data(device, &srp_client);
 
@@ -2515,35 +2509,17 @@ static void srp_remove_one(struct ib_device *device)
 		wait_for_completion(&host->released);
 
 		/*
-		 * Mark all target ports as removed, so we stop queueing
-		 * commands and don't try to reconnect.
+		 * Remove all target ports.
 		 */
 		spin_lock(&host->target_lock);
-		list_for_each_entry(target, &host->target_list, list) {
-			spin_lock_irq(&target->lock);
-			target->state = SRP_TARGET_REMOVED;
-			spin_unlock_irq(&target->lock);
-		}
+		list_for_each_entry(target, &host->target_list, list)
+			srp_queue_remove_work(target);
 		spin_unlock(&host->target_lock);
 
 		/*
-		 * Wait for any reconnection tasks that may have
-		 * started before we marked our target ports as
-		 * removed, and any target port removal tasks.
+		 * Wait for target port removal tasks.
 		 */
-		flush_workqueue(ib_wq);
-
-		list_for_each_entry_safe(target, tmp_target,
-					 &host->target_list, list) {
-			srp_del_scsi_host_attr(target->scsi_host);
-			srp_remove_host(target->scsi_host);
-			scsi_remove_host(target->scsi_host);
-			srp_disconnect_target(target);
-			ib_destroy_cm_id(target->cm_id);
-			srp_free_target_ib(target);
-			srp_free_req_data(target);
-			scsi_host_put(target->scsi_host);
-		}
+		flush_workqueue(system_long_wq);
 
 		kfree(host);
 	}
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index ef95fa4..de2d0b3 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -80,8 +80,7 @@ enum {
 
 enum srp_target_state {
 	SRP_TARGET_LIVE,
-	SRP_TARGET_DEAD,
-	SRP_TARGET_REMOVED
+	SRP_TARGET_REMOVED,
 };
 
 enum srp_iu_type {
@@ -175,7 +174,7 @@ struct srp_target_port {
 	struct srp_iu	       *rx_ring[SRP_RQ_SIZE];
 	struct srp_request	req_ring[SRP_CMD_SQ_SIZE];
 
-	struct work_struct	work;
+	struct work_struct	remove_work;
 
 	struct list_head	list;
 	struct completion	done;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 08/19] ib_srp: Keep processing commands during host removal

[Bart Van Assche]

Some SCSI upper layer drivers, e.g. sd, issue SCSI commands from
inside scsi_remove_host() (see also the sd_shutdown() call in
sd_remove()). Make sure that these commands have a chance to reach
the SCSI device.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 23612c1..8ae2070 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1344,12 +1344,6 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	unsigned long flags;
 	int len;
 
-	if (target->state == SRP_TARGET_REMOVED) {
-		scmnd->result = DID_BAD_TARGET << 16;
-		scmnd->scsi_done(scmnd);
-		return 0;
-	}
-
 	spin_lock_irqsave(&target->lock, flags);
 	iu = __srp_get_tx_iu(target, SRP_IU_CMD);
 	if (!iu)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 09/19] ib_srp: Make srp_disconnect_target() wait for IB completions

[Bart Van Assche]

Modify srp_disconnect_target() such that it waits until it is
sure that no new IB completions will be received anymore.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |  104 ++++++++++++++++++++++++++++++-----
 drivers/infiniband/ulp/srp/ib_srp.h |    6 ++
 2 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8ae2070..58d19a2 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -40,7 +40,7 @@
 #include <linux/parser.h>
 #include <linux/random.h>
 #include <linux/jiffies.h>
-
+#include <linux/delay.h>
 #include <linux/atomic.h>
 
 #include <scsi/scsi.h>
@@ -229,14 +229,16 @@ static int srp_create_target_ib(struct srp_target_port *target)
 		return -ENOMEM;
 
 	target->recv_cq = ib_create_cq(target->srp_host->srp_dev->dev,
-				       srp_recv_completion, NULL, target, SRP_RQ_SIZE, 0);
+				       srp_recv_completion, NULL, target,
+				       SRP_RQ_SIZE + 1, 0);
 	if (IS_ERR(target->recv_cq)) {
 		ret = PTR_ERR(target->recv_cq);
 		goto err;
 	}
 
 	target->send_cq = ib_create_cq(target->srp_host->srp_dev->dev,
-				       srp_send_completion, NULL, target, SRP_SQ_SIZE, 0);
+				       srp_send_completion, NULL, target,
+				       SRP_SQ_SIZE + 1, 0);
 	if (IS_ERR(target->send_cq)) {
 		ret = PTR_ERR(target->send_cq);
 		goto err_recv_cq;
@@ -245,8 +247,8 @@ static int srp_create_target_ib(struct srp_target_port *target)
 	ib_req_notify_cq(target->recv_cq, IB_CQ_NEXT_COMP);
 
 	init_attr->event_handler       = srp_qp_event;
-	init_attr->cap.max_send_wr     = SRP_SQ_SIZE;
-	init_attr->cap.max_recv_wr     = SRP_RQ_SIZE;
+	init_attr->cap.max_send_wr     = SRP_SQ_SIZE + 1;
+	init_attr->cap.max_recv_wr     = SRP_RQ_SIZE + 1;
 	init_attr->cap.max_recv_sge    = 1;
 	init_attr->cap.max_send_sge    = 1;
 	init_attr->sq_sig_type         = IB_SIGNAL_ALL_WR;
@@ -460,11 +462,69 @@ static bool srp_change_conn_state(struct srp_target_port *target,
 	return changed;
 }
 
+static void srp_wait_last_recv_wqe(struct srp_target_port *target)
+{
+	static struct ib_recv_wr wr = {
+		.wr_id = SRP_LAST_RECV,
+	};
+	struct ib_recv_wr *bad_wr;
+	int ret;
+
+	if (target->last_recv_wqe)
+		return;
+
+	ret = ib_post_recv(target->qp, &wr, &bad_wr);
+	if (ret < 0) {
+		shost_printk(KERN_ERR, target->scsi_host,
+			     "ib_post_recv() failed (%d)\n", ret);
+		return;
+	}
+
+	ret = wait_event_timeout(target->qp_wq, target->last_recv_wqe,
+				 target->rq_tmo_jiffies);
+	WARN(ret <= 0, "Timeout while waiting for last recv WQE (ret = %d)\n",
+	     ret);
+}
+
+static void srp_wait_last_send_wqe(struct srp_target_port *target)
+{
+	static struct ib_send_wr wr = {
+		.wr_id = SRP_LAST_SEND,
+	};
+	struct ib_send_wr *bad_wr;
+	unsigned long deadline = jiffies + target->rq_tmo_jiffies;
+	int ret;
+
+	if (target->last_send_wqe)
+		return;
+
+	ret = ib_post_send(target->qp, &wr, &bad_wr);
+	if (ret < 0) {
+		shost_printk(KERN_ERR, target->scsi_host,
+			     "ib_post_send() failed (%d)\n", ret);
+		return;
+	}
+
+	while (!target->last_send_wqe && time_before(jiffies, deadline)) {
+		srp_send_completion(target->send_cq, target);
+		msleep(20);
+	}
+
+	WARN_ON(!target->last_send_wqe);
+}
+
 static void srp_disconnect_target(struct srp_target_port *target)
 {
+	static struct ib_qp_attr qp_attr = {
+		.qp_state = IB_QPS_ERR
+	};
+	int ret;
+
 	if (srp_change_conn_state(target, false)) {
 		/* XXX should send SRP_I_LOGOUT request */
 
+		BUG_ON(!target->cm_id);
+
 		init_completion(&target->done);
 		if (ib_send_cm_dreq(target->cm_id, NULL, 0)) {
 			shost_printk(KERN_DEBUG, target->scsi_host,
@@ -473,6 +533,20 @@ static void srp_disconnect_target(struct srp_target_port *target)
 			wait_for_completion(&target->done);
 		}
 	}
+
+	if (target->cm_id) {
+		ib_destroy_cm_id(target->cm_id);
+		target->cm_id = NULL;
+	}
+
+	if (target->qp) {
+		ret = ib_modify_qp(target->qp, &qp_attr, IB_QP_STATE);
+		WARN(ret != 0, "ib_modify_qp() failed: %d\n", ret);
+
+		srp_wait_last_recv_wqe(target);
+
+		srp_wait_last_send_wqe(target);
+	}
 }
 
 static void srp_free_req_data(struct srp_target_port *target)
@@ -516,7 +590,6 @@ static void srp_remove_target(struct srp_target_port *target)
 	srp_remove_host(target->scsi_host);
 	scsi_remove_host(target->scsi_host);
 	srp_disconnect_target(target);
-	ib_destroy_cm_id(target->cm_id);
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
 	scsi_host_put(target->scsi_host);
@@ -544,6 +617,8 @@ static int srp_connect_target(struct srp_target_port *target)
 	WARN_ON(target->connected);
 
 	target->qp_in_error = false;
+	target->last_recv_wqe = false;
+	target->last_send_wqe = false;
 
 	ret = srp_lookup_path(target);
 	if (ret)
@@ -679,7 +754,6 @@ static int srp_reconnect_target(struct srp_target_port *target)
 {
 	struct Scsi_Host *shost = target->scsi_host;
 	struct ib_qp_attr qp_attr;
-	struct ib_wc wc;
 	int i, ret;
 
 	if (target->state != SRP_TARGET_LIVE)
@@ -705,11 +779,6 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	if (ret)
 		goto err;
 
-	while (ib_poll_cq(target->recv_cq, 1, &wc) > 0)
-		; /* nothing */
-	while (ib_poll_cq(target->send_cq, 1, &wc) > 0)
-		; /* nothing */
-
 	for (i = 0; i < SRP_CMD_SQ_SIZE; ++i) {
 		struct srp_request *req = &target->req_ring[i];
 		if (req->scmnd)
@@ -1293,7 +1362,7 @@ static void srp_handle_qp_err(enum ib_wc_status wc_status,
 			      enum ib_wc_opcode wc_opcode,
 			      struct srp_target_port *target)
 {
-	if (target->connected)
+	if (target->connected && !target->qp_in_error)
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "failed %s status %d\n",
 			     wc_opcode & IB_WC_RECV ? "receive" : "send",
@@ -1311,8 +1380,11 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
 		if (likely(wc.status == IB_WC_SUCCESS)) {
 			srp_handle_recv(target, &wc);
 		} else {
+			if (wc.wr_id == SRP_LAST_RECV) {
+				target->last_recv_wqe = true;
+				wake_up(&target->qp_wq);
+			}
 			srp_handle_qp_err(wc.status, wc.opcode, target);
-			break;
 		}
 	}
 }
@@ -1328,8 +1400,9 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
 			iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
 			list_add(&iu->list, &target->free_tx);
 		} else {
+			if (wc.wr_id == SRP_LAST_SEND)
+				target->last_send_wqe = true;
 			srp_handle_qp_err(wc.status, wc.opcode, target);
-			break;
 		}
 	}
 }
@@ -2260,6 +2333,7 @@ static ssize_t srp_create_target(struct device *dev,
 	spin_lock_init(&target->lock);
 	INIT_LIST_HEAD(&target->free_tx);
 	INIT_LIST_HEAD(&target->free_reqs);
+	init_waitqueue_head(&target->qp_wq);
 	for (i = 0; i < SRP_CMD_SQ_SIZE; ++i) {
 		struct srp_request *req = &target->req_ring[i];
 
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index de2d0b3..1b11117 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -76,6 +76,9 @@ enum {
 
 	SRP_MAP_ALLOW_FMR	= 0,
 	SRP_MAP_NO_FMR		= 1,
+
+	SRP_LAST_RECV		= 0,
+	SRP_LAST_SEND		= 0,
 };
 
 enum srp_target_state {
@@ -180,6 +183,9 @@ struct srp_target_port {
 	struct completion	done;
 	int			status;
 	bool			qp_in_error;
+	bool			last_recv_wqe;
+	bool			last_send_wqe;
+	wait_queue_head_t	qp_wq;
 
 	struct completion	tsk_mgmt_done;
 	u8			tsk_mgmt_status;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 10/19] ib_srp: Document sysfs attributes

[Bart Van Assche]

Document the sysfs attributes of the SRP initiator (ib_srp) according
to the rules specified in Documentation/ABI/README.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 Documentation/ABI/stable/sysfs-driver-ib_srp |  156 ++++++++++++++++++++++++++
 1 file changed, 156 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-driver-ib_srp

diff --git a/Documentation/ABI/stable/sysfs-driver-ib_srp b/Documentation/ABI/stable/sysfs-driver-ib_srp
new file mode 100644
index 0000000..481aae9
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-driver-ib_srp
@@ -0,0 +1,156 @@
+What:		/sys/class/infiniband_srp/srp-<hca>-<port_number>/add_target
+Date:		January 2, 2006
+KernelVersion:	2.6.15
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Interface for making ib_srp connect to a new target.
+		One can request ib_srp to connect to a new target by writing
+		a comma-separated list of login parameters to this sysfs
+		attribute. The supported parameters are:
+		* id_ext, a 16-digit hexadecimal number specifying the eight
+		  byte identifier extension in the 16-byte SRP target port
+		  identifier. The target port identifier is sent by ib_srp
+		  to the target in the SRP_LOGIN_REQ request.
+		* ioc_guid, a 16-digit hexadecimal number specifying the eight
+		  byte I/O controller GUID portion of the 16-byte target port
+		  identifier.
+		* dgid, a 32-digit hexadecimal number specifying the
+		  destination GID.
+		* pkey, a four-digit hexadecimal number specifying the
+		  InfiniBand partition key.
+		* service_id, a 16-digit hexadecimal number specifying the
+		  InfiniBand service ID used to establish communication with
+		  the SRP target. How to find out the value of the service ID
+		  is specified in the documentation of the SRP target.
+		* max_sect, a decimal number specifying the maximum number of
+		  512-byte sectors to be transferred via a single SCSI command.
+		* max_cmd_per_lun, a decimal number specifying the maximum
+		  number of outstanding commands for a single LUN.
+		* io_class, a hexadecimal number specifying the SRP I/O class.
+		  Must be either 0xff00 (rev 10) or 0x0100 (rev 16a). The I/O
+		  class defines the format of the SRP initiator and target
+		  port identifiers.
+		* initiator_ext, a 16-digit hexadecimal number specifying the
+		  identifier extension portion of the SRP initiator port
+		  identifier. This data is sent by the initiator to the target
+		  in the SRP_LOGIN_REQ request.
+		* cmd_sg_entries, a number in the range 1..255 that specifies
+		  the maximum number of data buffer descriptors stored in the
+		  SRP_CMD information unit itself. With allow_ext_sg=0 the
+		  parameter cmd_sg_entries defines the maximum S/G list length
+		  for a single SRP_CMD, and commands whose S/G list length
+		  exceeds this limit after S/G list collapsing will fail.
+		* allow_ext_sg, whether ib_srp is allowed to include a partial
+		  memory descriptor list in an SRP_CMD instead of the entire
+		  list. If a partial memory descriptor list has been included
+		  in an SRP_CMD the remaining memory descriptors are
+		  communicated from initiator to target via an additional RDMA
+		  transfer. Setting allow_ext_sg to 1 increases the maximum
+		  amount of data that can be transferred between initiator and
+		  target via a single SCSI command. Since not all SRP target
+		  implementations support partial memory descriptor lists the
+		  default value for this option is 0.
+		* sg_tablesize, a number in the range 1..2048 specifying the
+		  maximum S/G list length the SCSI layer is allowed to pass to
+		  ib_srp. Specifying a value that exceeds cmd_sg_entries is
+		  only safe with partial memory descriptor list support enabled
+		  (allow_ext_sg=1).
+
+What:		/sys/class/infiniband_srp/srp-<hca>-<port_number>/ibdev
+Date:		January 2, 2006
+KernelVersion:	2.6.15
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	HCA name (<hca>).
+
+What:		/sys/class/infiniband_srp/srp-<hca>-<port_number>/port
+Date:		January 2, 2006
+KernelVersion:	2.6.15
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	HCA port number (<port_number>).
+
+What:		/sys/class/scsi_host/host<n>/allow_ext_sg
+Date:		May 19, 2011
+KernelVersion:	2.6.39
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Whether ib_srp is allowed to include a partial memory
+		descriptor list in an SRP_CMD when communicating with an SRP
+		target.
+
+What:		/sys/class/scsi_host/host<n>/cmd_sg_entries
+Date:		May 19, 2011
+KernelVersion:	2.6.39
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Maximum number of data buffer descriptors that may be sent to
+		the target in a single SRP_CMD request.
+
+What:		/sys/class/scsi_host/host<n>/dgid
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	InfiniBand destination GID used for communication with the SRP
+		target. Differs from orig_dgid if port redirection has happened.
+
+What:		/sys/class/scsi_host/host<n>/id_ext
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Eight-byte identifier extension portion of the 16-byte target
+		port identifier.
+
+What:		/sys/class/scsi_host/host<n>/ioc_guid
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Eight-byte I/O controller GUID portion of the 16-byte target
+		port identifier.
+
+What:		/sys/class/scsi_host/host<n>/local_ib_device
+Date:		November 29, 2006
+KernelVersion:	2.6.19
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Name of the InfiniBand HCA used for communicating with the
+		SRP target.
+
+What:		/sys/class/scsi_host/host<n>/local_ib_port
+Date:		November 29, 2006
+KernelVersion:	2.6.19
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Number of the HCA port used for communicating with the
+		SRP target.
+
+What:		/sys/class/scsi_host/host<n>/orig_dgid
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	InfiniBand destination GID specified in the parameters
+		written to the add_target sysfs attribute.
+
+What:		/sys/class/scsi_host/host<n>/pkey
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	A 16-bit number representing the InfiniBand partition key used
+		for communication with the SRP target.
+
+What:		/sys/class/scsi_host/host<n>/req_lim
+Date:		October 20, 2010
+KernelVersion:	2.6.36
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Number of requests ib_srp can send to the target before it has
+		to wait for more credits. For more information see also the
+		SRP credit algorithm in the SRP specification.
+
+What:		/sys/class/scsi_host/host<n>/service_id
+Date:		June 17, 2006
+KernelVersion:	2.6.17
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	InfiniBand service ID used for establishing communication with
+		the SRP	target.
+
+What:		/sys/class/scsi_host/host<n>/zero_req_lim
+Date:		September 20, 2006
+KernelVersion:	2.6.18
+Contact:	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Number of times the initiator had to wait before sending a
+		request to the target because it ran out of credits. For more
+		information see also the SRP credit algorithm in the SRP
+		specification.
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 11/19] srp_transport: Fix attribute registration

[Bart Van Assche]

Register transport attributes after the attribute array has been
set up instead of before. The current code can trigger a race
condition because the code reading the attribute array can run
on another thread than the code that initialized that array.
Make sure that any code reading the attribute array will see all
values written into that array.

Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>
Cc: David Dillow <dillowda@ornl.gov>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_transport_srp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 21a045e..07c4394 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -324,13 +324,14 @@ srp_attach_transport(struct srp_function_template *ft)
 	i->rport_attr_cont.ac.attrs = &i->rport_attrs[0];
 	i->rport_attr_cont.ac.class = &srp_rport_class.class;
 	i->rport_attr_cont.ac.match = srp_rport_match;
-	transport_container_register(&i->rport_attr_cont);
 
 	count = 0;
 	SETUP_RPORT_ATTRIBUTE_RD(port_id);
 	SETUP_RPORT_ATTRIBUTE_RD(roles);
 	i->rport_attrs[count] = NULL;
 
+	transport_container_register(&i->rport_attr_cont);
+
 	i->f = ft;
 
 	return &i->t;
-- 
1.7.10.4

[PATCH 12/19] srp_transport: Simplify attribute initialization code

[Bart Van Assche]

Eliminate the private_rport_attrs[] array and the SETUP_*() macros
used to set up that array since the information in that array
duplicates the information in the static device attributes. Also,
verify whether SRP_RPORT_ATTRS is large enough since it is easy
to forget to update that macro when adding new attributes.

Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Robert Jennings <rcj-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c |   26 ++++----------------------
 1 file changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 07c4394..0d85f79 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -47,7 +47,6 @@ struct srp_internal {
 	struct device_attribute *host_attrs[SRP_HOST_ATTRS + 1];
 
 	struct device_attribute *rport_attrs[SRP_RPORT_ATTRS + 1];
-	struct device_attribute private_rport_attrs[SRP_RPORT_ATTRS];
 	struct transport_container rport_attr_cont;
 };
 
@@ -72,24 +71,6 @@ static DECLARE_TRANSPORT_CLASS(srp_host_class, "srp_host", srp_host_setup,
 static DECLARE_TRANSPORT_CLASS(srp_rport_class, "srp_remote_ports",
 			       NULL, NULL, NULL);
 
-#define SETUP_TEMPLATE(attrb, field, perm, test, ro_test, ro_perm)	\
-	i->private_##attrb[count] = dev_attr_##field;		\
-	i->private_##attrb[count].attr.mode = perm;			\
-	if (ro_test) {							\
-		i->private_##attrb[count].attr.mode = ro_perm;		\
-		i->private_##attrb[count].store = NULL;			\
-	}								\
-	i->attrb[count] = &i->private_##attrb[count];			\
-	if (test)							\
-		count++
-
-#define SETUP_RPORT_ATTRIBUTE_RD(field)					\
-	SETUP_TEMPLATE(rport_attrs, field, S_IRUGO, 1, 0, 0)
-
-#define SETUP_RPORT_ATTRIBUTE_RW(field)					\
-	SETUP_TEMPLATE(rport_attrs, field, S_IRUGO | S_IWUSR,		\
-		       1, 1, S_IRUGO)
-
 #define SRP_PID(p) \
 	(p)->port_id[0], (p)->port_id[1], (p)->port_id[2], (p)->port_id[3], \
 	(p)->port_id[4], (p)->port_id[5], (p)->port_id[6], (p)->port_id[7], \
@@ -326,9 +307,10 @@ srp_attach_transport(struct srp_function_template *ft)
 	i->rport_attr_cont.ac.match = srp_rport_match;
 
 	count = 0;
-	SETUP_RPORT_ATTRIBUTE_RD(port_id);
-	SETUP_RPORT_ATTRIBUTE_RD(roles);
-	i->rport_attrs[count] = NULL;
+	i->rport_attrs[count++] = &dev_attr_port_id;
+	i->rport_attrs[count++] = &dev_attr_roles;
+	i->rport_attrs[count++] = NULL;
+	BUG_ON(count > ARRAY_SIZE(i->rport_attrs));
 
 	transport_container_register(&i->rport_attr_cont);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 13/19] srp_transport: Document sysfs attributes

[Bart Van Assche]

Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>
Cc: David Dillow <dillowda@ornl.gov>
Cc: Roland Dreier <roland@purestorage.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 Documentation/ABI/stable/sysfs-transport-srp |   12 ++++++++++++
 1 file changed, 12 insertions(+)
 create mode 100644 Documentation/ABI/stable/sysfs-transport-srp

diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
new file mode 100644
index 0000000..7b0d4a5
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-transport-srp
@@ -0,0 +1,12 @@
+What:		/sys/class/srp_remote_ports/port-<h>:<n>/port_id
+Date:		June 27, 2007
+KernelVersion:	2.6.24
+Contact:	linux-scsi@vger.kernel.org
+Description:	16-byte local SRP port identifier in hexadecimal format. An
+		example: 4c:49:4e:55:58:20:56:49:4f:00:00:00:00:00:00:00.
+
+What:		/sys/class/srp_remote_ports/port-<h>:<n>/roles
+Date:		June 27, 2007
+KernelVersion:	2.6.24
+Contact:	linux-scsi@vger.kernel.org
+Description:	Role of the remote port. Either "SRP Initiator" or "SRP Target".
-- 
1.7.10.4

[PATCH 14/19] ib_srp: Allow SRP disconnect through sysfs

[Bart Van Assche]

Make it possible to disconnect the IB RC connection used by the
SRP protocol to communicate with a target.

Let the SRP transport layer create a sysfs "delete" attribute for
initiator drivers that support this functionality.

Cc: David Dillow <dillowda@ornl.gov>
Cc: Roland Dreier <roland@purestorage.com>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 Documentation/ABI/stable/sysfs-transport-srp |    7 +++++++
 drivers/infiniband/ulp/srp/ib_srp.c          |   10 ++++++++++
 drivers/scsi/scsi_transport_srp.c            |   22 +++++++++++++++++++++-
 include/scsi/scsi_transport_srp.h            |    8 ++++++++
 4 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
index 7b0d4a5..b36fb0d 100644
--- a/Documentation/ABI/stable/sysfs-transport-srp
+++ b/Documentation/ABI/stable/sysfs-transport-srp
@@ -1,3 +1,10 @@
+What:		/sys/class/srp_remote_ports/port-<h>:<n>/delete
+Date:		June 1, 2012
+KernelVersion:	3.7
+Contact:	linux-scsi@vger.kernel.org, linux-rdma@vger.kernel.org
+Description:	Instructs an SRP initiator to disconnect from a target and to
+		remove all LUNs imported from that target.
+
 What:		/sys/class/srp_remote_ports/port-<h>:<n>/port_id
 Date:		June 27, 2007
 KernelVersion:	2.6.24
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 58d19a2..96e9bc2 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -609,6 +609,13 @@ static void srp_remove_work(struct work_struct *work)
 	srp_remove_target(target);
 }
 
+static void srp_rport_delete(struct srp_rport *rport)
+{
+	struct srp_target_port *target = rport->lld_data;
+
+	srp_queue_remove_work(target);
+}
+
 static int srp_connect_target(struct srp_target_port *target)
 {
 	int retries = 3;
@@ -2034,6 +2041,8 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 		return PTR_ERR(rport);
 	}
 
+	rport->lld_data = target;
+
 	spin_lock(&host->target_lock);
 	list_add_tail(&target->list, &host->target_list);
 	spin_unlock(&host->target_lock);
@@ -2601,6 +2610,7 @@ static void srp_remove_one(struct ib_device *device)
 }
 
 static struct srp_function_template ib_srp_transport_functions = {
+	.rport_delete		 = srp_rport_delete,
 };
 
 static int __init srp_init_module(void)
diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 0d85f79..f379c7f 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -38,7 +38,7 @@ struct srp_host_attrs {
 #define to_srp_host_attrs(host)	((struct srp_host_attrs *)(host)->shost_data)
 
 #define SRP_HOST_ATTRS 0
-#define SRP_RPORT_ATTRS 2
+#define SRP_RPORT_ATTRS 3
 
 struct srp_internal {
 	struct scsi_transport_template t;
@@ -116,6 +116,24 @@ show_srp_rport_roles(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR(roles, S_IRUGO, show_srp_rport_roles, NULL);
 
+static ssize_t store_srp_rport_delete(struct device *dev,
+				      struct device_attribute *attr,
+				      const char *buf, size_t count)
+{
+	struct srp_rport *rport = transport_class_to_srp_rport(dev);
+	struct Scsi_Host *shost = dev_to_shost(dev);
+	struct srp_internal *i = to_srp_internal(shost->transportt);
+
+	if (i->f->rport_delete) {
+		i->f->rport_delete(rport);
+		return count;
+	} else {
+		return -ENOSYS;
+	}
+}
+
+static DEVICE_ATTR(delete, S_IWUSR, NULL, store_srp_rport_delete);
+
 static void srp_rport_release(struct device *dev)
 {
 	struct srp_rport *rport = dev_to_rport(dev);
@@ -309,6 +327,8 @@ srp_attach_transport(struct srp_function_template *ft)
 	count = 0;
 	i->rport_attrs[count++] = &dev_attr_port_id;
 	i->rport_attrs[count++] = &dev_attr_roles;
+	if (ft->rport_delete)
+		i->rport_attrs[count++] = &dev_attr_delete;
 	i->rport_attrs[count++] = NULL;
 	BUG_ON(count > ARRAY_SIZE(i->rport_attrs));
 
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index 9c60ca1..ff0f04a 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -14,13 +14,21 @@ struct srp_rport_identifiers {
 };
 
 struct srp_rport {
+	/* for initiator and target drivers */
+
 	struct device dev;
 
 	u8 port_id[16];
 	u8 roles;
+
+	/* for initiator drivers */
+
+	void *lld_data;	/* LLD private data */
 };
 
 struct srp_function_template {
+	/* for initiator drivers */
+	void (*rport_delete)(struct srp_rport *rport);
 	/* for target drivers */
 	int (* tsk_mgmt_response)(struct Scsi_Host *, u64, u64, int);
 	int (* it_nexus_response)(struct Scsi_Host *, u64, int);
-- 
1.7.10.4

[PATCH 15/19] ib_srp: Maintain a single connection per I_T nexus

[Bart Van Assche]

The sysfs attribute 'add_target' may be used to relogin to a
target. An SRP target that receives a second login request from
an initiator will disconnect the previous connection. So before
trying to reconnect, check whether another connection to the
same SRP target identifier already exists. If so, remove the
target port.  Add a target to the target list before connecting
instead of after such that this algorithm has a chance to work.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |  145 +++++++++++++++++++++++++++++------
 drivers/infiniband/ulp/srp/ib_srp.h |   12 +++
 2 files changed, 133 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 96e9bc2..b3a09e9 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -430,6 +430,24 @@ static int srp_send_req(struct srp_target_port *target)
 	return status;
 }
 
+static bool srp_change_state(struct srp_target_port *target,
+			     enum srp_target_state old,
+			     enum srp_target_state new)
+{
+	bool changed = false;
+
+	BUG_ON(old == SRP_TARGET_REMOVED);
+
+	spin_lock_irq(&target->lock);
+	if (target->state == old) {
+		target->state = new;
+		changed = true;
+	}
+	spin_unlock_irq(&target->lock);
+
+	return changed;
+}
+
 static bool srp_queue_remove_work(struct srp_target_port *target)
 {
 	bool changed = false;
@@ -584,15 +602,24 @@ static void srp_del_scsi_host_attr(struct Scsi_Host *shost)
 
 static void srp_remove_target(struct srp_target_port *target)
 {
+	struct Scsi_Host *shost = target->scsi_host;
+	bool scsi_host_added = false;
+
 	WARN_ON(target->state != SRP_TARGET_REMOVED);
 
-	srp_del_scsi_host_attr(target->scsi_host);
-	srp_remove_host(target->scsi_host);
-	scsi_remove_host(target->scsi_host);
+	mutex_lock(&target->mutex);
+	swap(target->scsi_host_added, scsi_host_added);
+	mutex_unlock(&target->mutex);
+
+	if (scsi_host_added) {
+		srp_del_scsi_host_attr(shost);
+		srp_remove_host(shost);
+		scsi_remove_host(shost);
+	}
 	srp_disconnect_target(target);
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
-	scsi_host_put(target->scsi_host);
+	scsi_host_put(shost);
 }
 
 static void srp_remove_work(struct work_struct *work)
@@ -616,6 +643,30 @@ static void srp_rport_delete(struct srp_rport *rport)
 	srp_queue_remove_work(target);
 }
 
+/**
+ * srp_conn_unique() - Check whether the connection to a target is unique.
+ */
+static bool srp_conn_unique(struct srp_host *host,
+			    struct srp_target_port *target)
+{
+	struct srp_target_port *t;
+	bool ret = true;
+
+	spin_lock(&host->target_lock);
+	list_for_each_entry(t, &host->target_list, list) {
+		if (t != target &&
+		    target->id_ext == t->id_ext &&
+		    target->ioc_guid == t->ioc_guid &&
+		    target->initiator_ext == t->initiator_ext) {
+			ret = false;
+			break;
+		}
+	}
+	spin_unlock(&host->target_lock);
+
+	return ret;
+}
+
 static int srp_connect_target(struct srp_target_port *target)
 {
 	int retries = 3;
@@ -763,10 +814,24 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	struct ib_qp_attr qp_attr;
 	int i, ret;
 
-	if (target->state != SRP_TARGET_LIVE)
+	if (!srp_conn_unique(target->srp_host, target) &&
+	    srp_queue_remove_work(target)) {
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "Deleting SCSI host because obsolete\n");
+		return -ENXIO;
+	}
+
+	if (target->state == SRP_TARGET_REMOVED) {
+		shost_printk(KERN_DEBUG, target->scsi_host,
+			     PFX "Removal already scheduled\n");
 		return -EAGAIN;
+	}
 
-	scsi_target_block(&shost->shost_gendev);
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		scsi_target_block(&shost->shost_gendev);
+	target->reconnecting = true;
+	mutex_unlock(&target->mutex);
 
 	srp_disconnect_target(target);
 	/*
@@ -796,18 +861,27 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	for (i = 0; i < SRP_SQ_SIZE; ++i)
 		list_add(&target->tx_ring[i]->list, &target->free_tx);
 
-	ret = srp_connect_target(target);
+	mutex_lock(&target->mutex);
+	ret = target->scsi_host_added ? srp_connect_target(target) : -ENODEV;
+	mutex_unlock(&target->mutex);
+
 	if (ret)
 		goto err;
 
+	mutex_lock(&target->mutex);
+	target->reconnecting = false;
 	scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
+	mutex_unlock(&target->mutex);
 
 	shost_printk(KERN_INFO, target->scsi_host, PFX "reconnect succeeded\n");
 
 	return ret;
 
 err:
+	mutex_lock(&target->mutex);
+	target->reconnecting = false;
 	scsi_target_unblock(&shost->shost_gendev, SDEV_TRANSPORT_OFFLINE);
+	mutex_unlock(&target->mutex);
 
 	shost_printk(KERN_ERR, target->scsi_host,
 		     PFX "reconnect failed (%d), removing target port.\n", ret);
@@ -1740,6 +1814,9 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "connection closed\n");
 
+		if (!target->reconnecting)
+			srp_queue_remove_work(target);
+
 		comp = 1;
 		target->status = 0;
 		break;
@@ -2043,16 +2120,6 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 
 	rport->lld_data = target;
 
-	spin_lock(&host->target_lock);
-	list_add_tail(&target->list, &host->target_list);
-	spin_unlock(&host->target_lock);
-
-	target->state = SRP_TARGET_LIVE;
-	target->connected = false;
-
-	scsi_scan_target(&target->scsi_host->shost_gendev,
-			 0, target->scsi_id, SCAN_WILD_CARD, 0);
-
 	return 0;
 }
 
@@ -2338,6 +2405,7 @@ static ssize_t srp_create_target(struct device *dev,
 			     sizeof (struct srp_indirect_buf) +
 			     target->cmd_sg_cnt * sizeof (struct srp_direct_buf);
 
+	mutex_init(&target->mutex);
 	INIT_WORK(&target->remove_work, srp_remove_work);
 	spin_lock_init(&target->lock);
 	INIT_LIST_HEAD(&target->free_tx);
@@ -2384,24 +2452,53 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_ib;
 
+	target->connected = false;
+	target->reconnecting = false;
+	target->rq_tmo_jiffies = 1 * HZ;
+	target->state = SRP_TARGET_CONNECTING;
+	target->scsi_host_added = false;
+
+	mutex_lock(&target->mutex);
+	spin_lock(&host->target_lock);
+	list_add_tail(&target->list, &host->target_list);
+	spin_unlock(&host->target_lock);
+
+	if (!srp_change_state(target, SRP_TARGET_CONNECTING, SRP_TARGET_LIVE)) {
+		ret = -ENOENT;
+		goto err_unlock_remove;
+	}
+
 	ret = srp_connect_target(target);
 	if (ret) {
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "Connection failed\n");
-		goto err_cm_id;
+		goto err_unlock_remove;
 	}
 
 	ret = srp_add_target(host, target);
+	target->scsi_host_added = ret == 0;
+
+	WARN_ON(!scsi_host_get(target_host));
+	mutex_unlock(&target->mutex);
+
+	if (ret == 0)
+		scsi_scan_target(&target->scsi_host->shost_gendev, 0,
+				 target->scsi_id, SCAN_WILD_CARD, 0);
+
+	scsi_host_put(target_host);
+
 	if (ret)
-		goto err_disconnect;
+		goto err_remove;
 
 	return count;
 
-err_disconnect:
-	srp_disconnect_target(target);
-
-err_cm_id:
-	ib_destroy_cm_id(target->cm_id);
+err_unlock_remove:
+	mutex_unlock(&target->mutex);
+err_remove:
+	if (srp_queue_remove_work(target))
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "deleting SCSI host (ret = %d).\n", ret);
+	return ret;
 
 err_free_ib:
 	srp_free_target_ib(target);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 1b11117..4e0b413 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -81,7 +81,16 @@ enum {
 	SRP_LAST_SEND		= 0,
 };
 
+/**
+ * enum srp_target_state - State of an SRP target.
+ * @SRP_TARGET_CONNECTING: IB connection being established and SCSI host being
+ *                      added.
+ * @SRP_TARGET_LIVE: IB RC connection has been established.
+ * @SRP_TARGET_REMOVED: IB RC connection is about to be closed and SCSI host
+ *                      removal is pending.
+ */
 enum srp_target_state {
+	SRP_TARGET_CONNECTING,
 	SRP_TARGET_LIVE,
 	SRP_TARGET_REMOVED,
 };
@@ -166,6 +175,8 @@ struct srp_target_port {
 
 	u32			rq_tmo_jiffies;
 	bool			connected;
+	bool			reconnecting;
+	bool			scsi_host_added;
 
 	struct ib_cm_id	       *cm_id;
 
@@ -186,6 +197,7 @@ struct srp_target_port {
 	bool			last_recv_wqe;
 	bool			last_send_wqe;
 	wait_queue_head_t	qp_wq;
+	struct mutex		mutex;
 
 	struct completion	tsk_mgmt_done;
 	u8			tsk_mgmt_status;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 16/19] srp_transport: Add transport layer error handling

[Bart Van Assche]

Add the necessary functions in the SRP transport module to allow
an SRP initiator driver to implement transport error handling.
This includes:
- Support for implementing fast_io_fail_tmo, the time that should
  elapse after having detected a transport layer problem and
  before failing I/O.
- Support for implementing dev_loss_tmo, the time that should
  elapse after having detected a transport layer problem and
  before removing a remote port.

Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Robert Jennings <rcj-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 Documentation/ABI/stable/sysfs-transport-srp |   17 +++
 drivers/scsi/scsi_transport_srp.c            |  192 +++++++++++++++++++++++++-
 include/scsi/scsi_transport_srp.h            |   10 +-
 3 files changed, 216 insertions(+), 3 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
index b36fb0d..2f14a5b 100644
--- a/Documentation/ABI/stable/sysfs-transport-srp
+++ b/Documentation/ABI/stable/sysfs-transport-srp
@@ -5,6 +5,23 @@ Contact:	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 Description:	Instructs an SRP initiator to disconnect from a target and to
 		remove all LUNs imported from that target.
 
+What:		/sys/class/srp_remote_ports/port-<h>:<n>/dev_loss_tmo
+Date:		January 1, 2012
+KernelVersion:	3.7
+Contact:	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Number of seconds the SCSI layer will wait after a transport
+		layer error has been observed before removing a target port.
+		Zero means immediate removal.
+
+What:		/sys/class/srp_remote_ports/port-<h>:<n>/fast_io_fail_tmo
+Date:		January 1, 2012
+KernelVersion:	3.7
+Contact:	linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Number of seconds the SCSI layer will wait after a transport
+		layer error has been observed before failing I/O. Zero means
+		immediate removal. A negative value will disable this
+		behavior.
+
 What:		/sys/class/srp_remote_ports/port-<h>:<n>/port_id
 Date:		June 27, 2007
 KernelVersion:	2.6.24
diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index f379c7f..8b452c6 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -30,6 +30,7 @@
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
 #include <scsi/scsi_transport_srp.h>
+#include "scsi_priv.h"
 #include "scsi_transport_srp_internal.h"
 
 struct srp_host_attrs {
@@ -38,7 +39,7 @@ struct srp_host_attrs {
 #define to_srp_host_attrs(host)	((struct srp_host_attrs *)(host)->shost_data)
 
 #define SRP_HOST_ATTRS 0
-#define SRP_RPORT_ATTRS 3
+#define SRP_RPORT_ATTRS 5
 
 struct srp_internal {
 	struct scsi_transport_template t;
@@ -54,6 +55,10 @@ struct srp_internal {
 
 #define	dev_to_rport(d)	container_of(d, struct srp_rport, dev)
 #define transport_class_to_srp_rport(dev) dev_to_rport((dev)->parent)
+static inline struct Scsi_Host *rport_to_shost(struct srp_rport *r)
+{
+	return dev_to_shost(r->dev.parent);
+}
 
 static int srp_host_setup(struct transport_container *tc, struct device *dev,
 			  struct device *cdev)
@@ -134,6 +139,175 @@ static ssize_t store_srp_rport_delete(struct device *dev,
 
 static DEVICE_ATTR(delete, S_IWUSR, NULL, store_srp_rport_delete);
 
+/**
+ * srp_tmo_valid() - Check timeout combination validity.
+ *
+ * If no fast I/O fail timeout has been configured then the device loss timeout
+ * must be below SCSI_DEVICE_BLOCK_MAX_TIMEOUT. If a fast I/O fail timeout has
+ * been configured then it must be below the device loss timeout.
+ */
+static int srp_tmo_valid(int fast_io_fail_tmo, unsigned dev_loss_tmo)
+{
+	return (fast_io_fail_tmo < 0 &&
+		dev_loss_tmo <= SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
+		|| (0 <= fast_io_fail_tmo &&
+		    fast_io_fail_tmo < dev_loss_tmo &&
+		    dev_loss_tmo < ULONG_MAX / HZ) ? 0 : -EINVAL;
+}
+
+static ssize_t show_srp_rport_fast_io_fail_tmo(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct srp_rport *rport = transport_class_to_srp_rport(dev);
+
+	if (rport->fast_io_fail_tmo >= 0)
+		return sprintf(buf, "%d\n", rport->fast_io_fail_tmo);
+	else
+		return sprintf(buf, "off\n");
+}
+
+static ssize_t store_srp_rport_fast_io_fail_tmo(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t count)
+{
+	struct srp_rport *rport = transport_class_to_srp_rport(dev);
+	char ch[16];
+	int res;
+	int fast_io_fail_tmo;
+
+	if (count >= 3 && memcmp(buf, "off", 3) == 0) {
+		fast_io_fail_tmo = -1;
+	} else {
+		sprintf(ch, "%.*s", min_t(int, sizeof(ch) - 1, count), buf);
+		res = kstrtoint(ch, 0, &fast_io_fail_tmo);
+		if (res)
+			goto out;
+	}
+	res = srp_tmo_valid(fast_io_fail_tmo, rport->dev_loss_tmo);
+	if (res)
+		goto out;
+	rport->fast_io_fail_tmo = fast_io_fail_tmo;
+	res = count;
+out:
+	return res;
+}
+
+static DEVICE_ATTR(fast_io_fail_tmo, S_IRUGO | S_IWUSR,
+		   show_srp_rport_fast_io_fail_tmo,
+		   store_srp_rport_fast_io_fail_tmo);
+
+static ssize_t show_srp_rport_dev_loss_tmo(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	struct srp_rport *rport = transport_class_to_srp_rport(dev);
+
+	return sprintf(buf, "%u\n", rport->dev_loss_tmo);
+}
+
+static ssize_t store_srp_rport_dev_loss_tmo(struct device *dev,
+					    struct device_attribute *attr,
+					    const char *buf, size_t count)
+{
+	struct srp_rport *rport = transport_class_to_srp_rport(dev);
+	char ch[16];
+	int res;
+	unsigned dev_loss_tmo;
+
+	sprintf(ch, "%.*s", min_t(int, sizeof(ch) - 1, count), buf);
+	res = kstrtouint(ch, 0, &dev_loss_tmo);
+	if (res)
+		goto out;
+	res = srp_tmo_valid(rport->fast_io_fail_tmo, dev_loss_tmo);
+	if (res)
+		goto out;
+	rport->dev_loss_tmo = dev_loss_tmo;
+	res = count;
+out:
+	return res;
+}
+
+static DEVICE_ATTR(dev_loss_tmo, S_IRUGO | S_IWUSR,
+		   show_srp_rport_dev_loss_tmo,
+		   store_srp_rport_dev_loss_tmo);
+
+/**
+ * rport_fast_io_fail_timedout() - Fast I/O failure timeout handler.
+ *
+ * Unblocks the SCSI host.
+ */
+static void rport_fast_io_fail_timedout(struct work_struct *work)
+{
+	struct srp_rport *rport =
+		container_of(to_delayed_work(work), struct srp_rport,
+			     fast_io_fail_work);
+	struct Scsi_Host *shost;
+	struct srp_internal *i;
+
+	pr_err("SRP transport: fast_io_fail_tmo (%ds) expired - unblocking %s.\n",
+	       rport->fast_io_fail_tmo, dev_name(&rport->dev));
+
+	shost = rport_to_shost(rport);
+	i = to_srp_internal(shost->transportt);
+	/* Involve the LLDD if possible to terminate all io on the rport. */
+	if (i->f->terminate_rport_io)
+		i->f->terminate_rport_io(rport);
+
+	scsi_target_unblock(rport->dev.parent, SDEV_TRANSPORT_OFFLINE);
+}
+
+/**
+ * rport_dev_loss_timedout() - Device loss timeout handler.
+ *
+ * Note: rport->ft->rport_delete must either unblock the SCSI host or schedule
+ * SCSI host removal.
+ */
+static void rport_dev_loss_timedout(struct work_struct *work)
+{
+	struct srp_rport *rport =
+		container_of(to_delayed_work(work), struct srp_rport,
+			     dev_loss_work);
+	struct Scsi_Host *shost;
+	struct srp_internal *i;
+
+	pr_err("SRP transport: dev_loss_tmo (%ds) expired - removing %s.\n",
+	       rport->dev_loss_tmo, dev_name(&rport->dev));
+
+	shost = rport_to_shost(rport);
+	i = to_srp_internal(shost->transportt);
+	BUG_ON(!i->f);
+	BUG_ON(!i->f->rport_delete);
+
+	i->f->rport_delete(rport);
+}
+
+/**
+ * srp_start_tl_fail_timers() - Start the transport layer failure timers.
+ * @rport: rport on which to start the transport layer failure timers.
+ * @elapsed: Time in jiffies that has already elapsed since the failure.
+ *
+ * Start the transport layer fast I/O failure and device loss timers. Do not
+ * modify a timer that was already started.
+ */
+void srp_start_tl_fail_timers(struct srp_rport *rport, int elapsed)
+{
+	if (rport->fast_io_fail_tmo >= 0)
+		queue_delayed_work(system_long_wq, &rport->fast_io_fail_work,
+				   max_t(long, 1UL * rport->fast_io_fail_tmo
+					 * HZ - elapsed, 0));
+	queue_delayed_work(system_long_wq, &rport->dev_loss_work,
+		max_t(long, 1UL * rport->dev_loss_tmo * HZ - elapsed, 0));
+}
+EXPORT_SYMBOL(srp_start_tl_fail_timers);
+
+void srp_stop_tl_fail_timers(struct srp_rport *rport)
+{
+	cancel_delayed_work_sync(&rport->fast_io_fail_work);
+	cancel_delayed_work_sync(&rport->dev_loss_work);
+}
+EXPORT_SYMBOL(srp_stop_tl_fail_timers);
+
 static void srp_rport_release(struct device *dev)
 {
 	struct srp_rport *rport = dev_to_rport(dev);
@@ -210,6 +384,12 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost,
 	memcpy(rport->port_id, ids->port_id, sizeof(rport->port_id));
 	rport->roles = ids->roles;
 
+	rport->fast_io_fail_tmo = -1;
+	rport->dev_loss_tmo = 60;
+	INIT_DELAYED_WORK(&rport->fast_io_fail_work,
+			  rport_fast_io_fail_timedout);
+	INIT_DELAYED_WORK(&rport->dev_loss_work, rport_dev_loss_timedout);
+
 	id = atomic_inc_return(&to_srp_host_attrs(shost)->next_port_id);
 	dev_set_name(&rport->dev, "port-%d:%d", shost->host_no, id);
 
@@ -252,6 +432,11 @@ void srp_rport_del(struct srp_rport *rport)
 	struct device *dev = &rport->dev;
 	struct Scsi_Host *shost = dev_to_shost(dev->parent);
 
+	device_remove_file(dev, &dev_attr_fast_io_fail_tmo);
+	device_remove_file(dev, &dev_attr_dev_loss_tmo);
+	srp_stop_tl_fail_timers(rport);
+	scsi_target_unblock(rport->dev.parent, SDEV_RUNNING);
+
 	if (shost->active_mode & MODE_TARGET &&
 	    rport->roles == SRP_RPORT_ROLE_INITIATOR)
 		srp_tgt_it_nexus_destroy(shost, (unsigned long)rport);
@@ -327,8 +512,11 @@ srp_attach_transport(struct srp_function_template *ft)
 	count = 0;
 	i->rport_attrs[count++] = &dev_attr_port_id;
 	i->rport_attrs[count++] = &dev_attr_roles;
-	if (ft->rport_delete)
+	if (ft->rport_delete) {
+		i->rport_attrs[count++] = &dev_attr_dev_loss_tmo;
+		i->rport_attrs[count++] = &dev_attr_fast_io_fail_tmo;
 		i->rport_attrs[count++] = &dev_attr_delete;
+	}
 	i->rport_attrs[count++] = NULL;
 	BUG_ON(count > ARRAY_SIZE(i->rport_attrs));
 
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index ff0f04a..eb996db 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -23,11 +23,17 @@ struct srp_rport {
 
 	/* for initiator drivers */
 
-	void *lld_data;	/* LLD private data */
+	void			*lld_data;	/* LLD private data */
+
+	int			fast_io_fail_tmo;
+	unsigned		dev_loss_tmo;
+	struct delayed_work	fast_io_fail_work;
+	struct delayed_work	dev_loss_work;
 };
 
 struct srp_function_template {
 	/* for initiator drivers */
+	void (*terminate_rport_io)(struct srp_rport *rport);
 	void (*rport_delete)(struct srp_rport *rport);
 	/* for target drivers */
 	int (* tsk_mgmt_response)(struct Scsi_Host *, u64, u64, int);
@@ -41,6 +47,8 @@ extern void srp_release_transport(struct scsi_transport_template *);
 extern struct srp_rport *srp_rport_add(struct Scsi_Host *,
 				       struct srp_rport_identifiers *);
 extern void srp_rport_del(struct srp_rport *);
+extern void srp_start_tl_fail_timers(struct srp_rport *rport, int elapsed);
+extern void srp_stop_tl_fail_timers(struct srp_rport *rport);
 
 extern void srp_remove_host(struct Scsi_Host *);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 17/19] ib_srp: Add dev_loss_tmo support

[Bart Van Assche]

Remove an SRP host if dev_loss_tmo expired.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   30 +++++++++++++++++++++++++++++-
 drivers/infiniband/ulp/srp/ib_srp.h |    2 ++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b3a09e9..7ed0c26 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -617,6 +617,7 @@ static void srp_remove_target(struct srp_target_port *target)
 		scsi_remove_host(shost);
 	}
 	srp_disconnect_target(target);
+	cancel_work_sync(&target->tl_err_work);
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
 	scsi_host_put(shost);
@@ -834,6 +835,13 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	mutex_unlock(&target->mutex);
 
 	srp_disconnect_target(target);
+	cancel_work_sync(&target->tl_err_work);
+
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		srp_stop_tl_fail_timers(target->rport);
+	mutex_unlock(&target->mutex);
+
 	/*
 	 * Now get a new local CM ID so that we avoid confusing the
 	 * target in case things are really fouled up.
@@ -1439,15 +1447,32 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
 			     PFX "Recv failed with error code %d\n", res);
 }
 
+/**
+ * srp_tl_err_work - Start the transport layer failure timers.
+ */
+static void srp_tl_err_work(struct work_struct *work)
+{
+	struct srp_target_port *target;
+
+	target = container_of(work, struct srp_target_port, tl_err_work);
+
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		srp_start_tl_fail_timers(target->rport, target->rq_tmo_jiffies);
+	mutex_unlock(&target->mutex);
+}
+
 static void srp_handle_qp_err(enum ib_wc_status wc_status,
 			      enum ib_wc_opcode wc_opcode,
 			      struct srp_target_port *target)
 {
-	if (target->connected && !target->qp_in_error)
+	if (target->connected && !target->qp_in_error) {
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "failed %s status %d\n",
 			     wc_opcode & IB_WC_RECV ? "receive" : "send",
 			     wc_status);
+		queue_work(system_long_wq, &target->tl_err_work);
+	}
 	target->qp_in_error = true;
 }
 
@@ -1808,6 +1833,7 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 		if (ib_send_cm_drep(cm_id, NULL, 0))
 			shost_printk(KERN_ERR, target->scsi_host,
 				     PFX "Sending CM DREP failed\n");
+		queue_work(system_long_wq, &target->tl_err_work);
 		break;
 
 	case IB_CM_TIMEWAIT_EXIT:
@@ -2119,6 +2145,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 	}
 
 	rport->lld_data = target;
+	target->rport = rport;
 
 	return 0;
 }
@@ -2406,6 +2433,7 @@ static ssize_t srp_create_target(struct device *dev,
 			     target->cmd_sg_cnt * sizeof (struct srp_direct_buf);
 
 	mutex_init(&target->mutex);
+	INIT_WORK(&target->tl_err_work, srp_tl_err_work);
 	INIT_WORK(&target->remove_work, srp_remove_work);
 	spin_lock_init(&target->lock);
 	INIT_LIST_HEAD(&target->free_tx);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 4e0b413..99485a3 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -164,6 +164,7 @@ struct srp_target_port {
 	u16			io_class;
 	struct srp_host	       *srp_host;
 	struct Scsi_Host       *scsi_host;
+	struct srp_rport       *rport;
 	char			target_name[32];
 	unsigned int		scsi_id;
 	unsigned int		sg_tablesize;
@@ -188,6 +189,7 @@ struct srp_target_port {
 	struct srp_iu	       *rx_ring[SRP_RQ_SIZE];
 	struct srp_request	req_ring[SRP_CMD_SQ_SIZE];
 
+	struct work_struct	tl_err_work;
 	struct work_struct	remove_work;
 
 	struct list_head	list;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Bart Van Assche]

This patch is a modified version of a patch from Karandeep Chahal
that was posted on May 29, 2012 on the linux-rdma mailing list
(http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11796.html).

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Cc: Karandeep Chahal <kchahal-LfVdkaOWEx8@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   68 +++++++++++++++++++++++++++--------
 drivers/infiniband/ulp/srp/ib_srp.h |    1 +
 2 files changed, 55 insertions(+), 14 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 7ed0c26..91f86ea 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -451,13 +451,14 @@ static bool srp_change_state(struct srp_target_port *target,
 static bool srp_queue_remove_work(struct srp_target_port *target)
 {
 	bool changed = false;
+	unsigned long flags;
 
-	spin_lock_irq(&target->lock);
+	spin_lock_irqsave(&target->lock, flags);
 	if (target->state != SRP_TARGET_REMOVED) {
 		target->state = SRP_TARGET_REMOVED;
 		changed = true;
 	}
-	spin_unlock_irq(&target->lock);
+	spin_unlock_irqrestore(&target->lock, flags);
 
 	if (changed)
 		queue_work(system_long_wq, &target->remove_work);
@@ -630,9 +631,9 @@ static void srp_remove_work(struct work_struct *work)
 
 	WARN_ON(target->state != SRP_TARGET_REMOVED);
 
-	spin_lock(&target->srp_host->target_lock);
+	spin_lock_irq(&target->srp_host->target_lock);
 	list_del(&target->list);
-	spin_unlock(&target->srp_host->target_lock);
+	spin_unlock_irq(&target->srp_host->target_lock);
 
 	srp_remove_target(target);
 }
@@ -653,7 +654,7 @@ static bool srp_conn_unique(struct srp_host *host,
 	struct srp_target_port *t;
 	bool ret = true;
 
-	spin_lock(&host->target_lock);
+	spin_lock_irq(&host->target_lock);
 	list_for_each_entry(t, &host->target_list, list) {
 		if (t != target &&
 		    target->id_ext == t->id_ext &&
@@ -663,7 +664,7 @@ static bool srp_conn_unique(struct srp_host *host,
 			break;
 		}
 	}
-	spin_unlock(&host->target_lock);
+	spin_unlock_irq(&host->target_lock);
 
 	return ret;
 }
@@ -2487,9 +2488,9 @@ static ssize_t srp_create_target(struct device *dev,
 	target->scsi_host_added = false;
 
 	mutex_lock(&target->mutex);
-	spin_lock(&host->target_lock);
+	spin_lock_irq(&host->target_lock);
 	list_add_tail(&target->list, &host->target_list);
-	spin_unlock(&host->target_lock);
+	spin_unlock_irq(&host->target_lock);
 
 	if (!srp_change_state(target, SRP_TARGET_CONNECTING, SRP_TARGET_LIVE)) {
 		ret = -ENOENT;
@@ -2600,13 +2601,48 @@ free_host:
 	return NULL;
 }
 
+static void srp_host_remove_all(struct srp_host *host)
+{
+	struct srp_target_port *target;
+	unsigned long flags;
+
+	spin_lock_irqsave(&host->target_lock, flags);
+	list_for_each_entry(target, &host->target_list, list)
+		srp_queue_remove_work(target);
+	spin_unlock_irqrestore(&host->target_lock, flags);
+}
+
+static void srp_event_handler(struct ib_event_handler *handler,
+			      struct ib_event *event)
+{
+	struct srp_device *srp_dev = container_of(handler, struct srp_device,
+						  event_handler);
+	u8 port;
+	struct srp_host *host;
+
+	switch (event->event) {
+	case IB_EVENT_DEVICE_FATAL:
+	case IB_EVENT_PORT_ERR:
+		port = event->element.port_num;
+		pr_info("an error occurred on port %s-%d\n", srp_dev->dev->name,
+			port);
+
+		list_for_each_entry(host, &srp_dev->dev_list, list)
+			if (host->port == port)
+				srp_host_remove_all(host);
+		break;
+	default:
+		break;
+	}
+}
+
 static void srp_add_one(struct ib_device *device)
 {
 	struct srp_device *srp_dev;
 	struct ib_device_attr *dev_attr;
 	struct ib_fmr_pool_param fmr_param;
 	struct srp_host *host;
-	int max_pages_per_fmr, fmr_page_shift, s, e, p;
+	int max_pages_per_fmr, fmr_page_shift, s, e, p, ret;
 
 	dev_attr = kmalloc(sizeof *dev_attr, GFP_KERNEL);
 	if (!dev_attr)
@@ -2680,6 +2716,12 @@ static void srp_add_one(struct ib_device *device)
 			list_add_tail(&host->list, &srp_dev->dev_list);
 	}
 
+	INIT_IB_EVENT_HANDLER(&srp_dev->event_handler, device,
+			      srp_event_handler);
+	ret = ib_register_event_handler(&srp_dev->event_handler);
+	if (ret)
+		pr_err("ib_register_event_handler() failed: %d", ret);
+
 	ib_set_client_data(device, &srp_client, srp_dev);
 
 	goto free_attr;
@@ -2698,10 +2740,11 @@ static void srp_remove_one(struct ib_device *device)
 {
 	struct srp_device *srp_dev;
 	struct srp_host *host, *tmp_host;
-	struct srp_target_port *target;
 
 	srp_dev = ib_get_client_data(device, &srp_client);
 
+	ib_unregister_event_handler(&srp_dev->event_handler);
+
 	list_for_each_entry_safe(host, tmp_host, &srp_dev->dev_list, list) {
 		device_unregister(&host->dev);
 		/*
@@ -2713,10 +2756,7 @@ static void srp_remove_one(struct ib_device *device)
 		/*
 		 * Remove all target ports.
 		 */
-		spin_lock(&host->target_lock);
-		list_for_each_entry(target, &host->target_list, list)
-			srp_queue_remove_work(target);
-		spin_unlock(&host->target_lock);
+		srp_host_remove_all(host);
 
 		/*
 		 * Wait for target port removal tasks.
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 99485a3..f7841c8 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -107,6 +107,7 @@ struct srp_device {
 	struct ib_pd	       *pd;
 	struct ib_mr	       *mr;
 	struct ib_fmr_pool     *fmr_pool;
+	struct ib_event_handler event_handler;
 	u64			fmr_page_mask;
 	int			fmr_page_size;
 	int			fmr_max_size;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 19/19] scsi_transport_srp: Fail I/O faster

[Bart Van Assche]

Start the transport layer failure timer if a SCSI timeout occurs.

Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c |   95 ++++++++++++++++++++++++++-----------
 include/scsi/scsi_transport_srp.h |    1 -
 2 files changed, 68 insertions(+), 28 deletions(-)

diff --git a/drivers/scsi/scsi_transport_srp.c b/drivers/scsi/scsi_transport_srp.c
index 8b452c6..a96647a 100644
--- a/drivers/scsi/scsi_transport_srp.c
+++ b/drivers/scsi/scsi_transport_srp.c
@@ -29,12 +29,15 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
+#include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_transport_srp.h>
 #include "scsi_priv.h"
 #include "scsi_transport_srp_internal.h"
 
 struct srp_host_attrs {
-	atomic_t next_port_id;
+	atomic_t	 next_port_id;
+	spinlock_t	 lock; /* protects rport */
+	struct srp_rport *rport;
 };
 #define to_srp_host_attrs(host)	((struct srp_host_attrs *)(host)->shost_data)
 
@@ -67,6 +70,9 @@ static int srp_host_setup(struct transport_container *tc, struct device *dev,
 	struct srp_host_attrs *srp_host = to_srp_host_attrs(shost);
 
 	atomic_set(&srp_host->next_port_id, 0);
+	spin_lock_init(&srp_host->lock);
+	srp_host->rport = NULL;
+
 	return 0;
 }
 
@@ -308,6 +314,45 @@ void srp_stop_tl_fail_timers(struct srp_rport *rport)
 }
 EXPORT_SYMBOL(srp_stop_tl_fail_timers);
 
+/**
+ * srp_timed_out - SRP transport intercept of the SCSI timeout EH
+ *
+ * If a timeout occurs while an rport is in the blocked state, ask the SCSI
+ * EH to continue waiting (BLK_EH_RESET_TIMER). Otherwise let the SCSI core
+ * handle the timeout (BLK_EH_NOT_HANDLED). Also, if the rport is still
+ * present, start the SRP transport layer failure timers taking the time that
+ * has already elapsed into account.
+ *
+ * Note: This function is called from soft-IRQ context and with the request
+ * queue lock held.
+ */
+static enum blk_eh_timer_return srp_timed_out(struct scsi_cmnd *scmd)
+{
+	struct scsi_device *sdev = scmd->device;
+	struct srp_host_attrs *srp_host = to_srp_host_attrs(sdev->host);
+	struct srp_rport *rport;
+	struct request *req = scmd->request;
+	unsigned long flags;
+	enum blk_eh_timer_return rtn;
+
+	spin_lock_irqsave(&srp_host->lock, flags);
+	rport = srp_host->rport;
+	pr_err("%s() called for shost %s / rport %s\n", __func__,
+	       dev_name(&sdev->host->shost_gendev),
+	       rport ? dev_name(&rport->dev) : "(deleted)");
+	if (!rport) {
+		rtn = BLK_EH_NOT_HANDLED;
+	} else if (scsi_device_blocked(sdev)) {
+		rtn = BLK_EH_RESET_TIMER;
+	} else {
+		srp_start_tl_fail_timers(rport, req->timeout);
+		rtn = BLK_EH_NOT_HANDLED;
+	}
+	spin_unlock_irqrestore(&srp_host->lock, flags);
+
+	return rtn;
+}
+
 static void srp_rport_release(struct device *dev)
 {
 	struct srp_rport *rport = dev_to_rport(dev);
@@ -370,6 +415,7 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost,
 {
 	struct srp_rport *rport;
 	struct device *parent = &shost->shost_gendev;
+	struct srp_host_attrs *srp_host = to_srp_host_attrs(shost);
 	int id, ret;
 
 	rport = kzalloc(sizeof(*rport), GFP_KERNEL);
@@ -395,6 +441,10 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost,
 
 	transport_setup_device(&rport->dev);
 
+	spin_lock_irq(&srp_host->lock);
+	srp_host->rport = rport;
+	spin_unlock_irq(&srp_host->lock);
+
 	ret = device_add(&rport->dev);
 	if (ret) {
 		transport_destroy_device(&rport->dev);
@@ -422,50 +472,39 @@ struct srp_rport *srp_rport_add(struct Scsi_Host *shost,
 EXPORT_SYMBOL_GPL(srp_rport_add);
 
 /**
- * srp_rport_del  -  remove a SRP remote port
- * @rport:	SRP remote port to remove
+ * srp_remove_host  -  tear down a Scsi_Host's SRP data structures
+ * @shost:	Scsi Host that is torn down
  *
- * Removes the specified SRP remote port.
+ * Removes all SRP remote ports for a given Scsi_Host.
+ * Must be called just before scsi_remove_host for SRP HBAs.
  */
-void srp_rport_del(struct srp_rport *rport)
+void srp_remove_host(struct Scsi_Host *shost)
 {
+	struct srp_host_attrs *srp_host = to_srp_host_attrs(shost);
+	struct srp_rport *rport = srp_host->rport;
 	struct device *dev = &rport->dev;
-	struct Scsi_Host *shost = dev_to_shost(dev->parent);
+
+	BUG_ON(!rport);
 
 	device_remove_file(dev, &dev_attr_fast_io_fail_tmo);
 	device_remove_file(dev, &dev_attr_dev_loss_tmo);
-	srp_stop_tl_fail_timers(rport);
 	scsi_target_unblock(rport->dev.parent, SDEV_RUNNING);
 
 	if (shost->active_mode & MODE_TARGET &&
 	    rport->roles == SRP_RPORT_ROLE_INITIATOR)
 		srp_tgt_it_nexus_destroy(shost, (unsigned long)rport);
 
+	spin_lock_irq(&srp_host->lock);
+	srp_host->rport = NULL;
+	spin_unlock_irq(&srp_host->lock);
+
+	srp_stop_tl_fail_timers(rport);
+
 	transport_remove_device(dev);
 	device_del(dev);
 	transport_destroy_device(dev);
 	put_device(dev);
 }
-EXPORT_SYMBOL_GPL(srp_rport_del);
-
-static int do_srp_rport_del(struct device *dev, void *data)
-{
-	if (scsi_is_srp_rport(dev))
-		srp_rport_del(dev_to_rport(dev));
-	return 0;
-}
-
-/**
- * srp_remove_host  -  tear down a Scsi_Host's SRP data structures
- * @shost:	Scsi Host that is torn down
- *
- * Removes all SRP remote ports for a given Scsi_Host.
- * Must be called just before scsi_remove_host for SRP HBAs.
- */
-void srp_remove_host(struct Scsi_Host *shost)
-{
-	device_for_each_child(&shost->shost_gendev, NULL, do_srp_rport_del);
-}
 EXPORT_SYMBOL_GPL(srp_remove_host);
 
 static int srp_tsk_mgmt_response(struct Scsi_Host *shost, u64 nexus, u64 tm_id,
@@ -495,6 +534,8 @@ srp_attach_transport(struct srp_function_template *ft)
 	if (!i)
 		return NULL;
 
+	i->t.eh_timed_out = srp_timed_out;
+
 	i->t.tsk_mgmt_response = srp_tsk_mgmt_response;
 	i->t.it_nexus_response = srp_it_nexus_response;
 
diff --git a/include/scsi/scsi_transport_srp.h b/include/scsi/scsi_transport_srp.h
index eb996db..c019e93 100644
--- a/include/scsi/scsi_transport_srp.h
+++ b/include/scsi/scsi_transport_srp.h
@@ -46,7 +46,6 @@ extern void srp_release_transport(struct scsi_transport_template *);
 
 extern struct srp_rport *srp_rport_add(struct Scsi_Host *,
 				       struct srp_rport_identifiers *);
-extern void srp_rport_del(struct srp_rport *);
 extern void srp_start_tl_fail_timers(struct srp_rport *rport, int elapsed);
 extern void srp_stop_tl_fail_timers(struct srp_rport *rport);
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Or Gerlitz]

Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
> - multipathd is notified faster about transport layer failures.
> - Transport layer failures reliably result in SCSI host removal.
> - Switchover can be triggered explicitly by deleting an initiator device.
> - Disconnecting from a target without unloading ib_srp is now possible.

Can you describe few use cases, HA tests which don't pass w.o these
patches and do pass or react better with them?

> Changes since v2:
[...]
> - Dropped the patches for integration with multipathd.
can you explain this please? are  these non SRP patches which we
submitted/accepted
through another maintainer? can you point on the upstream commits?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Or Gerlitz]

Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> This patch is a modified version of a patch from Karandeep Chahal
> that was posted on May 29, 2012 on the linux-rdma mailing list
> (http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11796.html)

If you want your patch to land upstream, I would expect here a
change-log that explains why we want to do that... e.g what happens if
someone mounts a file system on this block device, do we want to pull
the rug behind the FS legs, why? what's special in port down even from
any other loss of IB connectivity that we have to give it special
treatment? why its needed to react on IB events and not just on
connectivity loss?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Or Gerlitz]

Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
> - multipathd is notified faster about transport layer failures.
> - Transport layer failures reliably result in SCSI host removal.
> - Switchover can be triggered explicitly by deleting an initiator device.
> - Disconnecting from a target without unloading ib_srp is now possible.

Hi Bart,

Few more questions/comments,

> The individual patches are:
> 0001-ib_srp-Enlarge-block-layer-timeout.patch
> 0002-ib_srp-Eliminate-state-SRP_TARGET_CONNECTING.patch
> 0003-ib_srp-Introduce-srp_handle_qp_err.patch
> 0004-ib_srp-Suppress-superfluous-error-messages.patch
> 0005-ib_srp-Avoid-that-SCSI-error-handling-causes-trouble.patch
> 0006-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch
> 0007-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch
> 0008-ib_srp-Keep-processing-commands-during-host-removal.patch
> 0009-ib_srp-Make-srp_disconnect_target-wait-for-IB-comple.patch
> 0010-ib_srp-Document-sysfs-attributes.patch
> 0011-srp_transport-Fix-atttribute-registration.patch
> 0012-srp_transport-Simplify-attribute-initialization-code.patch
> 0013-srp_transport-Document-sysfs-attributes.patch
> 0014-ib_srp-Allow-SRP-disconnect-through-sysfs.patch
> 0015-ib_srp-Maintain-a-single-connection-per-I_T-nexus.patch
> 0016-srp_transport-Add-transport-layer-error-handling.patch
> 0017-ib_srp-Add-dev_loss_tmo-support.patch
> 0018-ib_srp-Remove-SCSI-devices-upon-port-down-event.patch
> 0019-scsi_transport_srp-Fail-I-O-faster.patch

I understand that as a whole, this series makes SRP to function better
in HA schems, however, can you break this list of patches to cleanups,
new functionality and actual HA related bug fixes and concrete
improvements. Also, if you can spare some high level words on the
design directions/decisions taken as the patch series evolved, as this
effort in underway for some time now.

Also for V6 -- next-time -- etc, please make sure to have Vn to appear
in the [PATCH] brackets, as it critical when folks browse between
different versions/postings of your to the same patch set, if you use
git format-patch, this is easily achieved with --subject-prefix="PATCH
Vn"

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

On 11/12/12 23:51, Or Gerlitz wrote:
> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
>> - multipathd is notified faster about transport layer failures.
>> - Transport layer failures reliably result in SCSI host removal.
>> - Switchover can be triggered explicitly by deleting an initiator device.
>> - Disconnecting from a target without unloading ib_srp is now possible.
>
> Hi Bart,
>
> Few more questions/comments,
>
>> The individual patches are:
>> 0001-ib_srp-Enlarge-block-layer-timeout.patch
>> 0002-ib_srp-Eliminate-state-SRP_TARGET_CONNECTING.patch
>> 0003-ib_srp-Introduce-srp_handle_qp_err.patch
>> 0004-ib_srp-Suppress-superfluous-error-messages.patch
>> 0005-ib_srp-Avoid-that-SCSI-error-handling-causes-trouble.patch
>> 0006-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch
>> 0007-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch
>> 0008-ib_srp-Keep-processing-commands-during-host-removal.patch
>> 0009-ib_srp-Make-srp_disconnect_target-wait-for-IB-comple.patch
>> 0010-ib_srp-Document-sysfs-attributes.patch
>> 0011-srp_transport-Fix-atttribute-registration.patch
>> 0012-srp_transport-Simplify-attribute-initialization-code.patch
>> 0013-srp_transport-Document-sysfs-attributes.patch
>> 0014-ib_srp-Allow-SRP-disconnect-through-sysfs.patch
>> 0015-ib_srp-Maintain-a-single-connection-per-I_T-nexus.patch
>> 0016-srp_transport-Add-transport-layer-error-handling.patch
>> 0017-ib_srp-Add-dev_loss_tmo-support.patch
>> 0018-ib_srp-Remove-SCSI-devices-upon-port-down-event.patch
>> 0019-scsi_transport_srp-Fail-I-O-faster.patch
>
> I understand that as a whole, this series makes SRP to function better
> in HA schemes, however, can you break this list of patches to cleanups,
> new functionality and actual HA related bug fixes and concrete
> improvements. Also, if you can spare some high level words on the
> design directions/decisions taken as the patch series evolved, as this
> effort in underway for some time now.

The goal of this patch series is to reduce path failover time in a 
multipath setup. I've done my best to indicate in the patch descriptions 
which patches are cleanup patches and which patches introduce new 
functionality. Which patches should be considered bug fixes depends on 
what is considered a bug - is it e.g. considered a bug that it can take 
more than 60s with the upstream SRP initiator before multipathd switches 
paths ? Is it considered a bug that the upstream SRP initiator does not 
remove SCSI hosts that correspond to failed paths ?

> Also for V6 -- next-time -- etc, please make sure to have Vn to appear
> in the [PATCH] brackets, as it critical when folks browse between
> different versions/postings of your to the same patch set, if you use
> git format-patch, this is easily achieved with --subject-prefix="PATCH
> Vn"

Thanks, will do.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

On 11/12/12 23:36, Or Gerlitz wrote:
> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
>> - multipathd is notified faster about transport layer failures.
>> - Transport layer failures reliably result in SCSI host removal.
>> - Switchover can be triggered explicitly by deleting an initiator device.
>> - Disconnecting from a target without unloading ib_srp is now possible.
>
> Can you describe few use cases, HA tests which don't pass w.o these
> patches and do pass or react better with them?

This patch series reduces path failover time significantly. Instead of 
having to wait until the SCSI error handler has finished recovery, 
multipathd switches paths as soon as fast_fail_tmo has elapsed. Also, 
SCSI hosts that correspond to failed paths are removed. With the 
upstream SRP initiator and when triggering path failover repeatedly 
after some time hundreds of obsolete SCSI hosts are present.

>> Changes since v2:
> [...]
>> - Dropped the patches for integration with multipathd.
> can you explain this please? are  these non SRP patches which we
> submitted/accepted
> through another maintainer? can you point on the upstream commits?

With that comment I was referring to the dev_loss_tmo and 
fast_io_fail_tmo sysfs variables that had been dropped in v2 of this 
patch set but that have been reintroduced in v3 of this patch set. If 
these parameters have been set in /etc/multipath.conf then multipathd 
passes these on to the block driver (ib_srp in this case), at least the 
block driver provides the dev_loss_tmo and fast_io_fail_tmo sysfs 
attributes.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Bart Van Assche]

On 11/12/12 23:40, Or Gerlitz wrote:
> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> This patch is a modified version of a patch from Karandeep Chahal
>> that was posted on May 29, 2012 on the linux-rdma mailing list
>> (http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11796.html)
> 
> If you want your patch to land upstream, I would expect here a
> change-log that explains why we want to do that... e.g what happens if
> someone mounts a file system on this block device, do we want to pull
> the rug behind the FS legs, why? what's special in port down even from
> any other loss of IB connectivity that we have to give it special
> treatment? why its needed to react on IB events and not just on
> connectivity loss?

This patch is not an essential part of this patch series. All it does
is to trigger failover more quickly if a port down event has been
received. Without this patch, if an IB cable has been disconnected long
enough, a QP error will be generated anyway and that event will trigger
the path failure logic introduced in the earlier patches of this series.

Regarding file system behavior: if a file system should be shielded
from path failures in a multipath setup then it should be mounted on
top of a multipath device instead of using the SCSI host directly
created by ib_srp. In the file system tests I ran I have been using
the following multipathd options:

defaults {
    queue_without_daemon no
}
devices {
    device {
        ...
        features          "3 queue_if_no_path pg_init_retries 50"
        fast_io_fail_tmo  15
        dev_loss_tmo      60
    }
}

Are you perhaps worrying about what will happen in a setup with a single
path between initiator and target and where the IB connection disappears
and reappears quickly ? Shouldn't multipath be used even in such a setup
to avoid that the filesystem encounters an I/O error if the path disappears
for a longer time than what is tolerated by the SCSI error handler in order
to recover gracefully ?

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Or Gerlitz]

Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> On 11/12/12 23:40, Or Gerlitz wrote:
> This patch is not an essential part of this patch series. All it does
> is to trigger failover more quickly if a port down event has been
> received. Without this patch, if an IB cable has been disconnected long
> enough, a QP error will be generated anyway and that event will trigger
> the path failure logic introduced in the earlier patches of this series.

But if you have IB link which went down only to few milli-seconds or
even few hundred msecs, why disconnected the IB RC connection? IB is
layered and the RC transport is layer four, why we want to manually
break it if we have L2 event of port down?

Also, for the use case of multipath, an essentail part of the mpath
driver is to deal with (say) two devices when at some point at least
one of them becomes "failed" from the mpath point of view. So now this
patch comes and delets failed devices, but we've put mpath there so we
can deal with failed devices! also its very confusing for the mpath
users that would expect to be able to observe all the devices which
this mpath is set on and  their state, agree?





> Regarding file system behavior: if a file system should be shielded
> from path failures in a multipath setup then it should be mounted on
> top of a multipath device instead of using the SCSI host directly
> created by ib_srp. In the file system tests I ran I have been using
> the following multipathd options:
>
> defaults {
>     queue_without_daemon no
> }
> devices {
>     device {
>         ...
>         features          "3 queue_if_no_path pg_init_retries 50"
>         fast_io_fail_tmo  15
>         dev_loss_tmo      60
>     }
> }
>
> Are you perhaps worrying about what will happen in a setup with a single
> path between initiator and target and where the IB connection disappears
> and reappears quickly ? Shouldn't multipath be used even in such a setup
> to avoid that the filesystem encounters an I/O error if the path disappears
> for a longer time than what is tolerated by the SCSI error handler in order
> to recover gracefully ?

this gets way too much complicated, and just for patch which you said
"is not an essential part of this patch series" ... can we just drop
it altogether from the series?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Or Gerlitz]

Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> On 11/12/12 23:36, Or Gerlitz wrote:

> This patch series reduces path failover time significantly. Instead of
> having to wait until the SCSI error handler has finished recovery,

When a SCSI device is selected by mpath and used as a path, aren't failed
commands returned back to the mpath driver for possibly re-submission over
a different path?


> multipathd switches paths as soon as fast_fail_tmo has elapsed. Also, SCSI
> hosts that correspond to failed paths are removed. With the upstream SRP
> initiator and when triggering path failover repeatedly after some time
> hundreds of obsolete SCSI hosts are present.


>>> - Dropped the patches for integration with multipathd.

>> can you explain this please? are  these non SRP patches which we
>> submitted/accepted
>> through another maintainer? can you point on the upstream commits?

>
> With that comment I was referring to the dev_loss_tmo and fast_io_fail_tmo
> sysfs variables that had been dropped in v2 of this patch set but that have
> been reintroduced in v3 of this patch set. If these parameters have been set
> in /etc/multipath.conf then multipathd passes these on to the block driver
> (ib_srp in this case), at least the block driver provides the dev_loss_tmo
> and fast_io_fail_tmo sysfs attributes.

So these are attributes you added to the block layer, or to SRP? I'm
not clear on that

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Bart Van Assche]

On 11/13/12 21:54, Or Gerlitz wrote:
> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> [ ... ]
> this gets way too much complicated, and just for patch which you said
> "is not an essential part of this patch series" ... can we just drop
> it altogether from the series?

OK.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Or Gerlitz]

On Tue, Nov 13, 2012 at 11:20 PM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> On 11/13/12 21:54, Or Gerlitz wrote:
>>
>> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> [ ... ]
>>
>> this gets way too much complicated, and just for patch which you said
>> "is not an essential part of this patch series" ... can we just drop
>> it altogether from the series?
>
>
> OK.

that was a fast move (what's the .conf you're using...) -- can you
actually confirm that the results obtained using this patch series
would remain in the same level without this patch?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

On 11/13/12 22:04, Or Gerlitz wrote:
> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> On 11/12/12 23:36, Or Gerlitz wrote:
>
>> This patch series reduces path failover time significantly. Instead of
>> having to wait until the SCSI error handler has finished recovery,
>
> When a SCSI device is selected by mpath and used as a path, aren't failed
> commands returned back to the mpath driver for possibly re-submission over
> a different path?

The advantage of having a configurable fast_io_fail_tmo parameter is 
that this parameter can be configured to a smaller value than the SCSI 
timeout and hence that failover mechanisms in higher layers (dm and 
multipathd) are triggered more quickly if an I/O error is encountered.

>> multipathd switches paths as soon as fast_fail_tmo has elapsed. Also, SCSI
>> hosts that correspond to failed paths are removed. With the upstream SRP
>> initiator and when triggering path failover repeatedly after some time
>> hundreds of obsolete SCSI hosts are present.
>>
>>>> - Dropped the patches for integration with multipathd.
>>>
>>> can you explain this please? are  these non SRP patches which we
>>> submitted/accepted
>>> through another maintainer? can you point on the upstream commits?
>>
>>
>> With that comment I was referring to the dev_loss_tmo and fast_io_fail_tmo
>> sysfs variables that had been dropped in v2 of this patch set but that have
>> been reintroduced in v3 of this patch set. If these parameters have been set
>> in /etc/multipath.conf then multipathd passes these on to the block driver
>> (ib_srp in this case), at least the block driver provides the dev_loss_tmo
>> and fast_io_fail_tmo sysfs attributes.
>
> So these are attributes you added to the block layer, or to SRP? I'm
> not clear on that.

These attributes have been added to the SRP transport layer. Since the 
ib_srp driver registers itself with the SRP transport layer the SRP 
transport layer creates these two attributes for the ib_srp driver. This 
is similar to how the FC transport layer creates these attributes for FC 
initiator drivers.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/19] ib_srp: Remove SCSI devices upon port down event

[Bart Van Assche]

On 11/13/12 22:23, Or Gerlitz wrote:
> On Tue, Nov 13, 2012 at 11:20 PM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> On 11/13/12 21:54, Or Gerlitz wrote:
>>>
>>> Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>>> [ ... ]
>>>
>>> this gets way too much complicated, and just for patch which you said
>>> "is not an essential part of this patch series" ... can we just drop
>>> it altogether from the series?
>>
>> OK.
>
> that was a fast move (what's the .conf you're using...) -- can you
> actually confirm that the results obtained using this patch series
> would remain in the same level without this patch?

Failover time in case of cable pulling will increase slightly but 
otherwise the behavior of this patch series will be unaffected. I'll 
retest this patch series anyway before reposting it.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Or Gerlitz]

On Tue, Nov 13, 2012 at 11:30 PM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> These attributes have been added to the SRP transport layer. Since the
> ib_srp driver registers itself with the SRP transport layer the SRP
> transport layer creates these two attributes for the ib_srp driver. This is
> similar to how the FC transport layer creates these attributes for FC
> initiator drivers.

thanks for the heads up... can you provide some pointer please 2-3
relevant commits and hopefully documentation?  also I recall an
earlier version of this patch set where you attempted to add to to SRP
or even on some higher level some probing mechanism which mimics the
NOPs used by iSCSI, where/how was this ended? does the fact  that
iscsi has NOPs makes these parameters un-needed where in SRP/FC they
are?
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/19, v5] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

On 11/13/12 22:41, Or Gerlitz wrote:
> On Tue, Nov 13, 2012 at 11:30 PM, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
>> These attributes have been added to the SRP transport layer. Since the
>> ib_srp driver registers itself with the SRP transport layer the SRP
>> transport layer creates these two attributes for the ib_srp driver. This is
>> similar to how the FC transport layer creates these attributes for FC
>> initiator drivers.
>
> thanks for the heads up... can you provide some pointer please 2-3
> relevant commits and hopefully documentation?  also I recall an
> earlier version of this patch set where you attempted to add to to SRP
> or even on some higher level some probing mechanism which mimics the
> NOPs used by iSCSI, where/how was this ended? does the fact  that
> iscsi has NOPs makes these parameters un-needed where in SRP/FC they
> are?

Hello Or,

Via patch number 16 of this series not only the fast_io_fail_tmo and 
dev_loss_tmo attributes were added to the SRP transport layer but also 
documentation for these attributes was added to 
Documentation/ABI/stable/sysfs-transport-srp. And if you would like to 
have a closer look at the implementation of these attributes for the FC 
initiator transport layer, you can find it in 
drivers/scsi/scsi_transport_fc.c.

The code for sending a TUR periodically that was present in a previous 
version of this patch series has been dropped because it's not a proper 
transport layer test. As far as I know there is no equivalent for iSCSI 
NOPs in the SRP protocol. There might be an appropriate equivalent in 
the IB RC layer but I haven't started looking into that yet.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html