All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes
@ 2012-03-25 14:45 Bart Van Assche
  2012-03-25 14:47 ` [PATCH 01/15] ib_srp: Enlarge block layer timeout Bart Van Assche
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:45 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg,
	vuhuong-VPRAkNaXOzVWk0Htik3J/w

This patch series makes the ib_srp driver better suited for use in a H.A. setup because:
- Switchover can be triggered explicitly by deleting an initiator device.
- Disconnecting from a target without unloading ib_srp becomes possible.

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-Introduce-srp_handle_qp_err.patch
0003-ib_srp-Micro-optimize-srp_queuecommand.patch
0004-ib_srp-Suppress-superfluous-error-messages.patch
0005-ib_srp-Avoid-that-error-handling-triggers-a-crash.patch
0006-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch
0007-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch
0008-ib_srp-Fix-race-triggered-by-module-removal.patch
0009-srp_transport-Fix-atttribute-registration.patch
0010-srp_transport-Simplify-attribute-initialization-code.patch
0011-srp_transport-Document-sysfs-attributes.patch
0012-ib_srp-Document-sysfs-attributes.patch
0013-ib_srp-Allow-SRP-disconnect-through-sysfs.patch
0014-ib_srp-Introduce-temporary-variable-in-srp_remove_ta.patch
0015-ib_srp-Maintain-a-single-connection-per-I_T-nexus.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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/15] ib_srp: Enlarge block layer timeout
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
@ 2012-03-25 14:47 ` Bart Van Assche
  2012-03-29 16:59   ` Dave Dillow
  2012-03-25 14:48 ` [PATCH 02/15] ib_srp: Introduce srp_handle_qp_err() Bart Van Assche
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:47 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

Enlarge the block layer timeout for disks such that it is above
the InfiniBand transport layer timeout. This is necessary to avoid
that an SRP response is received after the SCSI layer has already
killed the associated SCSI command.

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bcbf22e..5543c2b 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1372,6 +1372,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..C9-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)
@@ -1431,6 +1458,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;
@@ -1689,6 +1718,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)
 {
@@ -1821,6 +1865,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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/15] ib_srp: Introduce srp_handle_qp_err()
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
  2012-03-25 14:47 ` [PATCH 01/15] ib_srp: Enlarge block layer timeout Bart Van Assche
@ 2012-03-25 14:48 ` Bart Van Assche
  2012-03-25 14:50 ` [PATCH 03/15] ib_srp: Micro-optimize srp_queuecommand() Bart Van Assche
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:48 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

Move the IB completion handler error handling code into the new
function srp_handle_qp_err(). Convert srp_target_port.qp_in_error
from int to bool. Move the initialization of that variable into
srp_connect_target(). Help the CPU branch predictor by making it
clear that IB_WC_SUCCESS is the most likely case.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@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 5543c2b..6279343 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;
@@ -648,7 +650,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;
@@ -1215,6 +1216,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;
@@ -1222,15 +1232,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);
 	}
 }
 
@@ -1241,16 +1248,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);
 	}
 }
 
@@ -2237,7 +2241,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 e3a6304..f0daeb3 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -180,7 +180,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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/15] ib_srp: Micro-optimize srp_queuecommand()
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
  2012-03-25 14:47 ` [PATCH 01/15] ib_srp: Enlarge block layer timeout Bart Van Assche
  2012-03-25 14:48 ` [PATCH 02/15] ib_srp: Introduce srp_handle_qp_err() Bart Van Assche
@ 2012-03-25 14:50 ` Bart Van Assche
  2012-03-25 14:52 ` [PATCH 04/15] ib_srp: Suppress superfluous error messages Bart Van Assche
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:50 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

Block the SCSI host while reconnecting instead of representing
the reconnect activity as a distinct SRP target state.

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 6279343..04fbc43 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -610,13 +610,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
@@ -654,8 +657,7 @@ 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);
 
 	return ret;
 
@@ -673,13 +675,15 @@ 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);
 	}
 	spin_unlock_irq(&target->lock);
 
+	scsi_target_unblock(&shost->shost_gendev);
+
 	return ret;
 }
 
@@ -1268,9 +1272,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;
@@ -1334,7 +1335,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 f0daeb3..02dc3ac 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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/15] ib_srp: Suppress superfluous error messages
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (2 preceding siblings ...)
  2012-03-25 14:50 ` [PATCH 03/15] ib_srp: Micro-optimize srp_queuecommand() Bart Van Assche
