Re: [PATCH v2 02/10] IB/iser: Reuse ib_sg_to_pages

[Sagi Grimberg]

On 09/12/2015 21:28, Christoph Hellwig wrote:

On Wed, Dec 09, 2015 at 02:12:00PM +0200, Sagi Grimberg wrote:

We have in iser iser_sg_to_page_vec which has exactly
the same role as ib_sg_to_pages. Customize the page_vec
to hold a fake MR so we can reuse ib_sg_to_pages.


Looks good.  In the long run we should simply kill struct ib_fmr
and make FRMs operate on struct ib_mr so that it can use
ib_sg_to_pages directly.


We can do that. We'd need to add ib_mr a list member just for fmr
routines.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/10] IB/iser: Reuse ib_sg_to_pages

[Christoph Hellwig]

On Thu, Dec 10, 2015 at 10:28:59AM +0200, Sagi Grimberg wrote:
> We can do that. We'd need to add ib_mr a list member just for fmr
> routines.

We'll also need that for a FR pool abstraction that would be very
helpful.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/10] IB/iser: Reuse ib_sg_to_pages

[Sagi Grimberg]

We have in iser iser_sg_to_page_vec which has exactly
the same role as ib_sg_to_pages. Customize the page_vec
to hold a fake MR so we can reuse ib_sg_to_pages.

Signed-off-by: Sagi Grimberg 
---
 drivers/infiniband/ulp/iser/iscsi_iser.h  |   5 +-
 drivers/infiniband/ulp/iser/iser_memory.c | 105 +++---
 2 files changed, 25 insertions(+), 85 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.h 
b/drivers/infiniband/ulp/iser/iscsi_iser.h
index 8a5998e6a407..233ec0c2ae3d 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.h
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.h
@@ -579,9 +579,8 @@ struct iscsi_iser_task {
 
 struct iser_page_vec {
u64 *pages;
-   int length;
-   int offset;
-   int data_size;
+   int npages;
+   struct ib_mr fake_mr;
 };
 
 /**
diff --git a/drivers/infiniband/ulp/iser/iser_memory.c 
b/drivers/infiniband/ulp/iser/iser_memory.c
index ea765fb9664d..b7a2b88f48ce 100644
--- a/drivers/infiniband/ulp/iser/iser_memory.c
+++ b/drivers/infiniband/ulp/iser/iser_memory.c
@@ -131,67 +131,6 @@ iser_reg_desc_put_fmr(struct ib_conn *ib_conn,
 {
 }
 
-#define IS_4K_ALIGNED(addr)unsigned long)addr) & ~MASK_4K) == 0)
-
-/**
- * iser_sg_to_page_vec - Translates scatterlist entries to physical addresses
- * and returns the length of resulting physical address array (may be less than
- * the original due to possible compaction).
- *
- * we build a "page vec" under the assumption that the SG meets the RDMA
- * alignment requirements. Other then the first and last SG elements, all
- * the "internal" elements can be compacted into a list whose elements are
- * dma addresses of physical pages. The code supports also the weird case
- * where --few fragments of the same page-- are present in the SG as
- * consecutive elements. Also, it handles one entry SG.
- */
-
-static int iser_sg_to_page_vec(struct iser_data_buf *data,
-  struct ib_device *ibdev, u64 *pages,
-  int *offset, int *data_size)
-{
-   struct scatterlist *sg, *sgl = data->sg;
-   u64 start_addr, end_addr, page, chunk_start = 0;
-   unsigned long total_sz = 0;
-   unsigned int dma_len;
-   int i, new_chunk, cur_page, last_ent = data->dma_nents - 1;
-
-   /* compute the offset of first element */
-   *offset = (u64) sgl[0].offset & ~MASK_4K;
-
-   new_chunk = 1;
-   cur_page  = 0;
-   for_each_sg(sgl, sg, data->dma_nents, i) {
-   start_addr = ib_sg_dma_address(ibdev, sg);
-   if (new_chunk)
-   chunk_start = start_addr;
-   dma_len = ib_sg_dma_len(ibdev, sg);
-   end_addr = start_addr + dma_len;
-   total_sz += dma_len;
-
-   /* collect page fragments until aligned or end of SG list */
-   if (!IS_4K_ALIGNED(end_addr) && i < last_ent) {
-   new_chunk = 0;
-   continue;
-   }
-   new_chunk = 1;
-
-   /* address of the first page in the contiguous chunk;
-  masking relevant for the very first SG entry,
-  which might be unaligned */
-   page = chunk_start & MASK_4K;
-   do {
-   pages[cur_page++] = page;
-   page += SIZE_4K;
-   } while (page < end_addr);
-   }
-
-   *data_size = total_sz;
-   iser_dbg("page_vec->data_size:%d cur_page %d\n",
-*data_size, cur_page);
-   return cur_page;
-}
-
 static void iser_data_buf_dump(struct iser_data_buf *data,
   struct ib_device *ibdev)
 {
@@ -210,10 +149,10 @@ static void iser_dump_page_vec(struct iser_page_vec 
*page_vec)
 {
int i;
 
-   iser_err("page vec length %d data size %d\n",
-page_vec->length, page_vec->data_size);
-   for (i = 0; i < page_vec->length; i++)
-   iser_err("%d %lx\n",i,(unsigned long)page_vec->pages[i]);
+   iser_err("page vec npages %d data length %d\n",
+page_vec->npages, page_vec->fake_mr.length);
+   for (i = 0; i < page_vec->npages; i++)
+   iser_err("vec[%d]: %llx\n", i, page_vec->pages[i]);
 }
 
 int iser_dma_map_task_data(struct iscsi_iser_task *iser_task,
@@ -262,11 +201,16 @@ iser_reg_dma(struct iser_device *device, struct 
iser_data_buf *mem,
return 0;
 }
 
-/**
- * iser_reg_page_vec - Register physical memory
- *
- * returns: 0 on success, errno code on failure
- */
+static int iser_set_page(struct ib_mr *mr, u64 addr)
+{
+   struct iser_page_vec *page_vec =
+   container_of(mr, struct iser_page_vec, fake_mr);
+
+   page_vec->pages[page_vec->npages++] = addr;
+
+   return 0;
+}
+
 static
 int iser_fast_reg_fmr(struct iscsi_iser_task *iser_task,
  struct iser_data_buf *mem,
@@ -280,22 +224,19 @@ int 

Re: [PATCH v2 02/10] IB/iser: Reuse ib_sg_to_pages

[Christoph Hellwig]

On Wed, Dec 09, 2015 at 02:12:00PM +0200, Sagi Grimberg wrote:
> We have in iser iser_sg_to_page_vec which has exactly
> the same role as ib_sg_to_pages. Customize the page_vec
> to hold a fake MR so we can reuse ib_sg_to_pages.

Looks good.  In the long run we should simply kill struct ib_fmr
and make FRMs operate on struct ib_mr so that it can use
ib_sg_to_pages directly.

Signed-off-by: Christoph Hellwig 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html