[PATCH 00/20, v4] Make ib_srp better suited for H.A. purposes

[PATCH 00/20, v4] Make ib_srp better suited for H.A. purposes

[Bart Van Assche]

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

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

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

The individual patches are:
0001-ib_srp-Fix-a-race-condition.patch
0002-ib_srp-Enlarge-block-layer-timeout.patch
0003-ib_srp-Move-QP-state-check-into-srp_send_tsk_mgmt.patch
0004-ib_srp-Stop-queueing-if-QP-in-error.patch
0005-ib_srp-Eliminate-state-SRP_TARGET_CONNECTING.patch
0006-ib_srp-Suppress-superfluous-error-messages.patch
0007-ib_srp-Avoid-that-SCSI-error-handling-triggers-a-cra.patch
0008-ib_srp-Introduce-the-helper-function-srp_remove_targ.patch
0009-ib_srp-Eliminate-state-SRP_TARGET_DEAD.patch
0010-ib_srp-Keep-processing-commands-during-scsi_remove_h.patch
0011-ib_srp-Make-srp_disconnect_target-wait-for-IB-comple.patch
0012-ib_srp-Document-sysfs-attributes.patch
0013-srp_transport-Fix-atttribute-registration.patch
0014-srp_transport-Simplify-attribute-initialization-code.patch
0015-srp_transport-Document-sysfs-attributes.patch
0016-ib_srp-Allow-SRP-disconnect-through-sysfs.patch
0017-ib_srp-Introduce-a-temporary-variable-in-srp_remove_.patch
0018-ib_srp-Maintain-a-single-connection-per-I_T-nexus.patch
0019-srp_transport-Add-transport-layer-error-handling.patch
0020-ib_srp-Add-dev_loss_tmo-support.patch

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

[PATCH 14/20] srp_transport: Simplify attribute initialization code

[Bart Van Assche]

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

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Robert Jennings <rcj@linux.vnet.ibm.com>
Cc: David Dillow <dillowda@ornl.gov>
Cc: Roland Dreier <roland@purestorage.com>
---
 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

[PATCH 01/20] ib_srp: Fix a race condition

[Bart Van Assche]

Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp()
can trigger a crash by being invoked with scsi_done == NULL. That
could happen if a reply is received during or after a command abort.

BUG: unable to handle kernel NULL pointer dereference at (null)
Call Trace:
 <IRQ>
 [<ffffffffa04ab636>] ? srp_handle_recv+0x216/0x480 [ib_srp]
 [<ffffffffa04ab8ea>] srp_recv_completion+0x4a/0xb0 [ib_srp]
 [<ffffffffa032b497>] mlx4_ib_cq_comp+0x17/0x20 [mlx4_ib]
 [<ffffffffa02519f0>] mlx4_cq_completion+0x40/0x80 [mlx4_core]
 [<ffffffffa02529d3>] mlx4_eq_int+0x543/0x920 [mlx4_core]
 [<ffffffff81071acf>] ? local_clock+0x4f/0x60
 [<ffffffffa0252dc4>] mlx4_msi_x_interrupt+0x14/0x20 [mlx4_core]
 [<ffffffff810b82b5>] handle_irq_event_percpu+0x75/0x240
 [<ffffffff810b84ce>] handle_irq_event+0x4e/0x80
 [<ffffffff810badf5>] handle_edge_irq+0x85/0x130
 [<ffffffff81004355>] handle_irq+0x25/0x40
 [<ffffffff814153cd>] do_IRQ+0x5d/0xe0
 [<ffffffff8140b96c>] common_interrupt+0x6c/0x6c
 <EOI>
Kernel panic - not syncing: Fatal exception in interrupt

Reported-by: Joseph Glanville <joseph.glanville-2MxvZkOi9dvvnOemgxGiVw@public.gmane.org>
Reference: http://marc.info/?l=linux-rdma&m=134314367801595
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>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   82 ++++++++++++++++++++++++----------
 1 files changed, 58 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bcbf22e..9a61be2 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -586,24 +586,61 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
 			scmnd->sc_data_direction);
 }
 
-static void srp_remove_req(struct srp_target_port *target,
-			   struct srp_request *req, s32 req_lim_delta)
+/**
+ * srp_claim_req - Take ownership of the scmnd associated with a request.
+ * @target: SRP target port.
+ * @req: SRP request.
+ * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take
+ *         ownership of @req->scmnd if it equals @scmnd.
+ * @req_lim_delta: target->req_lim_delta increment.
+ *
+ * Return value:
+ * Either NULL or a pointer to the SCSI command the caller became owner of.
+ */
+static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
+				       struct srp_request *req,
+				       struct scsi_cmnd *scmnd,
+				       s32 req_lim_delta)
 {
 	unsigned long flags;
 
-	srp_unmap_data(req->scmnd, target, req);
 	spin_lock_irqsave(&target->lock, flags);
 	target->req_lim += req_lim_delta;
-	req->scmnd = NULL;
+	if (!scmnd)
+		swap(scmnd, req->scmnd);
+	else if (req->scmnd == scmnd)
+		req->scmnd = NULL;
+	else
+		scmnd = NULL;
+	spin_unlock_irqrestore(&target->lock, flags);
+
+	return scmnd;
+}
+
+/**
+ * srp_free_req() - Unmap data and add request to the free request list.
+ */
+static void srp_free_req(struct srp_target_port *target,
+			 struct srp_request *req, struct scsi_cmnd *scmnd)
+{
+	unsigned long flags;
+
+	srp_unmap_data(scmnd, target, req);
+
+	spin_lock_irqsave(&target->lock, flags);
 	list_add_tail(&req->list, &target->free_reqs);
 	spin_unlock_irqrestore(&target->lock, flags);
 }
 
 static void srp_reset_req(struct srp_target_port *target, struct srp_request *req)
 {
-	req->scmnd->result = DID_RESET << 16;
-	req->scmnd->scsi_done(req->scmnd);
-	srp_remove_req(target, req, 0);
+	struct scsi_cmnd *scmnd = req->scmnd;
+
+	if (srp_claim_req(target, req, scmnd, 0)) {
+		scmnd->result = DID_RESET << 16;
+		scmnd->scsi_done(scmnd);
+		srp_free_req(target, req, scmnd);
+	}
 }
 
 static int srp_reconnect_target(struct srp_target_port *target)
@@ -1073,11 +1110,14 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		complete(&target->tsk_mgmt_done);
 	} else {
 		req = &target->req_ring[rsp->tag];
-		scmnd = req->scmnd;
-		if (!scmnd)
+		scmnd = srp_claim_req(target, req, NULL,
+			       be32_to_cpu(rsp->req_lim_delta));
+		if (!scmnd) {
 			shost_printk(KERN_ERR, target->scsi_host,
 				     "Null scmnd for RSP w/tag %016llx\n",
 				     (unsigned long long) rsp->tag);
+			return;
+		}
 		scmnd->result = rsp->status;
 
 		if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {
@@ -1092,7 +1132,8 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		else if (rsp->flags & (SRP_RSP_FLAG_DIOVER | SRP_RSP_FLAG_DIUNDER))
 			scsi_set_resid(scmnd, be32_to_cpu(rsp->data_in_res_cnt));
 
-		srp_remove_req(target, req, be32_to_cpu(rsp->req_lim_delta));
+		srp_free_req(target, req, scmnd);
+
 		scmnd->host_scribble = NULL;
 		scmnd->scsi_done(scmnd);
 	}
@@ -1631,25 +1672,18 @@ static int srp_abort(struct scsi_cmnd *scmnd)
 {
 	struct srp_target_port *target = host_to_target(scmnd->device->host);
 	struct srp_request *req = (struct srp_request *) scmnd->host_scribble;
-	int ret = SUCCESS;
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
 
-	if (!req || target->qp_in_error)
+	if (!req || target->qp_in_error ||
+	    !srp_claim_req(target, req, scmnd, 0))
 		return FAILED;
-	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
-			      SRP_TSK_ABORT_TASK))
-		return FAILED;
-
-	if (req->scmnd) {
-		if (!target->tsk_mgmt_status) {
-			srp_remove_req(target, req, 0);
-			scmnd->result = DID_ABORT << 16;
-		} else
-			ret = FAILED;
-	}
+	srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
+			  SRP_TSK_ABORT_TASK);
+	srp_free_req(target, req, scmnd);
+	scmnd->result = DID_ABORT << 16;
 