@ 2012-03-25 14:52 ` Bart Van Assche
  2012-03-25 14:53 ` [PATCH 05/15] ib_srp: Avoid that SCSI error handling triggers a crash Bart Van Assche
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:52 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

Keep track of the connection state. Don't send any data over the
QP after it has been disconnected. 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.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@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 04fbc43..c88dde3 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:
@@ -1224,8 +1244,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;
 }
 
@@ -1585,6 +1608,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");
@@ -1908,6 +1932,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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/15] ib_srp: Avoid that SCSI error handling triggers a crash
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (3 preceding siblings ...)
  2012-03-25 14:52 ` [PATCH 04/15] ib_srp: Suppress superfluous error messages Bart Van Assche
@ 2012-03-25 14:53 ` Bart Van Assche
  2012-03-25 14:54 ` [PATCH 06/15] ib_srp: Introduce the helper function srp_remove_target() Bart Van Assche
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:53 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

Sending any data over a queue pair associated with a closed
connection is wrong. The HCA will send the data anyway and such
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. That
can result in ib_srp complaining about "Null scmnd for RSP ..."
followed by a kernel oops.

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index c88dde3..2c265ae 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1296,7 +1296,8 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	int len;
 
 	if (target->state == SRP_TARGET_DEAD ||
-	    target->state == SRP_TARGET_REMOVED) {
+	    target->state == SRP_TARGET_REMOVED ||
+	    !target->connected) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
 		return 0;
@@ -1647,7 +1648,8 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,
 	struct srp_tsk_mgmt *tsk_mgmt;
 
 	if (target->state == SRP_TARGET_DEAD ||
-	    target->state == SRP_TARGET_REMOVED)
+	    target->state == SRP_TARGET_REMOVED ||
+	    !target->connected)
 		return -1;
 
 	init_completion(&target->tsk_mgmt_done);
-- 
1.7.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/15] ib_srp: Introduce the helper function srp_remove_target()
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (4 preceding siblings ...)
  2012-03-25 14:53 ` [PATCH 05/15] ib_srp: Avoid that SCSI error handling triggers a crash Bart Van Assche
@ 2012-03-25 14:54 ` Bart Van Assche
  2012-03-25 14:55 ` [PATCH 07/15] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:54 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 2c265ae..b4b825d 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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/15] ib_srp: Eliminate state SRP_TARGET_DEAD
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (5 preceding siblings ...)
  2012-03-25 14:54 ` [PATCH 06/15] ib_srp: Introduce the helper function srp_remove_target() Bart Van Assche
@ 2012-03-25 14:55 ` Bart Van Assche
  2012-03-25 14:56 ` [PATCH 08/15] ib_srp: Make srp_disconnect_target() wait for IB completions Bart Van Assche
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:55 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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 a call to srp_remove_target().
Rename srp_target_port.work into srp_target_port.remove_work to
reflect its usage. Move function srp_change_state() to just
before srp_change_state_to_removed() to avoid having to
introduce a forward declaration.

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b4b825d..9125a9c 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -428,6 +428,27 @@ 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;
+
+	spin_lock_irq(&target->lock);
+	if (target->state == old) {
+		target->state = new;
+		changed = true;
+	}
+	spin_unlock_irq(&target->lock);
+
+	return changed;
+}
+
+static bool srp_change_state_to_removed(struct srp_target_port *target)
+{
+	return srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_REMOVED);
+}
+
 static bool srp_change_conn_state(struct srp_target_port *target,
 				  bool connected)
 {
@@ -458,21 +479,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 +514,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 +529,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);
@@ -695,17 +703,9 @@ 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);
+	if (srp_change_state_to_removed(target))
+		queue_work(ib_wq, &target->remove_work);
 
 	scsi_target_unblock(&shost->shost_gendev);
 
@@ -1300,9 +1300,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 ||
-	    !target->connected) {
+	if (target->state == SRP_TARGET_REMOVED || !target->connected) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
 		return 0;
@@ -1652,9 +1650,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 ||
-	    !target->connected)
+	if (target->state == SRP_TARGET_REMOVED || !target->connected)
 		return -1;
 
 	init_completion(&target->tsk_mgmt_done);
@@ -2229,6 +2225,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);
@@ -2462,8 +2459,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);
 
@@ -2476,36 +2472,26 @@ 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. Wait for any removal tasks that
+		 * may already have been started.
 		 */
 		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);
+		while (!list_empty(&host->target_list)) {
+			target = list_first_entry(&host->target_list,
+						  struct srp_target_port, list);
+			if (srp_change_state_to_removed(target)) {
+				list_del(&target->list);
+				spin_unlock(&host->target_lock);
+				srp_remove_target(target);
+				spin_lock(&host->target_lock);
+			} else {
+				spin_unlock(&host->target_lock);
+				flush_work(&target->remove_work);
+				spin_lock(&host->target_lock);
+			}
 		}
 		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.
-		 */
-		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);
-		}
-
 		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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/15] ib_srp: Make srp_disconnect_target() wait for IB completions
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (6 preceding siblings ...)
  2012-03-25 14:55 ` [PATCH 07/15] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
