On Mon, Feb 11, 2019 at 09:25:06AM -0600, Shiraz Saleem wrote:
> From: "Shiraz, Saleem" <shiraz.saleem@xxxxxxxxx>
>
> Use the for_each_sg_dma_page iterator variant to walk the umem
> DMA-mapped SGL and get the page DMA address. This avoids the extra
> loop to iterate pages in the SGE when for_each_sg iterator is used.
>
> Additionally, purge umem->page_shift usage in the driver
> as its only relevant for ODP MRs. Use system page size and
> shift instead.
>
> Cc: Faisal Latif <faisal.latif@xxxxxxxxx>
> Signed-off-by: Shiraz, Saleem <shiraz.saleem@xxxxxxxxx>
> drivers/infiniband/hw/nes/nes_verbs.c | 205 +++++++++++++++-------------------
> 1 file changed, 92 insertions(+), 113 deletions(-)
>
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> index 6eb991d..0a766ae 100644
> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> @@ -2109,7 +2109,7 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> struct nes_device *nesdev = nesvnic->nesdev;
> struct nes_adapter *nesadapter = nesdev->nesadapter;
> struct ib_mr *ibmr = ERR_PTR(-EINVAL);
> - struct scatterlist *sg;
> + struct sg_dma_page_iter dma_iter;
> struct nes_ucontext *nes_ucontext;
> struct nes_pbl *nespbl;
> struct nes_mr *nesmr;
> @@ -2117,10 +2117,9 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> struct nes_mem_reg_req req;
> struct nes_vpbl vpbl;
> struct nes_root_vpbl root_vpbl;
> - int entry, page_index;
> + int page_index;
> int page_count = 0;
> int err, pbl_depth = 0;
> - int chunk_pages;
> int ret;
> u32 stag;
> u32 stag_index = 0;
> @@ -2132,7 +2131,6 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> u16 pbl_count;
> u8 single_page = 1;
> u8 stag_key;
> - int first_page = 1;
>
> region = ib_umem_get(udata, start, length, acc, 0);
> if (IS_ERR(region)) {
> @@ -2183,18 +2181,20 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> }
> nesmr->region = region;
>
> - for_each_sg(region->sg_head.sgl, sg, region->nmap, entry) {
> - if (sg_dma_address(sg) & ~PAGE_MASK) {
> + for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
> + struct sg_page_iter *piter = &dma_iter.base;
> +
> + if (sg_dma_address(piter->sg) & ~PAGE_MASK) {
Why add a piter variable?
Isn't this just sg_page_iter_dma_address(&dma_iter); ?
> ib_umem_release(region);
> nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
> nes_debug(NES_DBG_MR, "Unaligned Memory Buffer: 0x%x\n",
> - (unsigned int) sg_dma_address(sg));
> + (unsigned int)sg_dma_address(piter->sg));
> ibmr = ERR_PTR(-EINVAL);
> kfree(nesmr);
> goto reg_user_mr_err;
> }
>
> - if (!sg_dma_len(sg)) {
> + if (!sg_dma_len(piter->sg)) {
It makes no sense to mix sg_dma_len with the iterator. I think this
dma_len can't be zero so this protective if should just be deleted.
> ib_umem_release(region);
> nes_free_resource(nesadapter, nesadapter->allocated_mrs,
> stag_index);
> @@ -2204,103 +2204,94 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> goto reg_user_mr_err;
> }
>
> - region_length += sg_dma_len(sg);
> - chunk_pages = sg_dma_len(sg) >> 12;
> + region_length += PAGE_SIZE;
And this region_length thing not be OK... sg_dma_len is actually
smaller than PAGE_SIZE in some cases - but this only seems fed into
debuging, so let us ignore.
Jason