-	return ret;
+	return SUCCESS;
 }
 
 static int srp_reset_device(struct scsi_cmnd *scmnd)
-- 
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

Re: [PATCH 01/20] ib_srp: Fix a race condition

[David Dillow]

On Thu, 2012-08-09 at 15:43 +0000, Bart Van Assche wrote:
> Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp()
> can trigger a crash by being invoked with scsi_done == NULL. That
> could happen if a reply is received during or after a command abort.

Please leave the call trace out of the commit message; it adds nothing.

> +static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
> +				       struct srp_request *req,
> +				       struct scsi_cmnd *scmnd,
> +				       s32 req_lim_delta)
>  {
>  	unsigned long flags;
>  
> -	srp_unmap_data(req->scmnd, target, req);
>  	spin_lock_irqsave(&target->lock, flags);
>  	target->req_lim += req_lim_delta;

I still think that adding credits back before putting the request on the
free list is a crash waiting to happen. We don't check to see if the
free request list is empty in srp_queuecommand(), so this creates a new
race window where we get a response that gives us credits back, but we
don't have a free request to back them.

Arguably, we need to check for that anyway to protect against buggy
targets giving us back more credits than we have used...

Or am I missing something obvious?
-- 
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

Re: [PATCH 01/20] ib_srp: Fix a race condition

[Bart Van Assche]

On 08/14/12 03:19, David Dillow wrote:
> On Thu, 2012-08-09 at 15:43 +0000, Bart Van Assche wrote:
>> +static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
>> +				       struct srp_request *req,
>> +				       struct scsi_cmnd *scmnd,
>> +				       s32 req_lim_delta)
>>  {
>>  	unsigned long flags;
>>  
>> -	srp_unmap_data(req->scmnd, target, req);
>>  	spin_lock_irqsave(&target->lock, flags);
>>  	target->req_lim += req_lim_delta;
> 
> I still think that adding credits back before putting the request on the
> free list is a crash waiting to happen. We don't check to see if the
> free request list is empty in srp_queuecommand(), so this creates a new
> race window where we get a response that gives us credits back, but we
> don't have a free request to back them.

I've just posted an updated patch in which the above comment has been
addressed, and I've also pushed out a rebased and retested tree to
http://github.com/bvanassche/linux/tree/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

[PATCH 01/20 v4b] ib_srp: Fix a race condition

[Bart Van Assche]

Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp()
can trigger a crash by being invoked with scsi_done == NULL. That
could happen if a reply is received during or after a command abort.

Reported-by: Joseph Glanville <joseph.glanville-2MxvZkOi9dvvnOemgxGiVw@public.gmane.org>
Reference: http://marc.info/?l=linux-rdma&m=134314367801595
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>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
---
 drivers/infiniband/ulp/srp/ib_srp.c |   87 +++++++++++++++++++++++++----------
 1 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index bcbf22e..1b5b0c7 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -586,24 +586,62 @@ static void srp_unmap_data(struct scsi_cmnd *scmnd,
 			scmnd->sc_data_direction);
 }
 
-static void srp_remove_req(struct srp_target_port *target,
-			   struct srp_request *req, s32 req_lim_delta)
+/**
+ * srp_claim_req - Take ownership of the scmnd associated with a request.
+ * @target: SRP target port.
+ * @req: SRP request.
+ * @scmnd: If NULL, take ownership of @req->scmnd. If not NULL, only take
+ *         ownership of @req->scmnd if it equals @scmnd.
+ *
+ * Return value:
+ * Either NULL or a pointer to the SCSI command the caller became owner of.
+ */
+static struct scsi_cmnd *srp_claim_req(struct srp_target_port *target,
+				       struct srp_request *req,
+				       struct scsi_cmnd *scmnd)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&target->lock, flags);
+	if (!scmnd) {
+		scmnd = req->scmnd;
+		req->scmnd = NULL;
+	} else if (req->scmnd == scmnd) {
+		req->scmnd = NULL;
+	} else {
+		scmnd = NULL;
+	}
+	spin_unlock_irqrestore(&target->lock, flags);
+
+	return scmnd;
+}
+
+/**
+ * srp_free_req() - Unmap data and add request to the free request list.
+ */
+static void srp_free_req(struct srp_target_port *target,
+			 struct srp_request *req, struct scsi_cmnd *scmnd,
+			 s32 req_lim_delta)
 {
 	unsigned long flags;
 
-	srp_unmap_data(req->scmnd, target, req);
+	srp_unmap_data(scmnd, target, req);
+
 	spin_lock_irqsave(&target->lock, flags);
 	target->req_lim += req_lim_delta;
-	req->scmnd = NULL;
 	list_add_tail(&req->list, &target->free_reqs);
 	spin_unlock_irqrestore(&target->lock, flags);
 }
 
 static void srp_reset_req(struct srp_target_port *target, struct srp_request *req)
 {
-	req->scmnd->result = DID_RESET << 16;
-	req->scmnd->scsi_done(req->scmnd);
-	srp_remove_req(target, req, 0);
+	struct scsi_cmnd *scmnd = srp_claim_req(target, req, NULL);
+
+	if (scmnd) {
+		scmnd->result = DID_RESET << 16;
+		scmnd->scsi_done(scmnd);
+		srp_free_req(target, req, scmnd, 0);
+	}
 }
 
 static int srp_reconnect_target(struct srp_target_port *target)
@@ -1073,11 +1111,18 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		complete(&target->tsk_mgmt_done);
 	} else {
 		req = &target->req_ring[rsp->tag];
-		scmnd = req->scmnd;
-		if (!scmnd)
+		scmnd = srp_claim_req(target, req, NULL);
+		if (!scmnd) {
 			shost_printk(KERN_ERR, target->scsi_host,
 				     "Null scmnd for RSP w/tag %016llx\n",
 				     (unsigned long long) rsp->tag);
+
+			spin_lock_irqsave(&target->lock, flags);
+			target->req_lim += be32_to_cpu(rsp->req_lim_delta);
+			spin_unlock_irqrestore(&target->lock, flags);
+
+			return;
+		}
 		scmnd->result = rsp->status;
 
 		if (rsp->flags & SRP_RSP_FLAG_SNSVALID) {
@@ -1092,7 +1137,9 @@ static void srp_process_rsp(struct srp_target_port *target, struct srp_rsp *rsp)
 		else if (rsp->flags & (SRP_RSP_FLAG_DIOVER | SRP_RSP_FLAG_DIUNDER))
 			scsi_set_resid(scmnd, be32_to_cpu(rsp->data_in_res_cnt));
 
-		srp_remove_req(target, req, be32_to_cpu(rsp->req_lim_delta));
+		srp_free_req(target, req, scmnd,
+			     be32_to_cpu(rsp->req_lim_delta));
+
 		scmnd->host_scribble = NULL;
 		scmnd->scsi_done(scmnd);
 	}
@@ -1631,25 +1678,17 @@ static int srp_abort(struct scsi_cmnd *scmnd)
 {
 	struct srp_target_port *target = host_to_target(scmnd->device->host);
 	struct srp_request *req = (struct srp_request *) scmnd->host_scribble;
-	int ret = SUCCESS;
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
 
-	if (!req || target->qp_in_error)
+	if (!req || target->qp_in_error || !srp_claim_req(target, req, scmnd))
 		return FAILED;
-	if (srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
-			      SRP_TSK_ABORT_TASK))
-		return FAILED;
-
-	if (req->scmnd) {
-		if (!target->tsk_mgmt_status) {
-			srp_remove_req(target, req, 0);
-			scmnd->result = DID_ABORT << 16;
-		} else
-			ret = FAILED;
-	}
+	srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
+			  SRP_TSK_ABORT_TASK);
+	srp_free_req(target, req, scmnd, 0);
+	scmnd->result = DID_ABORT << 16;
 
