On 2/5/2019 6:33 PM, Bart Van Assche wrote:
On Mon, 2019-02-04 at 19:50 +0200, Max Gurtovoy wrote:
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 31d91538bbf4..fa56343aeb98 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -1511,15 +1511,15 @@ static void srp_reg_mr_err_done(struct ib_cq *cq, struct ib_wc *wc)
}
/*
- * Map up to sg_nents elements of state->sg where *sg_offset_p is the offset
- * where to start in the first element. If sg_offset_p != NULL then
- * *sg_offset_p is updated to the offset in state->sg[retval] of the first
+ * Map up to ib_sg->dma_nents elements of state->sg where ib_sg->offset
+ * is the offset where to start in the first element. If ib_sg->offset != 0 then
+ * ib_sg->offset is updated to the offset in state->sg[retval] of the first
* byte that has not yet been mapped.
*/
The above change modifies a correct comment into an incorrect comment. Since
this patch guarantees that &ib_sg->offset != NULL I think the references to
&ib_sg->offset == NULL should be left out.
if (unlikely(n < 0)) {
srp_fr_pool_put(ch->fr_pool, &desc, 1);
pr_debug("%s: ib_map_mr_sg(%d, %d) returned %d.\n",
- dev_name(&req->scmnd->device->sdev_gendev), sg_nents,
- sg_offset_p ? *sg_offset_p : -1, n);
+ dev_name(&req->scmnd->device->sdev_gendev),
+ ib_sg->dma_nents, ib_sg->offset ? : -1, n);
Same mistake here: you are changing sg_offset_p ? *sg_offset_p : -1 into the
following: *sg_offset_p ? *sg_offset_p : -1. These expressions are not
equivalent!
we'll fix debug prints and comments in V2.
thanks.
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 9044356e3bb7..f5d0df8d9fcb 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2157,6 +2157,18 @@ struct ib_counters_read_attr {
u32 flags; /* use enum ib_read_counters_flags */
};
+/*
+ * struct ib_scatterlist - Mapped scatterlist for RDMA operations
+ * @sg: dma mapped sg list
+ * @dma_nents: returned by dma_map_sg
+ * @offset: start offset in bytes into the first sg element
+ */
+struct ib_scatterlist {
+ struct scatterlist *sg;
+ int dma_nents;
+ unsigned int offset;
+};
There is nothing in this data structure that is RDMA specific. Are you sure
the name "ib_scatterlist" is a good choice?
Thanks,
Bart.
The dma_nents are the result we get from ib_dma_map_sg. This function
also may augment the SGL into a 'dma mapped SGL'.
There is no data structure that gathers all this information and since
we still use ib_dma_map_sg and not calling dma_map_sg directly, we can
add it to ib_verbs.h as ib_scatterlist.