@ 2012-03-25 14:56 ` Bart Van Assche
  2012-03-25 14:57 ` [PATCH 09/15] srp_transport: Fix atttribute registration Bart Van Assche
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:56 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 9125a9c..564ff08 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>
@@ -91,6 +91,7 @@ static void srp_remove_one(struct ib_device *device);
 static void srp_recv_completion(struct ib_cq *cq, void *target_ptr);
 static void srp_send_completion(struct ib_cq *cq, void *target_ptr);
 static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event);
+static void srp_reset_req(struct srp_target_port *target, struct srp_request *req);
 
 static struct scsi_transport_template *ib_srp_transport_template;
 
@@ -229,14 +230,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;
@@ -464,11 +467,67 @@ 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 = {
+	};
+	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 = {
+	};
+	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,
@@ -477,6 +536,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)
@@ -520,7 +593,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);
@@ -548,6 +620,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)
@@ -645,7 +719,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)
@@ -671,11 +744,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)
@@ -1249,7 +1317,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",
@@ -1267,8 +1335,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 == 0) {
+				target->last_recv_wqe = true;
+				wake_up(&target->qp_wq);
+			}
 			srp_handle_qp_err(wc.status, wc.opcode, target);
-			break;
 		}
 	}
 }
@@ -1284,8 +1355,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 == 0)
+				target->last_send_wqe = true;
 			srp_handle_qp_err(wc.status, wc.opcode, target);
-			break;
 		}
 	}
 }
@@ -2229,6 +2301,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..84b6a4c 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -180,6 +180,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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/15] srp_transport: Fix atttribute registration
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (7 preceding siblings ...)
  2012-03-25 14:56 ` [PATCH 08/15] ib_srp: Make srp_disconnect_target() wait for IB completions Bart Van Assche
@ 2012-03-25 14:57 ` Bart Van Assche
  2012-03-25 14:58 ` [PATCH 10/15] srp_transport: Simplify attribute initialization code Bart Van Assche
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:57 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg,
	fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
	brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8

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.

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Brian King <brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
Cc: <stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/scsi/scsi_transport_srp.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

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


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/15] srp_transport: Simplify attribute initialization code
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (8 preceding siblings ...)
  2012-03-25 14:57 ` [PATCH 09/15] srp_transport: Fix atttribute registration Bart Van Assche