-	return ret;
+	return SUCCESS;
 }
 
 static int srp_reset_device(struct scsi_cmnd *scmnd)
-- 
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

Re: [PATCH 01/20 v4b] ib_srp: Fix a race condition

[David Dillow]

On Tue, 2012-08-14 at 13:18 +0000, Bart Van Assche wrote:
> Avoid that the scmnd->scsi_done(scmnd) call in srp_process_rsp()
> can trigger a crash by being invoked with scsi_done == NULL. That
> could happen if a reply is received during or after a command abort.

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

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

[PATCH 02/20] ib_srp: Enlarge block layer timeout

[Bart Van Assche]

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

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 9a61be2..8444d36 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1413,6 +1413,33 @@ err:
 	return -ENOMEM;
 }
 
+static uint32_t srp_compute_rq_tmo(struct ib_qp_attr *qp_attr, int attr_mask)
+{
+	uint64_t T_tr_ns, max_compl_time_ms;
+	uint32_t rq_tmo_jiffies;
+
+	/*
+	 * According to section 11.2.4.2 in the IBTA spec (Modify Queue Pair,
+	 * table 91), both the QP timeout and the retry count have to be set
+	 * for RC QP's during the RTR to RTS transition.
+	 */
+	WARN_ON((attr_mask & (IB_QP_TIMEOUT | IB_QP_RETRY_CNT)) !=
+		(IB_QP_TIMEOUT | IB_QP_RETRY_CNT));
+
+	/*
+	 * Set target->rq_tmo_jiffies to one second more than the largest time
+	 * it can take before an error completion is generated. See also
+	 * C9-140..142 in the IBTA spec for more information about how to
+	 * convert the QP Local ACK Timeout value to nanoseconds.
+	 */
+	T_tr_ns = 4096 * (1ULL << qp_attr->timeout);
+	max_compl_time_ms = qp_attr->retry_cnt * 4 * T_tr_ns;
+	do_div(max_compl_time_ms, NSEC_PER_MSEC);
+	rq_tmo_jiffies = msecs_to_jiffies(max_compl_time_ms + 1000);
+
+	return rq_tmo_jiffies;
+}
+
 static void srp_cm_rep_handler(struct ib_cm_id *cm_id,
 			       struct srp_login_rsp *lrsp,
 			       struct srp_target_port *target)
@@ -1472,6 +1499,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;
@@ -1723,6 +1752,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)
 {
@@ -1855,6 +1899,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

[PATCH 03/20] ib_srp: Move QP state check into srp_send_tsk_mgmt()

[Bart Van Assche]

Test the QP state inside srp_send_tsk_mgmt() instead of letting each
caller perform that test.

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 |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8444d36..f333705 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1660,7 +1660,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->qp_in_error)
 		return -1;
 
 	init_completion(&target->tsk_mgmt_done);
@@ -1704,8 +1705,7 @@ static int srp_abort(struct scsi_cmnd *scmnd)
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP abort called\n");
 
-	if (!req || target->qp_in_error ||
-	    !srp_claim_req(target, req, scmnd, 0))
+	if (!req || !srp_claim_req(target, req, scmnd, 0))
 		return FAILED;
 	srp_send_tsk_mgmt(target, req->index, scmnd->device->lun,
 			  SRP_TSK_ABORT_TASK);
@@ -1722,8 +1722,6 @@ static int srp_reset_device(struct scsi_cmnd *scmnd)
 
 	shost_printk(KERN_ERR, target->scsi_host, "SRP reset_device called\n");
 
-	if (target->qp_in_error)
-		return FAILED;
 	if (srp_send_tsk_mgmt(target, SRP_TAG_NO_REQ, scmnd->device->lun,
 			      SRP_TSK_LUN_RESET))
 		return FAILED;
-- 
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

[PATCH 04/20] ib_srp: Stop queueing if QP in error

[Bart Van Assche]

Fail SCSI commands if the QP between initiator and target is in the
error state. For connections monitored by the multipath software
the result is that multipathd doesn't have to wait until the SCSI
timeout has expired to find out that a path has failed.  Also,
introduce the function srp_handle_qp_err(), change the type of
qp_in_error from int into bool and move the initialization of that
variable from srp_reconnect_target() to srp_connect_target().

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 |   40
+++++++++++++++++++---------------
 drivers/infiniband/ulp/srp/ib_srp.h |    2 +-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c
b/drivers/infiniband/ulp/srp/ib_srp.c
index f333705..28a044e 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;
@@ -685,7 +687,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;
@@ -1256,6 +1257,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;
@@ -1263,15 +1273,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);
 	}
 }

@@ -1282,16 +1289,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);
 	}
 }

@@ -1309,7 +1313,8 @@ static int srp_queuecommand(struct Scsi_Host
*shost, struct scsi_cmnd *scmnd)
 		goto err;

 	if (target->state == SRP_TARGET_DEAD ||
-	    target->state == SRP_TARGET_REMOVED) {
+	    target->state == SRP_TARGET_REMOVED ||
+	    target->qp_in_error) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
 		return 0;
@@ -2269,7 +2274,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

[PATCH 05/20] ib_srp: Eliminate state SRP_TARGET_CONNECTING

