[ofa-general] [RFC PATCH] IB/srp: respect target credit limit

David Dillow dillowda at ornl.gov
Mon Dec 17 14:03:19 PST 2007


The SRP initiator will currently send requests, even if we have no
credits available. The results of sending extra requests is
vendor-specific, but on some devices, overrunning our credits will cost
us 85% of peak performance -- e.g. 100 MB/s vs 720 MB/s. Other devices
may just drop them.

This patch will tell the SCSI mid-layer to queue requests if there are
less than two credits remaining, and will not issue a task management
request if there are no credits remaining. The mid-layer will retry the
queued command once an outstanding command completes.

Also, it adds a queue length parameter to the target options, so that
the maximum number of commands the SCSI mid-layer will try to send can
be capped before it hits the credit limit.

Signed-off-by: David Dillow <dillowda at ornl.gov>
---

A cleanup would be to get rid of the zero_req_lim counter, as it should
never be hit now.

 ib_srp.c |   49 +++++++++++++++++++++++++++++++++++++++++++------
 ib_srp.h |    5 +++++
 2 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 950228f..18dbe89 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -925,6 +925,28 @@ static int srp_post_recv(struct srp_target_port *target)
 	return ret;
 }
 
+/* Must be called with target->scsi_host->host_lock held to protect
+ * req_lim. Lock cannot be dropped between here and srp_use_credit().
+ */
+static int srp_check_credit(struct srp_target_port *target,
+				enum srp_request_type req_type)
+{
+	/* Each request requires one credit, and we hold a credit in reserve
+	 * so that we can send task management requests.
+	 */
+	s32 min = (req_type == SRP_REQ_TASK_MGMT) ? 1 : 2;
+	return target->req_lim < min;
+}
+
+/* Must be called with target->scsi_host->host_lock held to protect
+ * req_lim. Lock cannot be dropped between the call to srp_check_credit()
+ * and the call to srp_use_credit()
+ */
+static void srp_use_credit(struct srp_target_port *target)
+{
+	target->req_lim--;
+}
+
 /*
  * Must be called with target->scsi_host->host_lock held to protect
  * req_lim and tx_head.  Lock cannot be dropped between call here and
@@ -965,10 +987,8 @@ static int __srp_post_send(struct srp_target_port *target,
 
 	ret = ib_post_send(target->qp, &wr, &bad_wr);
 
-	if (!ret) {
+	if (!ret)
 		++target->tx_head;
-		--target->req_lim;
-	}
 
 	return ret;
 }
@@ -993,6 +1013,9 @@ static int srp_queuecommand(struct scsi_cmnd *scmnd,
 		return 0;
 	}
 
+	if (srp_check_credit(target, SRP_REQ_NORMAL))
+		goto err;
+
 	iu = __srp_get_tx_iu(target);
 	if (!iu)
 		goto err;
@@ -1039,6 +1062,8 @@ static int srp_queuecommand(struct scsi_cmnd *scmnd,
 		goto err_unmap;
 	}
 
+	srp_use_credit(target);
+
 	list_move_tail(&req->list, &target->req_queue);
 
 	return 0;
@@ -1180,9 +1205,6 @@ static int srp_cm_handler(struct ib_cm_id *cm_id, struct ib_cm_event *event)
 
 			target->max_ti_iu_len = be32_to_cpu(rsp->max_ti_iu_len);
 			target->req_lim       = be32_to_cpu(rsp->req_lim_delta);
-
-			target->scsi_host->can_queue = min(target->req_lim,
-							   target->scsi_host->can_queue);
 		} else {
 			printk(KERN_WARNING PFX "Unhandled RSP opcode %#x\n", opcode);
 			target->status = -ECONNRESET;
@@ -1283,6 +1305,9 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,
 
 	init_completion(&req->done);
 
+	if (srp_check_credit(target, SRP_REQ_TASK_MGMT))
+		goto out;
+
 	iu = __srp_get_tx_iu(target);
 	if (!iu)
 		goto out;
@@ -1300,6 +1325,7 @@ static int srp_send_tsk_mgmt(struct srp_target_port *target,
 		goto out;
 
 	req->tsk_mgmt = iu;
+	srp_use_credit(target);
 
 	spin_unlock_irq(target->scsi_host->host_lock);
 
@@ -1540,6 +1566,7 @@ static struct scsi_host_template srp_template = {
 	.eh_device_reset_handler	= srp_reset_device,
 	.eh_host_reset_handler		= srp_reset_host,
 	.can_queue			= SRP_SQ_SIZE,
+	.max_host_blocked		= 1,
 	.this_id			= -1,
 	.cmd_per_lun			= SRP_SQ_SIZE,
 	.use_clustering			= ENABLE_CLUSTERING,
@@ -1610,6 +1637,7 @@ enum {
 	SRP_OPT_MAX_CMD_PER_LUN	= 1 << 6,
 	SRP_OPT_IO_CLASS	= 1 << 7,
 	SRP_OPT_INITIATOR_EXT	= 1 << 8,
+	SRP_OPT_QUEUE_LEN	= 1 << 9,
 	SRP_OPT_ALL		= (SRP_OPT_ID_EXT	|
 				   SRP_OPT_IOC_GUID	|
 				   SRP_OPT_DGID		|
@@ -1627,6 +1655,7 @@ static match_table_t srp_opt_tokens = {
 	{ SRP_OPT_MAX_CMD_PER_LUN,	"max_cmd_per_lun=%d" 	},
 	{ SRP_OPT_IO_CLASS,		"io_class=%x"		},
 	{ SRP_OPT_INITIATOR_EXT,	"initiator_ext=%s"	},
+	{ SRP_OPT_QUEUE_LEN,		"queue_len=%d" 		},
 	{ SRP_OPT_ERR,			NULL 			}
 };
 
@@ -1754,6 +1783,14 @@ static int srp_parse_options(const char *buf, struct srp_target_port *target)
 			kfree(p);
 			break;
 
+		case SRP_OPT_QUEUE_LEN:
+			if (match_int(args, &token)) {
+				printk(KERN_WARNING PFX "bad queue_len parameter '%s'\n", p);
+				goto out;
+			}
+			target->scsi_host->can_queue = min(token, SRP_SQ_SIZE);
+			break;
+
 		default:
 			printk(KERN_WARNING PFX "unknown parameter or missing value "
 			       "'%s' in target creation request\n", p);
diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h
index e3573e7..4a3c1f3 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.h
+++ b/drivers/infiniband/ulp/srp/ib_srp.h
@@ -79,6 +79,11 @@ enum srp_target_state {
 	SRP_TARGET_REMOVED
 };
 
+enum srp_request_type {
+	SRP_REQ_NORMAL,
+	SRP_REQ_TASK_MGMT,
+};
+
 struct srp_device {
 	struct list_head	dev_list;
 	struct ib_device       *dev;





More information about the general mailing list