@ 2012-03-25 14:58 ` Bart Van Assche
  2012-03-25 14:59 ` [PATCH 11/15] srp_transport: Document sysfs attributes Bart Van Assche
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:58 UTC (permalink / raw)
  To: linux-rdma, linux-scsi; +Cc: dillowda, roland, brking, fujita.tomonori

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.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Brian King <brking@linux.vnet.ibm.com>
Cc: David Dillow <dillowda@ornl.gov>
Cc: Roland Dreier <roland@purestorage.com>
---
 drivers/scsi/scsi_transport_srp.c |   26 ++++----------------------
 1 files 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.7



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 11/15] srp_transport: Document sysfs attributes
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (9 preceding siblings ...)
  2012-03-25 14:58 ` [PATCH 10/15] srp_transport: Simplify attribute initialization code Bart Van Assche
@ 2012-03-25 14:59 ` Bart Van Assche
  2012-03-25 15:00 ` [PATCH 12/15] ib_srp: " Bart Van Assche
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 14:59 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg,
	fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
	brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Brian King <brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 Documentation/ABI/stable/sysfs-transport-srp |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)
 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Role of the remote port. Either "SRP Initiator" or "SRP Target".
-- 
1.7.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 12/15] ib_srp: Document sysfs attributes
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (10 preceding siblings ...)
  2012-03-25 14:59 ` [PATCH 11/15] srp_transport: Document sysfs attributes Bart Van Assche
@ 2012-03-25 15:00 ` Bart Van Assche
  2012-03-25 15:01 ` [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 15:00 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 Documentation/ABI/stable/sysfs-driver-ib_srp |  156 ++++++++++++++++++++++++++
 1 files changed, 156 insertions(+), 0 deletions(-)
 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..cfc1ce2
--- /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 with 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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (11 preceding siblings ...)
  2012-03-25 15:00 ` [PATCH 12/15] ib_srp: " Bart Van Assche
@ 2012-03-25 15:01 ` Bart Van Assche
  2012-07-16 22:20   ` Mike Christie
  2012-03-25 15:02 ` [PATCH 14/15] ib_srp: Introduce a temporary variable in srp_remove_target() Bart Van Assche
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 15:01 UTC (permalink / raw)
  To: linux-rdma, linux-scsi; +Cc: dillowda, roland, fujita.tomonori, brking

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.

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

diff --git a/Documentation/ABI/stable/sysfs-transport-srp b/Documentation/ABI/stable/sysfs-transport-srp
index 7b0d4a5..9fbc703 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:		January 1, 2012
+KernelVersion:	3.4
+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 564ff08..9f940a5 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -612,6 +612,21 @@ static void srp_remove_work(struct work_struct *work)
 	srp_remove_target(target);
 }
 
+/*
+ * Note: it is up to the caller to ensure that invoking srp_rport_delete()
+ * does not trigger a race against target port removal in srp_remove_target().
+ * As an example, invoking srp_rport_delete() from the SCSI EH is not safe.
+ */
+static void srp_rport_delete(struct srp_rport *rport)
+{
+	struct srp_target_port *target = rport->lld_data;
+
+	BUG_ON(!target);
+
+	if (srp_change_state_to_removed(target))
+		queue_work(system_long_wq, &target->remove_work);
+}
+
 static int srp_connect_target(struct srp_target_port *target)
 {
 	int retries = 3;
@@ -1982,6 +1997,10 @@ static struct scsi_host_template srp_template = {
 	.shost_attrs			= srp_host_attrs
 };
 
+static struct srp_function_template ib_srp_transport_functions = {
+	.rport_delete		 = srp_rport_delete,
+};
+
 static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 {
 	struct srp_rport_identifiers ids;
@@ -2002,6 +2021,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);
@@ -2576,9 +2597,6 @@ static void srp_remove_one(struct ib_device *device)
 	kfree(srp_dev);
 }
 
-static struct srp_function_template ib_srp_transport_functions = {
-};
-
 static int __init srp_init_module(void)
 {
 	int ret;
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..f47549f 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.7



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 14/15] ib_srp: Introduce a temporary variable in srp_remove_target()
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (12 preceding siblings ...)
  2012-03-25 15:01 ` [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
@ 2012-03-25 15:02 ` Bart Van Assche
  2012-03-25 15:03 ` [PATCH 15/15] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
  2012-03-25 15:18 ` [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 15:02 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 9f940a5..63f57fd 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -587,15 +587,17 @@ 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;
+
 	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_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)