[Bart Van Assche]

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

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 28a044e..35e1220 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -647,13 +647,16 @@ static void srp_reset_req(struct srp_target_port *target, struct srp_request *re
 
 static int srp_reconnect_target(struct srp_target_port *target)
 {
+	struct Scsi_Host *shost = target->scsi_host;
 	struct ib_qp_attr qp_attr;
 	struct ib_wc wc;
 	int i, ret;
 
-	if (!srp_change_state(target, SRP_TARGET_LIVE, SRP_TARGET_CONNECTING))
+	if (target->state != SRP_TARGET_LIVE)
 		return -EAGAIN;
 
+	scsi_target_block(&shost->shost_gendev);
+
 	srp_disconnect_target(target);
 	/*
 	 * Now get a new local CM ID so that we avoid confusing the
@@ -691,12 +694,13 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	if (ret)
 		goto err;
 
-	if (!srp_change_state(target, SRP_TARGET_CONNECTING, SRP_TARGET_LIVE))
-		ret = -EAGAIN;
+	scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
 
 	return ret;
 
 err:
+	scsi_target_unblock(&shost->shost_gendev, SDEV_TRANSPORT_OFFLINE);
+
 	shost_printk(KERN_ERR, target->scsi_host,
 		     PFX "reconnect failed (%d), removing target port.\n", ret);
 
@@ -710,7 +714,7 @@ err:
 	 * the flush_scheduled_work() in srp_remove_one().
 	 */
 	spin_lock_irq(&target->lock);
-	if (target->state == SRP_TARGET_CONNECTING) {
+	if (target->state == SRP_TARGET_LIVE) {
 		target->state = SRP_TARGET_DEAD;
 		INIT_WORK(&target->work, srp_remove_work);
 		queue_work(ib_wq, &target->work);
@@ -1309,9 +1313,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 ||
 	    target->qp_in_error) {
@@ -1376,7 +1377,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

[PATCH 06/20] ib_srp: Suppress superfluous error messages

[Bart Van Assche]

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

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 35e1220..ff3e15a 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:
@@ -1265,8 +1285,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;
 }
 
@@ -1627,6 +1650,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");
@@ -1941,6 +1965,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

[PATCH 07/20] ib_srp: Avoid that SCSI error handling triggers a crash

[Bart Van Assche]

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 |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index ff3e15a..b7a5e92 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1338,6 +1338,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 
 	if (target->state == SRP_TARGET_DEAD ||
 	    target->state == SRP_TARGET_REMOVED ||
+	    !target->connected ||
 	    target->qp_in_error) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
@@ -1690,6 +1691,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,
 
 	if (target->state == SRP_TARGET_DEAD ||
 	    target->state == SRP_TARGET_REMOVED ||
+	    !target->connected ||
 	    target->qp_in_error)
 		return -1;
 
-- 
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

[PATCH 08/20] ib_srp: Introduce the helper function, srp_remove_target()

[Bart Van Assche]

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 b7a5e92..88557f7 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

[PATCH 09/20] ib_srp: Eliminate state SRP_TARGET_DEAD

[Bart Van Assche]

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

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 |   89 ++++++++++++----------------------
 drivers/infiniband/ulp/srp/ib_srp.h |    5 +-
 2 files changed, 34 insertions(+), 60 deletions(-)

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

[PATCH 10/20] ib_srp: Keep processing commands during scsi_remove_host()

[Bart Van Assche]

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

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 |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index a7f42f3..0e7825a 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1336,9 +1336,7 @@ static int srp_queuecommand(struct Scsi_Host *shost, struct scsi_cmnd *scmnd)
 	unsigned long flags;
 	int len;
 
-	if (target->state == SRP_TARGET_REMOVED ||
-	    !target->connected ||
-	    target->qp_in_error) {
+	if (!target->connected || target->qp_in_error) {
 		scmnd->result = DID_BAD_TARGET << 16;
 		scmnd->scsi_done(scmnd);
 		return 0;
@@ -1688,9 +1686,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_REMOVED ||
-	    !target->connected ||
-	    target->qp_in_error)
+	if (!target->connected || target->qp_in_error)
 		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

[PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions

[Bart Van Assche]

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

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 |  104 ++++++++++++++++++++++++++++++-----
 drivers/infiniband/ulp/srp/ib_srp.h |    6 ++
 2 files changed, 95 insertions(+), 15 deletions(-)

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

Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions

[Sebastian Riemer]

Hi Bart,

we've triggered the WARN_ON() in srp_wait_last_send_wqe() by connecting
to a disabled SCST SRP target.

I would remove that one.

Cheers,
Sebastian

 
On 09.08.2012 17:53, Bart Van Assche wrote:
> Modify srp_disconnect_target() such that it waits until it is
> sure that no new IB completions will be received anymore.
>
> 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 |  104 ++++++++++++++++++++++++++++++-----
>  drivers/infiniband/ulp/srp/ib_srp.h |    6 ++
>  2 files changed, 95 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 0e7825a..4de7c46 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -40,7 +40,7 @@
>  #include <linux/parser.h>
>  #include <linux/random.h>
>  #include <linux/jiffies.h>
> -
> +#include <linux/delay.h>
>  #include <linux/atomic.h>
>  
>  #include <scsi/scsi.h>
> @@ -229,14 +229,16 @@ static int srp_create_target_ib(struct srp_target_port *target)
>  		return -ENOMEM;
>  
>  	target->recv_cq = ib_create_cq(target->srp_host->srp_dev->dev,
> -				       srp_recv_completion, NULL, target, SRP_RQ_SIZE, 0);
> +				       srp_recv_completion, NULL, target,
> +				       SRP_RQ_SIZE + 1, 0);
>  	if (IS_ERR(target->recv_cq)) {
>  		ret = PTR_ERR(target->recv_cq);
>  		goto err;
>  	}
>  
>  	target->send_cq = ib_create_cq(target->srp_host->srp_dev->dev,
> -				       srp_send_completion, NULL, target, SRP_SQ_SIZE, 0);
> +				       srp_send_completion, NULL, target,
> +				       SRP_SQ_SIZE + 1, 0);
>  	if (IS_ERR(target->send_cq)) {
>  		ret = PTR_ERR(target->send_cq);
>  		goto err_recv_cq;
> @@ -245,8 +247,8 @@ static int srp_create_target_ib(struct srp_target_port *target)
>  	ib_req_notify_cq(target->recv_cq, IB_CQ_NEXT_COMP);
>  
>  	init_attr->event_handler       = srp_qp_event;
> -	init_attr->cap.max_send_wr     = SRP_SQ_SIZE;
> -	init_attr->cap.max_recv_wr     = SRP_RQ_SIZE;
> +	init_attr->cap.max_send_wr     = SRP_SQ_SIZE + 1;
> +	init_attr->cap.max_recv_wr     = SRP_RQ_SIZE + 1;
>  	init_attr->cap.max_recv_sge    = 1;
>  	init_attr->cap.max_send_sge    = 1;
>  	init_attr->sq_sig_type         = IB_SIGNAL_ALL_WR;
> @@ -460,11 +462,69 @@ static bool srp_change_conn_state(struct srp_target_port *target,
>  	return changed;
>  }
>  
> +static void srp_wait_last_recv_wqe(struct srp_target_port *target)
> +{
> +	static struct ib_recv_wr wr = {
> +		.wr_id = SRP_LAST_RECV,
> +	};
> +	struct ib_recv_wr *bad_wr;
> +	int ret;
> +
> +	if (target->last_recv_wqe)
> +		return;
> +
> +	ret = ib_post_recv(target->qp, &wr, &bad_wr);
> +	if (ret < 0) {
> +		shost_printk(KERN_ERR, target->scsi_host,
> +			     "ib_post_recv() failed (%d)\n", ret);
> +		return;
> +	}
> +
> +	ret = wait_event_timeout(target->qp_wq, target->last_recv_wqe,
> +				 target->rq_tmo_jiffies);
> +	WARN(ret <= 0, "Timeout while waiting for last recv WQE (ret = %d)\n",
> +	     ret);
> +}
> +
> +static void srp_wait_last_send_wqe(struct srp_target_port *target)
> +{
> +	static struct ib_send_wr wr = {
> +		.wr_id = SRP_LAST_SEND,
> +	};
> +	struct ib_send_wr *bad_wr;
> +	unsigned long deadline = jiffies + target->rq_tmo_jiffies;
> +	int ret;
> +
> +	if (target->last_send_wqe)
> +		return;
> +
> +	ret = ib_post_send(target->qp, &wr, &bad_wr);
> +	if (ret < 0) {
> +		shost_printk(KERN_ERR, target->scsi_host,
> +			     "ib_post_send() failed (%d)\n", ret);
> +		return;
> +	}
> +
> +	while (!target->last_send_wqe && time_before(jiffies, deadline)) {
> +		srp_send_completion(target->send_cq, target);
> +		msleep(20);
> +	}
> +
> +	WARN_ON(!target->last_send_wqe);

<-- here it is - remove it

> +}
> +
>  static void srp_disconnect_target(struct srp_target_port *target)
>  {
> +	static struct ib_qp_attr qp_attr = {
> +		.qp_state = IB_QPS_ERR
> +	};
> +	int ret;
> +
>  	if (srp_change_conn_state(target, false)) {
>  		/* XXX should send SRP_I_LOGOUT request */
>  
> +		BUG_ON(!target->cm_id);
> +
>  		init_completion(&target->done);
>  		if (ib_send_cm_dreq(target->cm_id, NULL, 0)) {
>  			shost_printk(KERN_DEBUG, target->scsi_host,
> @@ -473,6 +533,20 @@ static void srp_disconnect_target(struct srp_target_port *target)
>  			wait_for_completion(&target->done);
>  		}
>  	}
> +
> +	if (target->cm_id) {
> +		ib_destroy_cm_id(target->cm_id);
> +		target->cm_id = NULL;
> +	}
> +
> +	if (target->qp) {
> +		ret = ib_modify_qp(target->qp, &qp_attr, IB_QP_STATE);
> +		WARN(ret != 0, "ib_modify_qp() failed: %d\n", ret);
> +
> +		srp_wait_last_recv_wqe(target);
> +
> +		srp_wait_last_send_wqe(target);
> +	}
>  }
>  
>  static void srp_free_req_data(struct srp_target_port *target)
> @@ -516,7 +590,6 @@ static void srp_remove_target(struct srp_target_port *target)
>  	srp_remove_host(target->scsi_host);
>  	scsi_remove_host(target->scsi_host);
>  	srp_disconnect_target(target);
> -	ib_destroy_cm_id(target->cm_id);
>  	srp_free_target_ib(target);
>  	srp_free_req_data(target);
>  	scsi_host_put(target->scsi_host);
> @@ -544,6 +617,8 @@ static int srp_connect_target(struct srp_target_port *target)
>  	WARN_ON(target->connected);
>  
>  	target->qp_in_error = false;
> +	target->last_recv_wqe = false;
> +	target->last_send_wqe = false;
>  
>  	ret = srp_lookup_path(target);
>  	if (ret)
> @@ -678,7 +753,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)
> @@ -704,11 +778,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)
> @@ -1285,7 +1354,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",
> @@ -1303,8 +1372,11 @@ static void srp_recv_completion(struct ib_cq *cq, void *target_ptr)
>  		if (likely(wc.status == IB_WC_SUCCESS)) {
>  			srp_handle_recv(target, &wc);
>  		} else {
> +			if (wc.wr_id == SRP_LAST_RECV) {
> +				target->last_recv_wqe = true;
> +				wake_up(&target->qp_wq);
> +			}
>  			srp_handle_qp_err(wc.status, wc.opcode, target);
> -			break;
>  		}
>  	}
>  }
> @@ -1320,8 +1392,9 @@ static void srp_send_completion(struct ib_cq *cq, void *target_ptr)
>  			iu = (struct srp_iu *) (uintptr_t) wc.wr_id;
>  			list_add(&iu->list, &target->free_tx);
>  		} else {
> +			if (wc.wr_id == SRP_LAST_SEND)
> +				target->last_send_wqe = true;
>  			srp_handle_qp_err(wc.status, wc.opcode, target);
> -			break;
>  		}
>  	}
>  }
> @@ -2255,6 +2328,7 @@ static ssize_t srp_create_target(struct device *dev,
>  	spin_lock_init(&target->lock);
>  	INIT_LIST_HEAD(&target->free_tx);
>  	INIT_LIST_HEAD(&target->free_reqs);
> +	init_waitqueue_head(&target->qp_wq);
>  	for (i = 0; i < SRP_CMD_SQ_SIZE; ++i) {
>  		struct srp_request *req = &target->req_ring[i];
>  
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
> index de2d0b3..1b11117 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.h
> +++ b/drivers/infiniband/ulp/srp/ib_srp.h
> @@ -76,6 +76,9 @@ enum {
>  
>  	SRP_MAP_ALLOW_FMR	= 0,
>  	SRP_MAP_NO_FMR		= 1,
> +
> +	SRP_LAST_RECV		= 0,
> +	SRP_LAST_SEND		= 0,
>  };
>  
>  enum srp_target_state {
> @@ -180,6 +183,9 @@ struct srp_target_port {
>  	struct completion	done;
>  	int			status;
>  	bool			qp_in_error;
> +	bool			last_recv_wqe;
> +	bool			last_send_wqe;
> +	wait_queue_head_t	qp_wq;
>  
>  	struct completion	tsk_mgmt_done;
>  	u8			tsk_mgmt_status;

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

Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions

[Bart Van Assche]

On 08/23/12 15:59, Sebastian Riemer wrote:
> we've triggered the WARN_ON() in srp_wait_last_send_wqe() by connecting
> to a disabled SCST SRP target.
> 
> I would remove that one.
> 
> [ ... ]
>
>> +	while (!target->last_send_wqe && time_before(jiffies, deadline)) {
>> +		srp_send_completion(target->send_cq, target);
>> +		msleep(20);
>> +	}
>> +
>> +	WARN_ON(!target->last_send_wqe);
> 
> <-- here it is - remove it

But why was that WARN_ON() statement hit ? srp_wait_last_send_wqe() is
invoked after the QP has been transitioned into the error state. It is
the responsibility of the HCA to generate an error completion for any
work queued on a QP that is in the error state. If that WARN_ON()
statement has been hit that means that it took more than the RC timeout
before the HCA finished processing earlier queued work and generated an
error completion. That's not really something I had expected.

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

Re: [PATCH 11/20] ib_srp: Make srp_disconnect_target() wait for IB completions

[Dongsu Park]

Hi Bart,

I'll try to explain, as Sebastian is on vacation.

On 23.08.2012 16:43, Bart Van Assche wrote:
> On 08/23/12 15:59, Sebastian Riemer wrote:
> > we've triggered the WARN_ON() in srp_wait_last_send_wqe() by connecting
> > to a disabled SCST SRP target.
> > 
> > I would remove that one.
> > 
> > [ ... ]
> >
> >> +	while (!target->last_send_wqe && time_before(jiffies, deadline)) {
> >> +		srp_send_completion(target->send_cq, target);
> >> +		msleep(20);
> >> +	}
> >> +
> >> +	WARN_ON(!target->last_send_wqe);
> > 
> > <-- here it is - remove it
> 
> But why was that WARN_ON() statement hit ? srp_wait_last_send_wqe() is
> invoked after the QP has been transitioned into the error state. It is
> the responsibility of the HCA to generate an error completion for any
> work queued on a QP that is in the error state. If that WARN_ON()
> statement has been hit that means that it took more than the RC timeout
> before the HCA finished processing earlier queued work and generated an
> error completion. That's not really something I had expected.

That occurs usually when releasing multiple targets at the same time.
A typical situation is unloading kernel module ib_srp.ko immediately,
which leads to tearing down every Infiniband connection.
But it doesn't occur always, which makes it hard for us to test.

Example of kernel trace:

WARNING: at drivers/infiniband/ulp/srp/ib_srp.c:529
srp_disconnect_target+0x317/0x320 [ib_srp]()
Hardware name: H8DGU
Modules linked in:
rdma_ucm rdma_cm iw_cm ib_addr ib_ipoib ib_uverbs ib_umad ib_srp
scsi_transport_srp scsi_tgt ib_cm ib_sa loop ib_mthca psmouse ib_mad
amd64_edac_mod edac_core i2c_piix4 evdev serio_raw edac_mce_amd
ib_core tpm_tis tpm tpm_bios processor button thermal_sys sg
hid_cherry sd_mod crc_t10dif usb_storage ahci libahci libata scsi_mod
[last unloaded: scsi_wait_scan]
Pid: 101, comm: kworker/1:1 Tainted: G W 3.2.8-pserver #1
Call Trace:
[<ffffffff81048dbb>] ? warn_slowpath_common+0x7b/0xc0
[<ffffffffa00a9b07>] ? srp_disconnect_target+0x317/0x320 [ib_srp]
[<ffffffff8106a640>] ? wake_up_bit+0x40/0x40
[<ffffffffa00ab0bf>] ? srp_remove_work+0x13f/0x1c0 [ib_srp]
[<ffffffffa00aaf80>] ? srp_free_req_data+0xd0/0xd0 [ib_srp]
[<ffffffff81063383>] ? process_one_work+0x113/0x470
[<ffffffff81065c73>] ? worker_thread+0x163/0x3e0
[<ffffffff81065b10>] ? manage_workers+0x200/0x200
[<ffffffff81065b10>] ? manage_workers+0x200/0x200
[<ffffffff8106a126>] ? kthread+0x96/0xa0
[<ffffffff8165f674>] ? kernel_thread_helper+0x4/0x10
[<ffffffff8106a090>] ? kthread_worker_fn+0x180/0x180
[<ffffffff8165f670>] ? gs_change+0x13/0x13

Regards,
Dongsu

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

[PATCH 12/20] ib_srp: Document sysfs attributes

[Bart Van Assche]

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

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

[PATCH 13/20] srp_transport: Fix atttribute registration

[Bart Van Assche]

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

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Robert Jennings <rcj-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
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

[PATCH 15/20] srp_transport: Document sysfs attributes

[Bart Van Assche]

Signed-off-by: Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Robert Jennings <rcj-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
Cc: David Dillow <dillowda-1Heg1YXhbW8@public.gmane.org>
Cc: Roland Dreier <roland-BHEL68pLQRGGvPXPguhicg@public.gmane.org>
---
 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

[PATCH 16/20] ib_srp: Allow SRP disconnect through sysfs

[Bart Van Assche]

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

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

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>
Cc: FUJITA Tomonori <fujita.tomonori-Zyj7fXuS5i5L9jVzuh4AOg@public.gmane.org>
Cc: Robert Jennings <rcj-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 Documentation/ABI/stable/sysfs-transport-srp |    7 +++++++
 drivers/infiniband/ulp/srp/ib_srp.c          |   10 ++++++++++
 drivers/scsi/scsi_transport_srp.c            |   22 +++++++++++++++++++++-
 include/scsi/scsi_transport_srp.h            |    8 ++++++++
 4 files changed, 46 insertions(+), 1 deletions(-)

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

[PATCH 17/20] ib_srp: Introduce a temporary variable in srp_remove_target()

[Bart Van Assche]

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 d90100e..8129156 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -584,15 +584,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

[PATCH 18/20] ib_srp: Maintain a single connection per I_T nexus

[Bart Van Assche]

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

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 |  120 +++++++++++++++++++++++++++++------
 drivers/infiniband/ulp/srp/ib_srp.h |   11 +++
 2 files changed, 110 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 8129156..b1af74a 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -430,6 +430,24 @@ static int srp_send_req(struct srp_target_port *target)
 	return status;
 }
 
+static bool srp_change_state(struct srp_target_port *target,
+			     enum srp_target_state old,
+			     enum srp_target_state new)
+{
+	bool changed = false;
+
+	BUG_ON(old == SRP_TARGET_REMOVED);
+
+	spin_lock_irq(&target->lock);
+	if (target->state == old) {
+		target->state = new;
+		changed = true;
+	}
+	spin_unlock_irq(&target->lock);
+
+	return changed;
+}
+
 static bool srp_queue_remove_work(struct srp_target_port *target)
 {
 	bool changed = false;
@@ -588,10 +606,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_del_scsi_host_attr(shost);
+		srp_remove_host(shost);
+		scsi_remove_host(shost);
+	}
+	mutex_unlock(&target->mutex);
 	srp_disconnect_target(target);
+
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
 	scsi_host_put(shost);
@@ -618,6 +641,30 @@ static void srp_rport_delete(struct srp_rport *rport)
 	srp_queue_remove_work(target);
 }
 
+/**
+ * srp_conn_unique() - Check whether the connection to a target is unique.
+ */
+static bool srp_conn_unique(struct srp_host *host,
+			    struct srp_target_port *target)
+{
+	struct srp_target_port *t;
+	bool ret = true;
+
+	spin_lock(&host->target_lock);
+	list_for_each_entry(t, &host->target_list, list) {
+		if (t != target &&
+		    target->id_ext == t->id_ext &&
+		    target->ioc_guid == t->ioc_guid &&
+		    target->initiator_ext == t->initiator_ext) {
+			ret = false;
+			break;
+		}
+	}
+	spin_unlock(&host->target_lock);
+
+	return ret;
+}
+
 static int srp_connect_target(struct srp_target_port *target)
 {
 	int retries = 3;
@@ -764,8 +811,18 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	struct ib_qp_attr qp_attr;
 	int i, ret;
 
-	if (target->state != SRP_TARGET_LIVE)
+	if (!srp_conn_unique(target->srp_host, target) &&
+	    srp_queue_remove_work(target)) {
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "Deleting SCSI host because obsolete\n");
+		return -ENXIO;
+	}
+
+	if (target->state == SRP_TARGET_REMOVED) {
+		shost_printk(KERN_DEBUG, target->scsi_host,
+			     PFX "Removal already scheduled\n");
 		return -EAGAIN;
+	}
 
 	scsi_target_block(&shost->shost_gendev);
 
@@ -1740,6 +1797,9 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "connection closed\n");
 
+		if (!srp_conn_unique(target->srp_host, target))
+			srp_queue_remove_work(target);
+
 		comp = 1;
 		target->status = 0;
 		break;
@@ -2040,16 +2100,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;
 }
 
