From 1d3d98c4cf4698324d71b29c84dcdc7802444376 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:12:10 -0700 Subject: [PATCH 1/8] IB/srp: Fix a spelling error in a source code comment Change one occurrence of "boundries" into "boundaries". Signed-off-by: Bart Van Assche Reviewed-by: Sagi Grimberg Cc: Christoph Hellwig Cc: Laurence Oberman Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index b6bf204960210..9c58676459c47 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1398,7 +1398,7 @@ static int srp_map_sg_entry(struct srp_map_state *state, /* * If the last entry of the MR wasn't a full page, then we need to * close it out and start a new one -- we can only merge at page - * boundries. + * boundaries. */ ret = 0; if (len != dev->mr_page_size) From 6ec2ba02e6b7fc4fcf8032bfd2bc6e6e20ca9c4a Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:12:47 -0700 Subject: [PATCH 2/8] IB/srp: Fix a comment The free request list was removed through patch "IB/srp: Use block layer tags". Hence update a comment that refers to that free request list. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Laurence Oberman Reviewed-by: Sagi Grimberg Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 9c58676459c47..8c0827a11ece7 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1112,7 +1112,7 @@ static struct scsi_cmnd *srp_claim_req(struct srp_rdma_ch *ch, } /** - * srp_free_req() - Unmap data and add request to the free request list. + * srp_free_req() - Unmap data and adjust ch->req_lim. * @ch: SRP RDMA channel. * @req: Request to be freed. * @scmnd: SCSI command associated with @req. From 77269cdfcab5f01ebd2095273f4060fbe1001e68 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:13:09 -0700 Subject: [PATCH 3/8] IB/srp: Document srp_map_data() return value Signed-off-by: Bart Van Assche Reviewed-by: Sagi Grimberg Cc: Christoph Hellwig Cc: Laurence Oberman Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 8c0827a11ece7..269f5ebe82de2 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1534,6 +1534,15 @@ static int srp_map_idb(struct srp_rdma_ch *ch, struct srp_request *req, return 0; } +/** + * srp_map_data() - map SCSI data buffer onto an SRP request + * @scmnd: SCSI command to map + * @ch: SRP RDMA channel + * @req: SRP request + * + * Returns the length in bytes of the SRP_CMD IU or a negative value if + * mapping failed. + */ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, struct srp_request *req) { From e012f3639c95498d4e8d7a9f44e33f1cd2f241b0 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:13:35 -0700 Subject: [PATCH 4/8] IB/srp: Fix srp_map_data() error paths Ensure that req->nmdesc is set correctly in srp_map_sg() if mapping fails. Avoid that mapping failure causes a memory descriptor leak. Report srp_map_sg() failure to the caller. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Laurence Oberman Reviewed-by: Sagi Grimberg Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 269f5ebe82de2..47e753d9ab5b6 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1428,8 +1428,6 @@ static int srp_map_sg_fmr(struct srp_map_state *state, struct srp_rdma_ch *ch, if (ret) return ret; - req->nmdesc = state->nmdesc; - return 0; } @@ -1454,8 +1452,6 @@ static int srp_map_sg_fr(struct srp_map_state *state, struct srp_rdma_ch *ch, state->sg = sg_next(state->sg); } - req->nmdesc = state->nmdesc; - return 0; } @@ -1475,8 +1471,6 @@ static int srp_map_sg_dma(struct srp_map_state *state, struct srp_rdma_ch *ch, target->global_mr->rkey); } - req->nmdesc = state->nmdesc; - return 0; } @@ -1610,11 +1604,14 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, memset(&state, 0, sizeof(state)); if (dev->use_fast_reg) - srp_map_sg_fr(&state, ch, req, scat, count); + ret = srp_map_sg_fr(&state, ch, req, scat, count); else if (dev->use_fmr) - srp_map_sg_fmr(&state, ch, req, scat, count); + ret = srp_map_sg_fmr(&state, ch, req, scat, count); else - srp_map_sg_dma(&state, ch, req, scat, count); + ret = srp_map_sg_dma(&state, ch, req, scat, count); + req->nmdesc = state.nmdesc; + if (ret < 0) + goto unmap; /* We've mapped the request, now pull as much of the indirect * descriptor table as we can into the command buffer. If this @@ -1637,7 +1634,8 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, !target->allow_ext_sg)) { shost_printk(KERN_ERR, target->scsi_host, "Could not fit S/G list into SRP_CMD\n"); - return -EIO; + ret = -EIO; + goto unmap; } count = min(state.ndesc, target->cmd_sg_cnt); @@ -1655,7 +1653,7 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, ret = srp_map_idb(ch, req, state.gen.next, state.gen.end, idb_len, &idb_rkey); if (ret < 0) - return ret; + goto unmap; req->nmdesc++; } else { idb_rkey = cpu_to_be32(target->global_mr->rkey); @@ -1681,6 +1679,10 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, cmd->buf_fmt = fmt; return len; + +unmap: + srp_unmap_data(scmnd, ch, req); + return ret; } /* From fa9863f869202a4ccc673cbd8dd326bf54a8efff Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:13:57 -0700 Subject: [PATCH 5/8] IB/srp: Introduce target->mr_pool_size This patch does not change any functionality. Signed-off-by: Bart Van Assche Reviewed-by: Sagi Grimberg Cc: Christoph Hellwig Cc: Laurence Oberman Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 6 +++--- drivers/infiniband/ulp/srp/ib_srp.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 47e753d9ab5b6..92d2d98c9831e 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -316,7 +316,7 @@ static struct ib_fmr_pool *srp_alloc_fmr_pool(struct srp_target_port *target) struct ib_fmr_pool_param fmr_param; memset(&fmr_param, 0, sizeof(fmr_param)); - fmr_param.pool_size = target->scsi_host->can_queue; + fmr_param.pool_size = target->mr_pool_size; fmr_param.dirty_watermark = fmr_param.pool_size / 4; fmr_param.cache = 1; fmr_param.max_pages_per_fmr = dev->max_pages_per_mr; @@ -441,8 +441,7 @@ static struct srp_fr_pool *srp_alloc_fr_pool(struct srp_target_port *target) { struct srp_device *dev = target->srp_host->srp_dev; - return srp_create_fr_pool(dev->dev, dev->pd, - target->scsi_host->can_queue, + return srp_create_fr_pool(dev->dev, dev->pd, target->mr_pool_size, dev->max_pages_per_mr); } @@ -3229,6 +3228,7 @@ static ssize_t srp_create_target(struct device *dev, } target_host->sg_tablesize = target->sg_tablesize; + target->mr_pool_size = target->scsi_host->can_queue; target->indirect_size = target->sg_tablesize * sizeof (struct srp_direct_buf); target->max_iu_len = sizeof (struct srp_cmd) + diff --git a/drivers/infiniband/ulp/srp/ib_srp.h b/drivers/infiniband/ulp/srp/ib_srp.h index 9e05ce4a04fd0..a00914cdd44e5 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.h +++ b/drivers/infiniband/ulp/srp/ib_srp.h @@ -202,6 +202,7 @@ struct srp_target_port { char target_name[32]; unsigned int scsi_id; unsigned int sg_tablesize; + int mr_pool_size; int queue_size; int req_ring_size; int comp_vector; From ffc548bb3601f0250474afcfa10ceb0b8b8b9764 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:14:15 -0700 Subject: [PATCH 6/8] IB/srp: Avoid that mapping failure triggers an infinite loop The srp_queuecommand() function translates ENOMEM into QUEUE_FULL which causes the SCSI mid-layer to retry the command. All other error codes are translated into DID_ERROR which causes the SCSI command to fail. Return E2BIG if mapping will always fail to prevent that the SCSI mid-layer keeps resubmitting a command forever. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Laurence Oberman Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index 92d2d98c9831e..fbd2edbedf055 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1681,6 +1681,8 @@ static int srp_map_data(struct scsi_cmnd *scmnd, struct srp_rdma_ch *ch, unmap: srp_unmap_data(scmnd, ch, req); + if (ret == -ENOMEM && req->nmdesc >= target->mr_pool_size) + ret = -E2BIG; return ret; } From 3b59b7a693b0e5b2dc244bcd78899aa2585a434b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:14:43 -0700 Subject: [PATCH 7/8] IB/srp: Move code out of a loop Since all srp_map_finish_fr() callers pass a non-zero value as the fourth argument (sg_nents), the sg_nents == 0 check in that function can be removed. Add a count == 0 check in the caller of that function. Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Laurence Oberman Reviewed-by: Sagi Grimberg Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index fbd2edbedf055..ce2c379272cfb 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1315,9 +1315,6 @@ static int srp_map_finish_fr(struct srp_map_state *state, WARN_ON_ONCE(!dev->use_fast_reg); - if (sg_nents == 0) - return 0; - if (sg_nents == 1 && target->global_mr) { srp_map_desc(state, sg_dma_address(state->sg), sg_dma_len(state->sg), @@ -1439,6 +1436,9 @@ static int srp_map_sg_fr(struct srp_map_state *state, struct srp_rdma_ch *ch, state->fr.end = req->fr_list + ch->target->cmd_sg_cnt; state->sg = scat; + if (count == 0) + return 0; + while (count) { int i, n; From 3849e44d1c4b11c9e0f0f0343a0360b5e9594fb6 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Fri, 22 Apr 2016 14:15:04 -0700 Subject: [PATCH 8/8] IB/srp: Move common code into the caller Signed-off-by: Bart Van Assche Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Laurence Oberman Reviewed-by: Sagi Grimberg Reviewed-by: Christoph Hellwig Signed-off-by: Doug Ledford --- drivers/infiniband/ulp/srp/ib_srp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c index ce2c379272cfb..427dee3a3ab04 100644 --- a/drivers/infiniband/ulp/srp/ib_srp.c +++ b/drivers/infiniband/ulp/srp/ib_srp.c @@ -1409,7 +1409,6 @@ static int srp_map_sg_fmr(struct srp_map_state *state, struct srp_rdma_ch *ch, struct scatterlist *sg; int i, ret; - state->desc = req->indirect_desc; state->pages = req->map_page; state->fmr.next = req->fmr_list; state->fmr.end = req->fmr_list + ch->target->cmd_sg_cnt;