-- 
1.7.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 15/15] ib_srp: Maintain a single connection per I_T nexus
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (13 preceding siblings ...)
  2012-03-25 15:02 ` [PATCH 14/15] ib_srp: Introduce a temporary variable in srp_remove_target() Bart Van Assche
@ 2012-03-25 15:03 ` Bart Van Assche
  2012-03-25 15:18 ` [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
  15 siblings, 0 replies; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 15:03 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg

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.

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

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 63f57fd..eeb6d3f 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -449,7 +449,16 @@ static bool srp_change_state(struct srp_target_port *target,
 
 static bool srp_change_state_to_removed(struct srp_target_port *target)
 {
-	return srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_REMOVED);
+	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);
+
+	return changed;
 }
 
 static bool srp_change_conn_state(struct srp_target_port *target,
@@ -592,9 +601,15 @@ static void srp_remove_target(struct srp_target_port *target)
 	WARN_ON(target->state != SRP_TARGET_REMOVED);
 
 	srp_del_scsi_host_attr(shost);
-	srp_remove_host(shost);
-	scsi_remove_host(shost);
+
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added) {
+		srp_remove_host(shost);
+		scsi_remove_host(shost);
+	}
 	srp_disconnect_target(target);
+	mutex_unlock(&target->mutex);
+
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
 	scsi_host_put(shost);
@@ -629,6 +644,30 @@ static void srp_rport_delete(struct srp_rport *rport)
 		queue_work(system_long_wq, &target->remove_work);
 }
 
+/**
+ * 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;
@@ -738,8 +777,20 @@ 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_change_state_to_removed(target)) {
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "deleting SCSI host because obsolete.\n");
+		queue_work(system_long_wq, &target->remove_work);
+		return -ENXIO;
+	}
+	if (target->state == SRP_TARGET_REMOVED) {
+		shost_printk(KERN_DEBUG, target->scsi_host,
+			     PFX "Already removed\n");
 		return -EAGAIN;
+	}
+
+	mutex_lock(&target->mutex);
 
 	scsi_target_block(&shost->shost_gendev);
 
@@ -775,8 +826,11 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	if (ret)
 		goto err;
 
+unblock:
 	scsi_target_unblock(&shost->shost_gendev);
 
+	mutex_unlock(&target->mutex);
+
 	return ret;
 
 err:
@@ -792,9 +846,7 @@ err:
 	if (srp_change_state_to_removed(target))
 		queue_work(ib_wq, &target->remove_work);
 
-	scsi_target_unblock(&shost->shost_gendev);
-
-	return ret;
+	goto unblock;
 }
 
 static void srp_map_desc(struct srp_map_state *state, dma_addr_t dma_addr,
@@ -1711,6 +1763,10 @@ 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 (!srp_conn_unique(target->srp_host, target) &&
+		    srp_change_state_to_removed(target))
+			queue_work(system_long_wq, &target->remove_work);
+
 		comp = 1;
 		target->status = 0;
 		break;
@@ -2025,16 +2081,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;
 }
 
@@ -2320,6 +2366,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);
@@ -2366,24 +2413,53 @@ static ssize_t srp_create_target(struct device *dev,
 	if (ret)
 		goto err_free_ib;
 
+	target->connected = false;
+	target->rq_tmo_jiffies = 1 * HZ;
+	target->state = SRP_TARGET_CONNECTING;
+	target->scsi_host_added = false;
+
+	spin_lock(&host->target_lock);
+	list_add_tail(&target->list, &host->target_list);
+	spin_unlock(&host->target_lock);
+
+	mutex_lock(&target->mutex);
+	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);
+
+	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_change_state_to_removed(target)) {
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "deleting SCSI host (ret = %d).\n", ret);
+		srp_remove_work(&target->remove_work);
+	}
+	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 84b6a4c..0106183 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -78,7 +78,15 @@ enum {
 	SRP_MAP_NO_FMR		= 1,
 };
 
+/**
+ * enum srp_target_state - State of the SCSI host associated with an SRP target.
+ * @SRP_TARGET_CONNECTING: IB connection being established and SCSI host being
+ *                      added.
+ * @SRP_TARGET_LIVE: IB RC connection is established and SCSI host is unblocked.
+ * @SRP_TARGET_REMOVED: SCSI host removal is pending.
+ */
 enum srp_target_state {
+	SRP_TARGET_CONNECTING,
 	SRP_TARGET_LIVE,
 	SRP_TARGET_REMOVED,
 };
@@ -163,6 +171,7 @@ struct srp_target_port {
 
 	u32			rq_tmo_jiffies;
 	bool			connected;
+	bool			scsi_host_added;
 
 	struct ib_cm_id	       *cm_id;
 
@@ -183,6 +192,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.7


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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes
  2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
                   ` (14 preceding siblings ...)
  2012-03-25 15:03 ` [PATCH 15/15] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
@ 2012-03-25 15:18 ` Bart Van Assche
  2012-03-27 16:37   ` Dave Dillow
  15 siblings, 1 reply; 25+ messages in thread
From: Bart Van Assche @ 2012-03-25 15:18 UTC (permalink / raw)
  To: linux-rdma-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA
  Cc: dillowda-1Heg1YXhbW8, roland-BHEL68pLQRGGvPXPguhicg,
	vuhuong-VPRAkNaXOzVWk0Htik3J/w

On Sunday 25 March 2012 15:17, 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 [ ... ]

The patch series is also available here: http://github.com/bvanassche/linux/commits/srp-ha/.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes
  2012-03-25 15:18 ` [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
@ 2012-03-27 16:37   ` Dave Dillow
  0 siblings, 0 replies; 25+ messages in thread
From: Dave Dillow @ 2012-03-27 16:37 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg,
	vuhuong-VPRAkNaXOzVWk0Htik3J/w

On Sun, Mar 25, 2012 at 11:18:09AM -0400, Bart Van Assche wrote:
> On Sunday 25 March 2012 15:17, 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 [ ... ]
> 
> The patch series is also available here:
> http://github.com/bvanassche/linux/commits/srp-ha/.

Thanks Bart, seems the work mail server is still mangling whitespace, so
the repo link is much appreciated. I'll try to review these tomorrow
afternoon.

-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office
--
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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
  2012-03-25 14:47 ` [PATCH 01/15] ib_srp: Enlarge block layer timeout Bart Van Assche
@ 2012-03-29 16:59   ` Dave Dillow
       [not found]     ` <20120329165931.GA19700-1Heg1YXhbW8@public.gmane.org>
  0 siblings, 1 reply; 25+ messages in thread
From: Dave Dillow @ 2012-03-29 16:59 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, roland-BHEL68pLQRGGvPXPguhicg

On Sun, Mar 25, 2012 at 10:47:06AM -0400, Bart Van Assche wrote:
> Enlarge the block layer timeout for disks such that it is above
> the InfiniBand transport layer timeout. This is necessary to avoid
> that an SRP response is received after the SCSI layer has already
> killed the associated SCSI command.

> +
> +	/*
> +	 * 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));

I still don't like this useless splat when it could be written in a
more safe manner, but not enough to kick it back again.

Acked-by: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>

I haven't chewed on the rest yet, but would like to see this one at
least in 3.4 if possible.

-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office
--
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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
       [not found]     ` <20120329165931.GA19700-1Heg1YXhbW8@public.gmane.org>
@ 2012-04-22 16:01       ` Bart Van Assche
       [not found]         ` <4F942B74.4040106-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 25+ messages in thread
From: Bart Van Assche @ 2012-04-22 16:01 UTC (permalink / raw)
  To: Dave Dillow; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 03/29/12 16:59, Dave Dillow wrote:

> [ ... ]
> 
> I haven't chewed on the rest yet, but would like to see this one at
> least in 3.4 if possible.

Hi Dave,

If you have further comments about any of the patches in this series,
these are welcome. The 3.5 merge window isn't that far away anymore.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
       [not found]         ` <4F942B74.4040106-HInyCGIudOg@public.gmane.org>
@ 2012-04-30 15:11           ` David Dillow
       [not found]             ` <1335798691.441.2.camel-zHLflQxYYDO4Hhoo1DtQwJ9G+ZOsUmrO@public.gmane.org>
  0 siblings, 1 reply; 25+ messages in thread
From: David Dillow @ 2012-04-30 15:11 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Sun, 2012-04-22 at 16:01 +0000, Bart Van Assche wrote:
> On 03/29/12 16:59, Dave Dillow wrote:
> > I haven't chewed on the rest yet, but would like to see this one at
> > least in 3.4 if possible.

> If you have further comments about any of the patches in this series,
> these are welcome. The 3.5 merge window isn't that far away anymore.

I will attend to this before the end of the week, sooner if possible.

-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office


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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
       [not found]             ` <1335798691.441.2.camel-zHLflQxYYDO4Hhoo1DtQwJ9G+ZOsUmrO@public.gmane.org>
@ 2012-05-19 10:50               ` Bart Van Assche
       [not found]                 ` <4FB77AEE.7030100-HInyCGIudOg@public.gmane.org>
  0 siblings, 1 reply; 25+ messages in thread
From: Bart Van Assche @ 2012-05-19 10:50 UTC (permalink / raw)
  To: David Dillow; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On 04/30/12 15:11, David Dillow wrote:

> On Sun, 2012-04-22 at 16:01 +0000, Bart Van Assche wrote:
>> On 03/29/12 16:59, Dave Dillow wrote:
>>> I haven't chewed on the rest yet, but would like to see this one at
>>> least in 3.4 if possible.
> 
>> If you have further comments about any of the patches in this series,
>> these are welcome. The 3.5 merge window isn't that far away anymore.
> 
> I will attend to this before the end of the week, sooner if possible.


Hello Dave,

I hope you're still doing fine ?

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 01/15] ib_srp: Enlarge block layer timeout
       [not found]                 ` <4FB77AEE.7030100-HInyCGIudOg@public.gmane.org>
@ 2012-05-30  4:56                   ` David Dillow
  0 siblings, 0 replies; 25+ messages in thread
From: David Dillow @ 2012-05-30  4:56 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA

On Sat, 2012-05-19 at 10:50 +0000, Bart Van Assche wrote:
> On 04/30/12 15:11, David Dillow wrote:
> 
> > On Sun, 2012-04-22 at 16:01 +0000, Bart Van Assche wrote:
> >> On 03/29/12 16:59, Dave Dillow wrote:
> >>> I haven't chewed on the rest yet, but would like to see this one at
> >>> least in 3.4 if possible.
> > 
> >> If you have further comments about any of the patches in this series,
> >> these are welcome. The 3.5 merge window isn't that far away anymore.
> > 
> > I will attend to this before the end of the week, sooner if possible.
> 
> 
> Hello Dave,
> 
> I hope you're still doing fine ?

Yes, but I've been (mostly) under a rock for the past few weeks on
higher priority projects at work. Still, that's no excuse for keeping
you waiting; part of the idea was that I should have faster turn-around
than Roland...

This first patch was approved and should really go into the current
merge window if possible. I remain convinced that we should be able to
do the rest in a cleaner manner, without adding so many additional state
variables. I may be wrong, though.

My family is leaving Saturday for vacation, so I'll take some evening
hours this weekend/early next week and see if I can combine our ideas
into something I'm comfortable with.

Thank you for your exceptional patience,

-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs
  2012-03-25 15:01 ` [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
@ 2012-07-16 22:20   ` Mike Christie
       [not found]     ` <500493B4.4010303-hcNo3dDEHLuVc3sceRu5cw@public.gmane.org>
  0 siblings, 1 reply; 25+ messages in thread
From: Mike Christie @ 2012-07-16 22:20 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, dillowda-1Heg1YXhbW8,
	roland-BHEL68pLQRGGvPXPguhicg,
	fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
	brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8

On 03/25/2012 09:01 AM, Bart Van Assche wrote:
> +static void srp_rport_delete(struct srp_rport *rport)
> +{
> +	struct srp_target_port *target = rport->lld_data;
> +
> +	BUG_ON(!target);
> +

I don't think this null check is needed, because below you set the
lld_data before you call srp_rport_add which does the transport driver
model/sysfs file addition for the rport. So I think the rport delete
sysfs file won't show up before you set the lld_data.



>  static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
>  {
>  	struct srp_rport_identifiers ids;
> @@ -2002,6 +2021,8 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
>  		return PTR_ERR(rport);
>  	}
>  
> +	rport->lld_data = target;
> +

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs
       [not found]     ` <500493B4.4010303-hcNo3dDEHLuVc3sceRu5cw@public.gmane.org>
@ 2012-07-17 12:55       ` bart-LmZ/31Jx5xYUl6sPKNhdgg
  0 siblings, 0 replies; 25+ messages in thread
From: bart-LmZ/31Jx5xYUl6sPKNhdgg @ 2012-07-17 12:55 UTC (permalink / raw)
  To: Mike Christie
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, dillowda-1Heg1YXhbW8,
	roland-BHEL68pLQRGGvPXPguhicg,
	fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg,
	brking-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8

> On 03/25/2012 09:01 AM, Bart Van Assche wrote:
>> +static void srp_rport_delete(struct srp_rport *rport)
>> +{
>> +	struct srp_target_port *target = rport->lld_data;
>> +
>> +	BUG_ON(!target);
>> +
>
> I don't think this null check is needed, because below you set the
> lld_data before you call srp_rport_add which does the transport driver
> model/sysfs file addition for the rport. So I think the rport delete
> sysfs file won't show up before you set the lld_data.

Agreed. I'm fine with leaving that check out.

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2012-07-17 12:55 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-25 14:45 [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
2012-03-25 14:47 ` [PATCH 01/15] ib_srp: Enlarge block layer timeout Bart Van Assche
2012-03-29 16:59   ` Dave Dillow
     [not found]     ` <20120329165931.GA19700-1Heg1YXhbW8@public.gmane.org>
2012-04-22 16:01       ` Bart Van Assche
     [not found]         ` <4F942B74.4040106-HInyCGIudOg@public.gmane.org>
2012-04-30 15:11           ` David Dillow
     [not found]             ` <1335798691.441.2.camel-zHLflQxYYDO4Hhoo1DtQwJ9G+ZOsUmrO@public.gmane.org>
2012-05-19 10:50               ` Bart Van Assche
     [not found]                 ` <4FB77AEE.7030100-HInyCGIudOg@public.gmane.org>
2012-05-30  4:56                   ` David Dillow
2012-03-25 14:48 ` [PATCH 02/15] ib_srp: Introduce srp_handle_qp_err() Bart Van Assche
2012-03-25 14:50 ` [PATCH 03/15] ib_srp: Micro-optimize srp_queuecommand() Bart Van Assche
2012-03-25 14:52 ` [PATCH 04/15] ib_srp: Suppress superfluous error messages Bart Van Assche
2012-03-25 14:53 ` [PATCH 05/15] ib_srp: Avoid that SCSI error handling triggers a crash Bart Van Assche
2012-03-25 14:54 ` [PATCH 06/15] ib_srp: Introduce the helper function srp_remove_target() Bart Van Assche
2012-03-25 14:55 ` [PATCH 07/15] ib_srp: Eliminate state SRP_TARGET_DEAD Bart Van Assche
2012-03-25 14:56 ` [PATCH 08/15] ib_srp: Make srp_disconnect_target() wait for IB completions Bart Van Assche
2012-03-25 14:57 ` [PATCH 09/15] srp_transport: Fix atttribute registration Bart Van Assche
2012-03-25 14:58 ` [PATCH 10/15] srp_transport: Simplify attribute initialization code Bart Van Assche
2012-03-25 14:59 ` [PATCH 11/15] srp_transport: Document sysfs attributes Bart Van Assche
2012-03-25 15:00 ` [PATCH 12/15] ib_srp: " Bart Van Assche
2012-03-25 15:01 ` [PATCH 13/15] ib_srp: Allow SRP disconnect through sysfs Bart Van Assche
2012-07-16 22:20   ` Mike Christie
     [not found]     ` <500493B4.4010303-hcNo3dDEHLuVc3sceRu5cw@public.gmane.org>
2012-07-17 12:55       ` bart-LmZ/31Jx5xYUl6sPKNhdgg
2012-03-25 15:02 ` [PATCH 14/15] ib_srp: Introduce a temporary variable in srp_remove_target() Bart Van Assche
2012-03-25 15:03 ` [PATCH 15/15] ib_srp: Maintain a single connection per I_T nexus Bart Van Assche
2012-03-25 15:18 ` [PATCH 00/15, v3] Make ib_srp better suited for H.A. purposes Bart Van Assche
2012-03-27 16:37   ` Dave Dillow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.