@@ -2335,6 +2385,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);
@@ -2381,24 +2432,51 @@ 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_queue_remove_work(target))
+		shost_printk(KERN_INFO, target->scsi_host,
+			     PFX "deleting SCSI host (ret = %d).\n", ret);
+	return ret;
 
 err_free_ib:
 	srp_free_target_ib(target);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 1b11117..9b225f1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -81,7 +81,16 @@ enum {
 	SRP_LAST_SEND		= 0,
 };
 
+/**
+ * enum srp_target_state - State of an SRP target.
+ * @SRP_TARGET_CONNECTING: IB connection being established and SCSI host being
+ *                      added.
+ * @SRP_TARGET_LIVE: IB RC connection has been established.
+ * @SRP_TARGET_REMOVED: IB RC connection is about to be closed and SCSI host
+ *                      removal is pending.
+ */
 enum srp_target_state {
+	SRP_TARGET_CONNECTING,
 	SRP_TARGET_LIVE,
 	SRP_TARGET_REMOVED,
 };
@@ -166,6 +175,7 @@ struct srp_target_port {
 
 	u32			rq_tmo_jiffies;
 	bool			connected;
+	bool			scsi_host_added;
 
 	struct ib_cm_id	       *cm_id;
 
@@ -186,6 +196,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

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

[Bart Van Assche]

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

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

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

[PATCH 20/20] ib_srp: Add dev_loss_tmo support

[Bart Van Assche]

Remove an SRP host if either dev_loss_tmo expired or the target
closed the IB connection.

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 |   53 +++++++++++++++++++++++++++++-----
 drivers/infiniband/ulp/srp/ib_srp.h |    2 +
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index b1af74a..81b4432 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -603,17 +603,23 @@ 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;
+	struct srp_rport *rport = target->rport;
+	bool scsi_host_added = false;
 
 	WARN_ON(target->state != SRP_TARGET_REMOVED);
 
 	mutex_lock(&target->mutex);
-	if (target->scsi_host_added) {
+	swap(target->scsi_host_added, scsi_host_added);
+	mutex_unlock(&target->mutex);
+
+	if (scsi_host_added) {
 		srp_del_scsi_host_attr(shost);
+		srp_stop_rport(rport);
 		srp_remove_host(shost);
 		scsi_remove_host(shost);
 	}
-	mutex_unlock(&target->mutex);
 	srp_disconnect_target(target);
+	cancel_work_sync(&target->tl_err_work);
 
 	srp_free_target_ib(target);
 	srp_free_req_data(target);
@@ -824,9 +830,19 @@ static int srp_reconnect_target(struct srp_target_port *target)
 		return -EAGAIN;
 	}
 
-	scsi_target_block(&shost->shost_gendev);
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		scsi_target_block(&shost->shost_gendev);
+	mutex_unlock(&target->mutex);
 
 	srp_disconnect_target(target);
+	cancel_work_sync(&target->tl_err_work);
+
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		srp_stop_tl_fail_timers(target->rport);
+	mutex_unlock(&target->mutex);
+
 	/*
 	 * Now get a new local CM ID so that we avoid confusing the
 	 * target in case things are really fouled up.
@@ -858,12 +874,16 @@ static int srp_reconnect_target(struct srp_target_port *target)
 	if (ret)
 		goto err;
 
+	mutex_lock(&target->mutex);
 	scsi_target_unblock(&shost->shost_gendev, SDEV_RUNNING);
+	mutex_unlock(&target->mutex);
 
 	return ret;
 
 err:
+	mutex_lock(&target->mutex);
 	scsi_target_unblock(&shost->shost_gendev, SDEV_TRANSPORT_OFFLINE);
+	mutex_unlock(&target->mutex);
 
 	shost_printk(KERN_ERR, target->scsi_host,
 		     PFX "reconnect failed (%d), removing target port.\n", ret);
@@ -1416,15 +1436,32 @@ static void srp_handle_recv(struct srp_target_port *target, struct ib_wc *wc)
 			     PFX "Recv failed with error code %d\n", res);
 }
 
+/**
+ * srp_tl_err_work - Start the transport layer failure timers.
+ */
+static void srp_tl_err_work(struct work_struct *work)
+{
+	struct srp_target_port *target;
+
+	target = container_of(work, struct srp_target_port, tl_err_work);
+
+	mutex_lock(&target->mutex);
+	if (target->scsi_host_added)
+		srp_start_tl_fail_timers(target->rport);
+	mutex_unlock(&target->mutex);
+}
+
 static void srp_handle_qp_err(enum ib_wc_status wc_status,
 			      enum ib_wc_opcode wc_opcode,
 			      struct srp_target_port *target)
 {
-	if (target->connected && !target->qp_in_error)
+	if (target->connected && !target->qp_in_error) {
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "failed %s status %d\n",
 			     wc_opcode & IB_WC_RECV ? "receive" : "send",
 			     wc_status);
+		queue_work(system_long_wq, &target->tl_err_work);
+	}
 	target->qp_in_error = true;
 }
 
@@ -1791,15 +1828,13 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 		if (ib_send_cm_drep(cm_id, NULL, 0))
 			shost_printk(KERN_ERR, target->scsi_host,
 				     PFX "Sending CM DREP failed\n");
+		queue_work(system_long_wq, &target->tl_err_work);
 		break;
 
 	case IB_CM_TIMEWAIT_EXIT:
 		shost_printk(KERN_ERR, target->scsi_host,
 			     PFX "connection closed\n");
-
-		if (!srp_conn_unique(target->srp_host, target))
-			srp_queue_remove_work(target);
-
+		srp_queue_remove_work(target);
 		comp = 1;
 		target->status = 0;
 		break;
@@ -2099,6 +2134,7 @@ static int srp_add_target(struct srp_host *host, struct srp_target_port *target)
 	}
 
 	rport->lld_data = target;
+	target->rport = rport;
 
 	return 0;
 }
@@ -2386,6 +2422,7 @@ static ssize_t srp_create_target(struct device *dev,
 			     target->cmd_sg_cnt * sizeof (struct srp_direct_buf);
 
 	mutex_init(&target->mutex);
+	INIT_WORK(&target->tl_err_work, srp_tl_err_work);
 	INIT_WORK(&target->remove_work, srp_remove_work);
 	spin_lock_init(&target->lock);
 	INIT_LIST_HEAD(&target->free_tx);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index 9b225f1..ed22ddc 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -164,6 +164,7 @@ struct srp_target_port {
 	u16			io_class;
 	struct srp_host	       *srp_host;
 	struct Scsi_Host       *scsi_host;
+	struct srp_rport       *rport;
 	char			target_name[32];
 	unsigned int		scsi_id;
 	unsigned int		sg_tablesize;
@@ -187,6 +188,7 @@ struct srp_target_port {
 	struct srp_iu	       *rx_ring[SRP_RQ_SIZE];
 	struct srp_request	req_ring[SRP_CMD_SQ_SIZE];
 
+	struct work_struct	tl_err_work;
 	struct work_struct	remove_work;
 
 	struct list_head	list;
-- 
1.7.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

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

[Dongsu Park]

Hi Bart,

while testing ib_srp based on your srp-ha,
we sometimes hit kernel crashes with the call trace below.

How to reproduce:

0. Kernel 3.2.15 with SCST v4193 on the target,
   Kernel 3.2.8 with ib_srp-ha on the initiator.
1. Configure 500+ vdisks on target, and get initiator connected.
2. Exchange data intensively, which works well.
3. (On initiator) delete SRP remote port occasionally, e.g.
   # echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete
   And configure again the SRP target.
4. (On target) disable Infiniband interface, and enable it again.
5. Repeat 3 and 4.

Then the initiator's kernel suddenly crashes. (but not always)

Do you have any idea why?

Thanks in advance,
Dongsu

---------------------------------------------------------------
BUG: unable to handle kernel paging request at 0000000000010001
IP: [<ffffffff8139ec55>] strnlen+0x5/0x40
PGD 212fea067 PUD 2162f8067 PMD 0
Oops: 0000 [#1] SMP
CPU 0
Modules linked in: ib_srp scsi_transport_srp scsi_tgt rdma_ucm rdma_cm
iw_cm ib_addr ib_ipoib ib_cm ib_sa ib_uverbs ib_umad loop psmouse
serio_raw evdev i2c_piix4 tpm_tis tpm tpm_bios ib_mthca sg ib_mad
processor amd64_edac_mod edac_core thermal_sys edac_mce_amd ib_core
button sd_mod crc_t10dif hid_cherry usb_storage ahci libahci libata
scsi_mod [last unloaded: scsi_wait_scan]

Pid: 2311, comm: kworker/0:2 Not tainted 3.2.8 #1 Supermicro
H8DGU/H8DGU
RIP: 0010:[<ffffffff8139ec55>]  [<ffffffff8139ec55>] strnlen+0x5/0x40
RSP: 0018:ffff880215fe3c28  EFLAGS: 00010086
RAX: ffffffff81915991 RBX: ffffffff81ba5497 RCX: ffffffffff0a0004
RDX: 0000000000010001 RSI: ffffffffffffffff RDI: 0000000000010001
RBP: ffffffff81ba5860 R08: 000000000000fffb R09: 0000000000000001
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000010001
R13: ffffffffffffffff R14: 00000000ffffffff R15: 0000000000000000
FS:  00007faafb63e700(0000) GS:ffff880217c00000(0000)
knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000000010001 CR3: 0000000212f87000 CR4: 00000000000006f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
Process kworker/0:2 (pid: 2311, threadinfo ffff880215fe2000, task
ffff88020f2ce540)
Stack:
 ffffffff813a023c 0000000000000000 ffffffff81ba5497 ffffffffa0131d82
 ffffffffa0131d80 ffff880215fe3da0 ffffffff81ba5860 ffff880215fe3c90
 ffffffff813a142d 0000000000000016 ffffffff81ba5460 0000000000000400
Call Trace:
 [<ffffffff813a023c>] ? string+0x4c/0xe0
 [<ffffffff813a142d>] ? vsnprintf+0x1ed/0x5b0
 [<ffffffffa0131900>] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp]
 [<ffffffff813a18a9>] ? vscnprintf+0x9/0x20
 [<ffffffff81049b7f>] ? vprintk+0xaf/0x440
 [<ffffffff810f3cc0>] ? next_online_pgdat+0x20/0x50
 [<ffffffff810f3d20>] ? next_zone+0x30/0x40
 [<ffffffff810f4c60>] ? refresh_cpu_vm_stats+0xf0/0x160
 [<ffffffffa0131900>] ? do_srp_rport_del+0x30/0x30 [scsi_transport_srp]
 [<ffffffff816533b6>] ? printk+0x40/0x4a
 [<ffffffffa013192d>] ? rport_dev_loss_timedout+0x2d/0xa0
[scsi_transport_srp]
 [<ffffffff81063383>] ? process_one_work+0x113/0x470
 [<ffffffff81065c73>] ? worker_thread+0x163/0x3e0
 [<ffffffff81065b10>] ? manage_workers+0x200/0x200
 [<ffffffff81065b10>] ? manage_workers+0x200/0x200
 [<ffffffff8106a126>] ? kthread+0x96/0xa0
 [<ffffffff8165f674>] ? kernel_thread_helper+0x4/0x10
 [<ffffffff8106a090>] ? kthread_worker_fn+0x180/0x180
 [<ffffffff8165f670>] ? gs_change+0x13/0x13
Code: 1f 80 00 00 00 00 31 c0 80 3f 00 48 89 fa 74 14 66 0f 1f 44 00 00
48 ff c2 80 3a 00 75 f8 48 89 d0 48 29 f8 f3 c3 48 85 f6 74 27 <80> 3f
00 74 22 48 ff ce 48 89 f8 eb 0e 66 0f 1f 44 00 00 48 ff
RIP  [<ffffffff8139ec55>] strnlen+0x5/0x40
RSP <ffff880215fe3c28>
CR2: 0000000000010001
---[ end trace d55b61cd78c54a0a ]---
BUG: unable to handle kernel paging request at fffffffffffffff8


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

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

[Bart Van Assche]

On 08/27/12 18:37, Dongsu Park wrote:
> while testing ib_srp based on your srp-ha,
> we sometimes hit kernel crashes with the call trace below.
> 
> How to reproduce:
> 
> 0. Kernel 3.2.15 with SCST v4193 on the target,
>    Kernel 3.2.8 with ib_srp-ha on the initiator.
> 1. Configure 500+ vdisks on target, and get initiator connected.
> 2. Exchange data intensively, which works well.
> 3. (On initiator) delete SRP remote port occasionally, e.g.
>    # echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete
>    And configure again the SRP target.
> 4. (On target) disable Infiniband interface, and enable it again.
> 5. Repeat 3 and 4.
> 
> Then the initiator's kernel suddenly crashes. (but not always)
> 
> Do you have any idea why?

Hello Dongsu,

That's unfortunate. I've just finished running the above test 1000 times
on my test setup. The test ran perfectly - login succeeded every time,
the test finished in the expected time, no kernel crash did occur and no
memory was leaked. I've been running my test with kernel 3.6-rc3 instead
of kernel 3.2.8 though. Can you repeat your test with kernel 3.6-rc3 on
the initiator system instead of kernel 3.2.8 ? The 3.6-rc3 kernel
contains multiple patches that improve robustness with regard to SCSI
device removal.

Thanks,

Bart.

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

[Dongsu Park]

Hi Bart,

On 28.08.2012 10:04, Bart Van Assche wrote:
> On 08/27/12 18:37, Dongsu Park wrote:
> > while testing ib_srp based on your srp-ha,
> > we sometimes hit kernel crashes with the call trace below.
> > 
> > How to reproduce:
> > 
> > 0. Kernel 3.2.15 with SCST v4193 on the target,
> >    Kernel 3.2.8 with ib_srp-ha on the initiator.
> > 1. Configure 500+ vdisks on target, and get initiator connected.
> > 2. Exchange data intensively, which works well.
> > 3. (On initiator) delete SRP remote port occasionally, e.g.
> >    # echo "1" > /sys/class/srp_remote_ports/port-6\:1/delete
> >    And configure again the SRP target.
> > 4. (On target) disable Infiniband interface, and enable it again.
> > 5. Repeat 3 and 4.
> > 
> > Then the initiator's kernel suddenly crashes. (but not always)
> > 
> > Do you have any idea why?
> 
> Hello Dongsu,
> 
> That's unfortunate. I've just finished running the above test 1000 times
> on my test setup. The test ran perfectly - login succeeded every time,
> the test finished in the expected time, no kernel crash did occur and no
> memory was leaked. I've been running my test with kernel 3.6-rc3 instead
> of kernel 3.2.8 though. Can you repeat your test with kernel 3.6-rc3 on
> the initiator system instead of kernel 3.2.8 ? The 3.6-rc3 kernel
> contains multiple patches that improve robustness with regard to SCSI
> device removal.

Ok, when I get a chance to set up a new test system with kernel 3.6-rc3,
I'll do a new test and let you know.

By the way, as long as I've observed today, the crash occurs only if
rport_dev_loss_timedout() is called. It means, without device loss,
a simple rport_delete does not make any crash.

Is that probably because arguments to pr_err() are accessing to invalid
addresses?

drivers/scsi/scsi_transport_srp.c:275

        pr_err("SRP transport: dev_loss_tmo (%ds) expired - removing %s.\n",
               rport->dev_loss_tmo, dev_name(&rport->dev));

Cheers,
Dongsu

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

[Bart Van Assche]

On 08/28/12 12:25, Dongsu Park wrote:
> By the way, as long as I've observed today, the crash occurs only if
> rport_dev_loss_timedout() is called. It means, without device loss,
> a simple rport_delete does not make any crash.
> 
> Is that probably because arguments to pr_err() are accessing to invalid
> addresses?
> 
> drivers/scsi/scsi_transport_srp.c:275
> 
>         pr_err("SRP transport: dev_loss_tmo (%ds) expired - removing %s.\n",
>                rport->dev_loss_tmo, dev_name(&rport->dev));

It's not clear to me how that could happen. The dev_loss timer is
stopped before the rport device is removed. See also the
srp_stop_rport() call (which stops the dev_loss timer) and
srp_remove_host() call (which removes the rport) in srp_remove_target().

Bart.

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

[Bart Van Assche]

On 08/09/12 17:41, Bart Van Assche wrote:
> [ ... ]

Hello Dave,

More than six weeks have elapsed since I posted version four of this 
patch series. It would be appreciated if you could tell me when review 
comments for this patch series will be posted. I'd also like to remind 
you that some time ago you asked other people to wait with posting more 
ib_srp patches until this patch series is upstream [1, 2].

Thanks,

Bart.

[1] David Dillow, Re: [PATCH 1/1] ib_srp: Infiniband srp fast failover
patch, May 29, 2012
(http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11801.html).
[2] David Dillow, Re: [PATCH] srp: convert SRP_RQ_SHIFT into a module
parameter, May 29, 2012
(http://www.mail-archive.com/linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg11802.html).

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

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

[David Dillow]

On Tue, 2012-09-25 at 17:05 +0200, Bart Van Assche wrote:
> On 08/09/12 17:41, Bart Van Assche wrote:
> > [ ... ]
> 
> Hello Dave,
> 
> More than six weeks have elapsed since I posted version four of this 
> patch series. It would be appreciated if you could tell me when review 
> comments for this patch series will be posted. I'd also like to remind 
> you that some time ago you asked other people to wait with posting more 
> ib_srp patches until this patch series is upstream [1, 2].

Yes, it has taken me far more time than I expected to get to these. I am
in the middle of fiscal-year-end thrash, and will attend to the SRP
backlog next week.
-- 
Dave Dillow
National Center for Computational Science
Oak Ridge National Laboratory
(865) 241-6602 office

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

[Bart Van Assche]

On 09/27/12 02:31, David Dillow wrote:
> On Tue, 2012-09-25 at 17:05 +0200, Bart Van Assche wrote:
>> On 08/09/12 17:41, Bart Van Assche wrote:
>>> [ ... ]
>>
>> Hello Dave,
>>
>> More than six weeks have elapsed since I posted version four of this
>> patch series. It would be appreciated if you could tell me when review
>> comments for this patch series will be posted. I'd also like to remind
>> you that some time ago you asked other people to wait with posting more
>> ib_srp patches until this patch series is upstream [1, 2].
>
> Yes, it has taken me far more time than I expected to get to these. I am
> in the middle of fiscal-year-end thrash, and will attend to the SRP
> backlog next week.

(replying to an e-mail of one month ago)

Hello Dave,

Will you have time next week to review the srp-ha patch series ?

Thanks,

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

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

[David Dillow]

On Fri, 2012-11-23 at 16:07 +0100, Bart Van Assche wrote:
> On 09/27/12 02:31, David Dillow wrote:
> > On Tue, 2012-09-25 at 17:05 +0200, Bart Van Assche wrote:
> >> On 08/09/12 17:41, Bart Van Assche wrote:
> >>> [ ... ]
> >>
> >> Hello Dave,
> >>
> >> More than six weeks have elapsed since I posted version four of this
> >> patch series. It would be appreciated if you could tell me when review
> >> comments for this patch series will be posted. I'd also like to remind
> >> you that some time ago you asked other people to wait with posting more
> >> ib_srp patches until this patch series is upstream [1, 2].
> >
> > Yes, it has taken me far more time than I expected to get to these. I am
> > in the middle of fiscal-year-end thrash, and will attend to the SRP
> > backlog next week.
> 
> (replying to an e-mail of one month ago)
> 
> Hello Dave,
> 
> Will you have time next week to review the srp-ha patch series ?

I just pushed my first patch for your review; fingers crossed to build
some momentum.
-- 
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

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

[Bart Van Assche]

On 08/09/12 15:41, Bart Van Assche wrote:
> [ ... ]

The patch series is also available on top of 3.6-rc1 here:
http://github.com/bvanassche/linux/tree/srp-ha

Bart.

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

[Joseph Glanville]

Hi Bart,

I have tested this series, all issues relating to hard removing
targets are resolved.
Thanks!

Joseph.

On 10 August 2012 02:18, Bart Van Assche <bvanassche-HInyCGIudOg@public.gmane.org> wrote:
> On 08/09/12 15:41, Bart Van Assche wrote:
>> [ ... ]
>
> The patch series is also available on top of 3.6-rc1 here:
> http://github.com/bvanassche/linux/tree/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



-- 
CTO | Orion Virtualisation Solutions | www.orionvm.com.au
Phone: 1300 56 99 52 | Mobile: 0428 754 846
--
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