[rdma-next,10/12] RDMA/nes: Use for_each_sg_dma_page iterator on umem SGL

Commit Message

Saleem, Shiraz Feb. 11, 2019, 3:25 p.m. UTC

From: "Shiraz, Saleem" <[e-mail]>

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 <[e-mail]>
Signed-off-by: Shiraz, Saleem <[e-mail]>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 205 +++++++++++++++-------------------
 1 file changed, 92 insertions(+), 113 deletions(-)

Comments

Jason Gunthorpe Feb. 11, 2019, 9:51 p.m. UTC | #1

On Mon, Feb 11, 2019 at 09:25:06AM -0600, Shiraz Saleem wrote:
> From: "Shiraz, Saleem" <[e-mail]>
> 
> 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 <[e-mail]>
> Signed-off-by: Shiraz, Saleem <[e-mail]>
>  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

kernel test robot Feb. 12, 2019, 3:16 a.m. UTC | #2

Hi Saleem,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rdma/for-next]
[also build test ERROR on v5.0-rc4 next-20190211]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shiraz-Saleem/Adapt-drivers-to-handle-page-combining-on-umem-SGEs/20190212-104247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: x86_64-randconfig-x005-201906 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/infiniband//hw/nes/nes_verbs.c: In function 'nes_reg_user_mr':
>> drivers/infiniband//hw/nes/nes_verbs.c:2101:26: error: storage size of 'dma_iter' isn't known
     struct sg_dma_page_iter dma_iter;
                             ^~~~~~~~
>> drivers/infiniband//hw/nes/nes_verbs.c:2173:4: error: implicit declaration of function 'for_each_sg_dma_page'; did you mean 'for_each_sg_page'? [-Werror=implicit-function-declaration]
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
       ^~~~~~~~~~~~~~~~~~~~
       for_each_sg_page
>> drivers/infiniband//hw/nes/nes_verbs.c:2173:73: error: expected ';' before '{' token
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
                                                                            ^~
                                                                            ;
   drivers/infiniband//hw/nes/nes_verbs.c:2396:73: error: expected ';' before '{' token
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
                                                                            ^~
                                                                            ;
   drivers/infiniband//hw/nes/nes_verbs.c:2328:4: warning: label 'reg_user_mr_err' defined but not used [-Wunused-label]
       reg_user_mr_err:
       ^~~~~~~~~~~~~~~
   drivers/infiniband//hw/nes/nes_verbs.c:2110:6: warning: unused variable 'page_count' [-Wunused-variable]
     int page_count = 0;
         ^~~~~~~~~~
   drivers/infiniband//hw/nes/nes_verbs.c:2101:26: warning: unused variable 'dma_iter' [-Wunused-variable]
     struct sg_dma_page_iter dma_iter;
                             ^~~~~~~~
   drivers/infiniband//hw/nes/nes_verbs.c:2094:13: warning: unused variable 'last_dma_addr' [-Wunused-variable]
     dma_addr_t last_dma_addr = 0;
                ^~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +2101 drivers/infiniband//hw/nes/nes_verbs.c

  2084	
  2085	/**
  2086	 * nes_reg_user_mr
  2087	 */
  2088	static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
  2089			u64 virt, int acc, struct ib_udata *udata)
  2090	{
  2091		u64 iova_start;
  2092		__le64 *pbl;
  2093		u64 region_length;
  2094		dma_addr_t last_dma_addr = 0;
  2095		dma_addr_t first_dma_addr = 0;
  2096		struct nes_pd *nespd = to_nespd(pd);
  2097		struct nes_vnic *nesvnic = to_nesvnic(pd->device);
  2098		struct nes_device *nesdev = nesvnic->nesdev;
  2099		struct nes_adapter *nesadapter = nesdev->nesadapter;
  2100		struct ib_mr *ibmr = ERR_PTR(-EINVAL);
> 2101		struct sg_dma_page_iter dma_iter;
  2102		struct nes_ucontext *nes_ucontext;
  2103		struct nes_pbl *nespbl;
  2104		struct nes_mr *nesmr;
  2105		struct ib_umem *region;
  2106		struct nes_mem_reg_req req;
  2107		struct nes_vpbl vpbl;
  2108		struct nes_root_vpbl root_vpbl;
  2109		int page_index;
  2110		int page_count = 0;
  2111		int err, pbl_depth = 0;
  2112		int ret;
  2113		u32 stag;
  2114		u32 stag_index = 0;
  2115		u32 next_stag_index;
  2116		u32 driver_key;
  2117		u32 root_pbl_index = 0;
  2118		u32 cur_pbl_index = 0;
  2119		u32 skip_pages;
  2120		u16 pbl_count;
  2121		u8 single_page = 1;
  2122		u8 stag_key;
  2123	
  2124		region = ib_umem_get(udata, start, length, acc, 0);
  2125		if (IS_ERR(region)) {
  2126			return (struct ib_mr *)region;
  2127		}
  2128	
  2129		nes_debug(NES_DBG_MR, "User base = 0x%lX, Virt base = 0x%lX, length = %u,"
  2130				" offset = %u, page size = %lu.\n",
  2131				(unsigned long int)start, (unsigned long int)virt, (u32)length,
  2132				ib_umem_offset(region), BIT(region->page_shift));
  2133	
  2134		skip_pages = ((u32)ib_umem_offset(region)) >> 12;
  2135	
  2136		if (ib_copy_from_udata(&req, udata, sizeof(req))) {
  2137			ib_umem_release(region);
  2138			return ERR_PTR(-EFAULT);
  2139		}
  2140		nes_debug(NES_DBG_MR, "Memory Registration type = %08X.\n", req.reg_type);
  2141	
  2142		switch (req.reg_type) {
  2143			case IWNES_MEMREG_TYPE_MEM:
  2144				pbl_depth = 0;
  2145				region_length = 0;
  2146				vpbl.pbl_vbase = NULL;
  2147				root_vpbl.pbl_vbase = NULL;
  2148				root_vpbl.pbl_pbase = 0;
  2149	
  2150				get_random_bytes(&next_stag_index, sizeof(next_stag_index));
  2151				stag_key = (u8)next_stag_index;
  2152	
  2153				driver_key = next_stag_index & 0x70000000;
  2154	
  2155				next_stag_index >>= 8;
  2156				next_stag_index %= nesadapter->max_mr;
  2157	
  2158				err = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs,
  2159						nesadapter->max_mr, &stag_index, &next_stag_index, NES_RESOURCE_USER_MR);
  2160				if (err) {
  2161					ib_umem_release(region);
  2162					return ERR_PTR(err);
  2163				}
  2164	
  2165				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
  2166				if (!nesmr) {
  2167					ib_umem_release(region);
  2168					nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2169					return ERR_PTR(-ENOMEM);
  2170				}
  2171				nesmr->region = region;
  2172	
> 2173				for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
  2174					struct sg_page_iter *piter = &dma_iter.base;
  2175	
  2176					if (sg_dma_address(piter->sg) & ~PAGE_MASK) {
  2177						ib_umem_release(region);
  2178						nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2179						nes_debug(NES_DBG_MR, "Unaligned Memory Buffer: 0x%x\n",
  2180							  (unsigned int)sg_dma_address(piter->sg));
  2181						ibmr = ERR_PTR(-EINVAL);
  2182						kfree(nesmr);
  2183						goto reg_user_mr_err;
  2184					}
  2185	
  2186					if (!sg_dma_len(piter->sg)) {
  2187						ib_umem_release(region);
  2188						nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2189								  stag_index);
  2190						nes_debug(NES_DBG_MR, "Invalid Buffer Size\n");
  2191						ibmr = ERR_PTR(-EINVAL);
  2192						kfree(nesmr);
  2193						goto reg_user_mr_err;
  2194					}
  2195	
  2196					region_length += PAGE_SIZE;
  2197					region_length -= skip_pages << 12;
  2198					skip_pages = 0;
  2199					if ((page_count != 0) && (page_count << 12) - (ib_umem_offset(region) & (4096 - 1)) >= region->length)
  2200						goto enough_pages;
  2201					if ((page_count & 0x01FF) == 0) {
  2202						if (page_count >= 1024 * 512) {
  2203							ib_umem_release(region);
  2204							nes_free_resource(nesadapter,
  2205									  nesadapter->allocated_mrs, stag_index);
  2206							kfree(nesmr);
  2207							ibmr = ERR_PTR(-E2BIG);
  2208							goto reg_user_mr_err;
  2209						}
  2210						if (root_pbl_index == 1) {
  2211							root_vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev,
  2212									8192, &root_vpbl.pbl_pbase);
  2213							nes_debug(NES_DBG_MR, "Allocating root PBL, va = %p, pa = 0x%08X\n",
  2214								  root_vpbl.pbl_vbase, (unsigned int)root_vpbl.pbl_pbase);
  2215							if (!root_vpbl.pbl_vbase) {
  2216								ib_umem_release(region);
  2217								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2218										    vpbl.pbl_pbase);
  2219								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2220										  stag_index);
  2221								kfree(nesmr);
  2222								ibmr = ERR_PTR(-ENOMEM);
  2223								goto reg_user_mr_err;
  2224							}
  2225							root_vpbl.leaf_vpbl = kcalloc(1024,
  2226										      sizeof(*root_vpbl.leaf_vpbl),
  2227										      GFP_KERNEL);
  2228							if (!root_vpbl.leaf_vpbl) {
  2229								ib_umem_release(region);
  2230								pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
  2231										    root_vpbl.pbl_pbase);
  2232								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2233										    vpbl.pbl_pbase);
  2234								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2235										  stag_index);
  2236								kfree(nesmr);
  2237								ibmr = ERR_PTR(-ENOMEM);
  2238								goto reg_user_mr_err;
  2239							}
  2240							root_vpbl.pbl_vbase[0].pa_low =
  2241									cpu_to_le32((u32)vpbl.pbl_pbase);
  2242							root_vpbl.pbl_vbase[0].pa_high =
  2243									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
  2244							root_vpbl.leaf_vpbl[0] = vpbl;
  2245						}
  2246						vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev, 4096,
  2247								&vpbl.pbl_pbase);
  2248						nes_debug(NES_DBG_MR, "Allocating leaf PBL, va = %p, pa = 0x%08X\n",
  2249							  vpbl.pbl_vbase, (unsigned int)vpbl.pbl_pbase);
  2250						if (!vpbl.pbl_vbase) {
  2251							ib_umem_release(region);
  2252							nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2253							ibmr = ERR_PTR(-ENOMEM);
  2254							kfree(nesmr);
  2255							goto reg_user_mr_err;
  2256						}
  2257						if (1 <= root_pbl_index) {
  2258							root_vpbl.pbl_vbase[root_pbl_index].pa_low =
  2259									cpu_to_le32((u32)vpbl.pbl_pbase);
  2260							root_vpbl.pbl_vbase[root_pbl_index].pa_high =
  2261									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
  2262							root_vpbl.leaf_vpbl[root_pbl_index] = vpbl;
  2263						}
  2264						root_pbl_index++;
  2265						cur_pbl_index = 0;
  2266					}
  2267					if (single_page) {
  2268						if (page_count != 0) {
  2269							if ((last_dma_addr + 4096) != sg_page_iter_dma_address(&dma_iter))
  2270								single_page = 0;
  2271							last_dma_addr = sg_page_iter_dma_address(&dma_iter);
  2272						} else {
  2273							first_dma_addr = sg_page_iter_dma_address(&dma_iter);
  2274							last_dma_addr = first_dma_addr;
  2275						}
  2276					}
  2277	
  2278					vpbl.pbl_vbase[cur_pbl_index].pa_low =
  2279							cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
  2280					vpbl.pbl_vbase[cur_pbl_index].pa_high =
  2281							cpu_to_le32((u32)((u64)(sg_page_iter_dma_address(&dma_iter))));
  2282					cur_pbl_index++;
  2283					page_count++;
  2284				}
  2285	
  2286				enough_pages:
  2287				nes_debug(NES_DBG_MR, "calculating stag, stag_index=0x%08x, driver_key=0x%08x,"
  2288						" stag_key=0x%08x\n",
  2289						stag_index, driver_key, stag_key);
  2290				stag = stag_index << 8;
  2291				stag |= driver_key;
  2292				stag += (u32)stag_key;
  2293	
  2294				iova_start = virt;
  2295				/* Make the leaf PBL the root if only one PBL */
  2296				if (root_pbl_index == 1) {
  2297					root_vpbl.pbl_pbase = vpbl.pbl_pbase;
  2298				}
  2299	
  2300				if (single_page) {
  2301					pbl_count = 0;
  2302				} else {
  2303					pbl_count = root_pbl_index;
  2304					first_dma_addr = 0;
  2305				}
  2306				nes_debug(NES_DBG_MR, "Registering STag 0x%08X, VA = 0x%08X, length = 0x%08X,"
  2307						" index = 0x%08X, region->length=0x%08llx, pbl_count = %u\n",
  2308						stag, (unsigned int)iova_start,
  2309						(unsigned int)region_length, stag_index,
  2310						(unsigned long long)region->length, pbl_count);
  2311				ret = nes_reg_mr(nesdev, nespd, stag, region->length, &root_vpbl,
  2312						 first_dma_addr, pbl_count, (u16)cur_pbl_index, acc,
  2313						 &iova_start, &nesmr->pbls_used, &nesmr->pbl_4k);
  2314	
  2315				nes_debug(NES_DBG_MR, "ret=%d\n", ret);
  2316	
  2317				if (ret == 0) {
  2318					nesmr->ibmr.rkey = stag;
  2319					nesmr->ibmr.lkey = stag;
  2320					nesmr->mode = IWNES_MEMREG_TYPE_MEM;
  2321					ibmr = &nesmr->ibmr;
  2322				} else {
  2323					ib_umem_release(region);
  2324					kfree(nesmr);
  2325					ibmr = ERR_PTR(-ENOMEM);
  2326				}
  2327	
  2328				reg_user_mr_err:
  2329				/* free the resources */
  2330				if (root_pbl_index == 1) {
  2331					pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2332							vpbl.pbl_pbase);
  2333				} else {
  2334					for (page_index=0; page_index<root_pbl_index; page_index++) {
  2335						pci_free_consistent(nesdev->pcidev, 4096,
  2336								root_vpbl.leaf_vpbl[page_index].pbl_vbase,
  2337								root_vpbl.leaf_vpbl[page_index].pbl_pbase);
  2338					}
  2339					kfree(root_vpbl.leaf_vpbl);
  2340					pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
  2341							root_vpbl.pbl_pbase);
  2342				}
  2343	
  2344				nes_debug(NES_DBG_MR, "Leaving, ibmr=%p", ibmr);
  2345	
  2346				return ibmr;
  2347			case IWNES_MEMREG_TYPE_QP:
  2348			case IWNES_MEMREG_TYPE_CQ:
  2349				if (!region->length) {
  2350					nes_debug(NES_DBG_MR, "Unable to register zero length region for CQ\n");
  2351					ib_umem_release(region);
  2352					return ERR_PTR(-EINVAL);
  2353				}
  2354				nespbl = kzalloc(sizeof(*nespbl), GFP_KERNEL);
  2355				if (!nespbl) {
  2356					ib_umem_release(region);
  2357					return ERR_PTR(-ENOMEM);
  2358				}
  2359				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
  2360				if (!nesmr) {
  2361					ib_umem_release(region);
  2362					kfree(nespbl);
  2363					return ERR_PTR(-ENOMEM);
  2364				}
  2365				nesmr->region = region;
  2366				nes_ucontext = to_nesucontext(pd->uobject->context);
  2367				pbl_depth = region->length >> 12;
  2368				pbl_depth += (region->length & (4096-1)) ? 1 : 0;
  2369				nespbl->pbl_size = pbl_depth*sizeof(u64);
  2370				if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
  2371					nes_debug(NES_DBG_MR, "Attempting to allocate QP PBL memory");
  2372				} else {
  2373					nes_debug(NES_DBG_MR, "Attempting to allocate CP PBL memory");
  2374				}
  2375	
  2376				nes_debug(NES_DBG_MR, " %u bytes, %u entries.\n",
  2377						nespbl->pbl_size, pbl_depth);
  2378				pbl = pci_alloc_consistent(nesdev->pcidev, nespbl->pbl_size,
  2379						&nespbl->pbl_pbase);
  2380				if (!pbl) {
  2381					ib_umem_release(region);
  2382					kfree(nesmr);
  2383					kfree(nespbl);
  2384					nes_debug(NES_DBG_MR, "Unable to allocate PBL memory\n");
  2385					return ERR_PTR(-ENOMEM);
  2386				}
  2387	
  2388				nespbl->pbl_vbase = (u64 *)pbl;
  2389				nespbl->user_base = start;
  2390				nes_debug(NES_DBG_MR, "Allocated PBL memory, %u bytes, pbl_pbase=%lx,"
  2391						" pbl_vbase=%p user_base=0x%lx\n",
  2392					  nespbl->pbl_size, (unsigned long) nespbl->pbl_pbase,
  2393					  (void *) nespbl->pbl_vbase, nespbl->user_base);
  2394	
  2395				nespbl->page = sg_page(region->sg_head.sgl);
  2396				for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
  2397					((__le32 *)pbl)[0] = cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
  2398					((__le32 *)pbl)[1] = cpu_to_le32(((u64)(sg_page_iter_dma_address(&dma_iter)))>>32);
  2399					nes_debug(NES_DBG_MR, "pbl=%p, *pbl=0x%016llx, 0x%08x%08x\n", pbl,
  2400						  (unsigned long long)*pbl,
  2401						  le32_to_cpu(((__le32 *)pbl)[1]), le32_to_cpu(((__le32 *)pbl)[0]));
  2402					pbl++;
  2403				}
  2404	
  2405				if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
  2406					list_add_tail(&nespbl->list, &nes_ucontext->qp_reg_mem_list);
  2407				} else {
  2408					list_add_tail(&nespbl->list, &nes_ucontext->cq_reg_mem_list);
  2409				}
  2410				nesmr->ibmr.rkey = -1;
  2411				nesmr->ibmr.lkey = -1;
  2412				nesmr->mode = req.reg_type;
  2413				return &nesmr->ibmr;
  2414		}
  2415	
  2416		ib_umem_release(region);
  2417		return ERR_PTR(-ENOSYS);
  2418	}
  2419	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

kernel test robot Feb. 12, 2019, 11:53 a.m. UTC | #3

Hi Saleem,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on rdma/for-next]
[also build test ERROR on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shiraz-Saleem/Adapt-drivers-to-handle-page-combining-on-umem-SGEs/20190212-104247
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
config: i386-randconfig-a3-201906 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/infiniband/hw/nes/nes_verbs.c: In function 'nes_reg_user_mr':
   drivers/infiniband/hw/nes/nes_verbs.c:2101:26: error: storage size of 'dma_iter' isn't known
     struct sg_dma_page_iter dma_iter;
                             ^
>> drivers/infiniband/hw/nes/nes_verbs.c:2173:4: error: implicit declaration of function 'for_each_sg_dma_page' [-Werror=implicit-function-declaration]
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
       ^
   drivers/infiniband/hw/nes/nes_verbs.c:2173:74: error: expected ';' before '{' token
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
                                                                             ^
   drivers/infiniband/hw/nes/nes_verbs.c:2396:74: error: expected ';' before '{' token
       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
                                                                             ^
   drivers/infiniband/hw/nes/nes_verbs.c:2328:4: warning: label 'reg_user_mr_err' defined but not used [-Wunused-label]
       reg_user_mr_err:
       ^
   drivers/infiniband/hw/nes/nes_verbs.c:2110:6: warning: unused variable 'page_count' [-Wunused-variable]
     int page_count = 0;
         ^
   drivers/infiniband/hw/nes/nes_verbs.c:2101:26: warning: unused variable 'dma_iter' [-Wunused-variable]
     struct sg_dma_page_iter dma_iter;
                             ^
   drivers/infiniband/hw/nes/nes_verbs.c:2094:13: warning: unused variable 'last_dma_addr' [-Wunused-variable]
     dma_addr_t last_dma_addr = 0;
                ^
   cc1: some warnings being treated as errors

vim +/for_each_sg_dma_page +2173 drivers/infiniband/hw/nes/nes_verbs.c

  2084	
  2085	/**
  2086	 * nes_reg_user_mr
  2087	 */
  2088	static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
  2089			u64 virt, int acc, struct ib_udata *udata)
  2090	{
  2091		u64 iova_start;
  2092		__le64 *pbl;
  2093		u64 region_length;
  2094		dma_addr_t last_dma_addr = 0;
  2095		dma_addr_t first_dma_addr = 0;
  2096		struct nes_pd *nespd = to_nespd(pd);
  2097		struct nes_vnic *nesvnic = to_nesvnic(pd->device);
  2098		struct nes_device *nesdev = nesvnic->nesdev;
  2099		struct nes_adapter *nesadapter = nesdev->nesadapter;
  2100		struct ib_mr *ibmr = ERR_PTR(-EINVAL);
> 2101		struct sg_dma_page_iter dma_iter;
  2102		struct nes_ucontext *nes_ucontext;
  2103		struct nes_pbl *nespbl;
  2104		struct nes_mr *nesmr;
  2105		struct ib_umem *region;
  2106		struct nes_mem_reg_req req;
  2107		struct nes_vpbl vpbl;
  2108		struct nes_root_vpbl root_vpbl;
  2109		int page_index;
  2110		int page_count = 0;
  2111		int err, pbl_depth = 0;
  2112		int ret;
  2113		u32 stag;
  2114		u32 stag_index = 0;
  2115		u32 next_stag_index;
  2116		u32 driver_key;
  2117		u32 root_pbl_index = 0;
  2118		u32 cur_pbl_index = 0;
  2119		u32 skip_pages;
  2120		u16 pbl_count;
  2121		u8 single_page = 1;
  2122		u8 stag_key;
  2123	
  2124		region = ib_umem_get(udata, start, length, acc, 0);
  2125		if (IS_ERR(region)) {
  2126			return (struct ib_mr *)region;
  2127		}
  2128	
  2129		nes_debug(NES_DBG_MR, "User base = 0x%lX, Virt base = 0x%lX, length = %u,"
  2130				" offset = %u, page size = %lu.\n",
  2131				(unsigned long int)start, (unsigned long int)virt, (u32)length,
  2132				ib_umem_offset(region), BIT(region->page_shift));
  2133	
  2134		skip_pages = ((u32)ib_umem_offset(region)) >> 12;
  2135	
  2136		if (ib_copy_from_udata(&req, udata, sizeof(req))) {
  2137			ib_umem_release(region);
  2138			return ERR_PTR(-EFAULT);
  2139		}
  2140		nes_debug(NES_DBG_MR, "Memory Registration type = %08X.\n", req.reg_type);
  2141	
  2142		switch (req.reg_type) {
  2143			case IWNES_MEMREG_TYPE_MEM:
  2144				pbl_depth = 0;
  2145				region_length = 0;
  2146				vpbl.pbl_vbase = NULL;
  2147				root_vpbl.pbl_vbase = NULL;
  2148				root_vpbl.pbl_pbase = 0;
  2149	
  2150				get_random_bytes(&next_stag_index, sizeof(next_stag_index));
  2151				stag_key = (u8)next_stag_index;
  2152	
  2153				driver_key = next_stag_index & 0x70000000;
  2154	
  2155				next_stag_index >>= 8;
  2156				next_stag_index %= nesadapter->max_mr;
  2157	
  2158				err = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs,
  2159						nesadapter->max_mr, &stag_index, &next_stag_index, NES_RESOURCE_USER_MR);
  2160				if (err) {
  2161					ib_umem_release(region);
  2162					return ERR_PTR(err);
  2163				}
  2164	
  2165				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
  2166				if (!nesmr) {
  2167					ib_umem_release(region);
  2168					nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2169					return ERR_PTR(-ENOMEM);
  2170				}
  2171				nesmr->region = region;
  2172	
> 2173				for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
  2174					struct sg_page_iter *piter = &dma_iter.base;
  2175	
  2176					if (sg_dma_address(piter->sg) & ~PAGE_MASK) {
  2177						ib_umem_release(region);
  2178						nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2179						nes_debug(NES_DBG_MR, "Unaligned Memory Buffer: 0x%x\n",
  2180							  (unsigned int)sg_dma_address(piter->sg));
  2181						ibmr = ERR_PTR(-EINVAL);
  2182						kfree(nesmr);
  2183						goto reg_user_mr_err;
  2184					}
  2185	
  2186					if (!sg_dma_len(piter->sg)) {
  2187						ib_umem_release(region);
  2188						nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2189								  stag_index);
  2190						nes_debug(NES_DBG_MR, "Invalid Buffer Size\n");
  2191						ibmr = ERR_PTR(-EINVAL);
  2192						kfree(nesmr);
  2193						goto reg_user_mr_err;
  2194					}
  2195	
  2196					region_length += PAGE_SIZE;
  2197					region_length -= skip_pages << 12;
  2198					skip_pages = 0;
  2199					if ((page_count != 0) && (page_count << 12) - (ib_umem_offset(region) & (4096 - 1)) >= region->length)
  2200						goto enough_pages;
  2201					if ((page_count & 0x01FF) == 0) {
  2202						if (page_count >= 1024 * 512) {
  2203							ib_umem_release(region);
  2204							nes_free_resource(nesadapter,
  2205									  nesadapter->allocated_mrs, stag_index);
  2206							kfree(nesmr);
  2207							ibmr = ERR_PTR(-E2BIG);
  2208							goto reg_user_mr_err;
  2209						}
  2210						if (root_pbl_index == 1) {
  2211							root_vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev,
  2212									8192, &root_vpbl.pbl_pbase);
  2213							nes_debug(NES_DBG_MR, "Allocating root PBL, va = %p, pa = 0x%08X\n",
  2214								  root_vpbl.pbl_vbase, (unsigned int)root_vpbl.pbl_pbase);
  2215							if (!root_vpbl.pbl_vbase) {
  2216								ib_umem_release(region);
  2217								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2218										    vpbl.pbl_pbase);
  2219								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2220										  stag_index);
  2221								kfree(nesmr);
  2222								ibmr = ERR_PTR(-ENOMEM);
  2223								goto reg_user_mr_err;
  2224							}
  2225							root_vpbl.leaf_vpbl = kcalloc(1024,
  2226										      sizeof(*root_vpbl.leaf_vpbl),
  2227										      GFP_KERNEL);
  2228							if (!root_vpbl.leaf_vpbl) {
  2229								ib_umem_release(region);
  2230								pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
  2231										    root_vpbl.pbl_pbase);
  2232								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2233										    vpbl.pbl_pbase);
  2234								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
  2235										  stag_index);
  2236								kfree(nesmr);
  2237								ibmr = ERR_PTR(-ENOMEM);
  2238								goto reg_user_mr_err;
  2239							}
  2240							root_vpbl.pbl_vbase[0].pa_low =
  2241									cpu_to_le32((u32)vpbl.pbl_pbase);
  2242							root_vpbl.pbl_vbase[0].pa_high =
  2243									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
  2244							root_vpbl.leaf_vpbl[0] = vpbl;
  2245						}
  2246						vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev, 4096,
  2247								&vpbl.pbl_pbase);
  2248						nes_debug(NES_DBG_MR, "Allocating leaf PBL, va = %p, pa = 0x%08X\n",
  2249							  vpbl.pbl_vbase, (unsigned int)vpbl.pbl_pbase);
  2250						if (!vpbl.pbl_vbase) {
  2251							ib_umem_release(region);
  2252							nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
  2253							ibmr = ERR_PTR(-ENOMEM);
  2254							kfree(nesmr);
  2255							goto reg_user_mr_err;
  2256						}
  2257						if (1 <= root_pbl_index) {
  2258							root_vpbl.pbl_vbase[root_pbl_index].pa_low =
  2259									cpu_to_le32((u32)vpbl.pbl_pbase);
  2260							root_vpbl.pbl_vbase[root_pbl_index].pa_high =
  2261									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
  2262							root_vpbl.leaf_vpbl[root_pbl_index] = vpbl;
  2263						}
  2264						root_pbl_index++;
  2265						cur_pbl_index = 0;
  2266					}
  2267					if (single_page) {
  2268						if (page_count != 0) {
  2269							if ((last_dma_addr + 4096) != sg_page_iter_dma_address(&dma_iter))
  2270								single_page = 0;
  2271							last_dma_addr = sg_page_iter_dma_address(&dma_iter);
  2272						} else {
  2273							first_dma_addr = sg_page_iter_dma_address(&dma_iter);
  2274							last_dma_addr = first_dma_addr;
  2275						}
  2276					}
  2277	
  2278					vpbl.pbl_vbase[cur_pbl_index].pa_low =
  2279							cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
  2280					vpbl.pbl_vbase[cur_pbl_index].pa_high =
  2281							cpu_to_le32((u32)((u64)(sg_page_iter_dma_address(&dma_iter))));
  2282					cur_pbl_index++;
  2283					page_count++;
  2284				}
  2285	
  2286				enough_pages:
  2287				nes_debug(NES_DBG_MR, "calculating stag, stag_index=0x%08x, driver_key=0x%08x,"
  2288						" stag_key=0x%08x\n",
  2289						stag_index, driver_key, stag_key);
  2290				stag = stag_index << 8;
  2291				stag |= driver_key;
  2292				stag += (u32)stag_key;
  2293	
  2294				iova_start = virt;
  2295				/* Make the leaf PBL the root if only one PBL */
  2296				if (root_pbl_index == 1) {
  2297					root_vpbl.pbl_pbase = vpbl.pbl_pbase;
  2298				}
  2299	
  2300				if (single_page) {
  2301					pbl_count = 0;
  2302				} else {
  2303					pbl_count = root_pbl_index;
  2304					first_dma_addr = 0;
  2305				}
  2306				nes_debug(NES_DBG_MR, "Registering STag 0x%08X, VA = 0x%08X, length = 0x%08X,"
  2307						" index = 0x%08X, region->length=0x%08llx, pbl_count = %u\n",
  2308						stag, (unsigned int)iova_start,
  2309						(unsigned int)region_length, stag_index,
  2310						(unsigned long long)region->length, pbl_count);
  2311				ret = nes_reg_mr(nesdev, nespd, stag, region->length, &root_vpbl,
  2312						 first_dma_addr, pbl_count, (u16)cur_pbl_index, acc,
  2313						 &iova_start, &nesmr->pbls_used, &nesmr->pbl_4k);
  2314	
  2315				nes_debug(NES_DBG_MR, "ret=%d\n", ret);
  2316	
  2317				if (ret == 0) {
  2318					nesmr->ibmr.rkey = stag;
  2319					nesmr->ibmr.lkey = stag;
  2320					nesmr->mode = IWNES_MEMREG_TYPE_MEM;
  2321					ibmr = &nesmr->ibmr;
  2322				} else {
  2323					ib_umem_release(region);
  2324					kfree(nesmr);
  2325					ibmr = ERR_PTR(-ENOMEM);
  2326				}
  2327	
  2328				reg_user_mr_err:
  2329				/* free the resources */
  2330				if (root_pbl_index == 1) {
  2331					pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
  2332							vpbl.pbl_pbase);
  2333				} else {
  2334					for (page_index=0; page_index<root_pbl_index; page_index++) {
  2335						pci_free_consistent(nesdev->pcidev, 4096,
  2336								root_vpbl.leaf_vpbl[page_index].pbl_vbase,
  2337								root_vpbl.leaf_vpbl[page_index].pbl_pbase);
  2338					}
  2339					kfree(root_vpbl.leaf_vpbl);
  2340					pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
  2341							root_vpbl.pbl_pbase);
  2342				}
  2343	
  2344				nes_debug(NES_DBG_MR, "Leaving, ibmr=%p", ibmr);
  2345	
  2346				return ibmr;
  2347			case IWNES_MEMREG_TYPE_QP:
  2348			case IWNES_MEMREG_TYPE_CQ:
  2349				if (!region->length) {
  2350					nes_debug(NES_DBG_MR, "Unable to register zero length region for CQ\n");
  2351					ib_umem_release(region);
  2352					return ERR_PTR(-EINVAL);
  2353				}
  2354				nespbl = kzalloc(sizeof(*nespbl), GFP_KERNEL);
  2355				if (!nespbl) {
  2356					ib_umem_release(region);
  2357					return ERR_PTR(-ENOMEM);
  2358				}
  2359				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
  2360				if (!nesmr) {
  2361					ib_umem_release(region);
  2362					kfree(nespbl);
  2363					return ERR_PTR(-ENOMEM);
  2364				}
  2365				nesmr->region = region;
  2366				nes_ucontext = to_nesucontext(pd->uobject->context);
  2367				pbl_depth = region->length >> 12;
  2368				pbl_depth += (region->length & (4096-1)) ? 1 : 0;
  2369				nespbl->pbl_size = pbl_depth*sizeof(u64);
  2370				if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
  2371					nes_debug(NES_DBG_MR, "Attempting to allocate QP PBL memory");
  2372				} else {
  2373					nes_debug(NES_DBG_MR, "Attempting to allocate CP PBL memory");
  2374				}
  2375	
  2376				nes_debug(NES_DBG_MR, " %u bytes, %u entries.\n",
  2377						nespbl->pbl_size, pbl_depth);
  2378				pbl = pci_alloc_consistent(nesdev->pcidev, nespbl->pbl_size,
  2379						&nespbl->pbl_pbase);
  2380				if (!pbl) {
  2381					ib_umem_release(region);
  2382					kfree(nesmr);
  2383					kfree(nespbl);
  2384					nes_debug(NES_DBG_MR, "Unable to allocate PBL memory\n");
  2385					return ERR_PTR(-ENOMEM);
  2386				}
  2387	
  2388				nespbl->pbl_vbase = (u64 *)pbl;
  2389				nespbl->user_base = start;
  2390				nes_debug(NES_DBG_MR, "Allocated PBL memory, %u bytes, pbl_pbase=%lx,"
  2391						" pbl_vbase=%p user_base=0x%lx\n",
  2392					  nespbl->pbl_size, (unsigned long) nespbl->pbl_pbase,
  2393					  (void *) nespbl->pbl_vbase, nespbl->user_base);
  2394	
  2395				nespbl->page = sg_page(region->sg_head.sgl);
  2396				for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
  2397					((__le32 *)pbl)[0] = cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
  2398					((__le32 *)pbl)[1] = cpu_to_le32(((u64)(sg_page_iter_dma_address(&dma_iter)))>>32);
  2399					nes_debug(NES_DBG_MR, "pbl=%p, *pbl=0x%016llx, 0x%08x%08x\n", pbl,
  2400						  (unsigned long long)*pbl,
  2401						  le32_to_cpu(((__le32 *)pbl)[1]), le32_to_cpu(((__le32 *)pbl)[0]));
  2402					pbl++;
  2403				}
  2404	
  2405				if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
  2406					list_add_tail(&nespbl->list, &nes_ucontext->qp_reg_mem_list);
  2407				} else {
  2408					list_add_tail(&nespbl->list, &nes_ucontext->cq_reg_mem_list);
  2409				}
  2410				nesmr->ibmr.rkey = -1;
  2411				nesmr->ibmr.lkey = -1;
  2412				nesmr->mode = req.reg_type;
  2413				return &nesmr->ibmr;
  2414		}
  2415	
  2416		ib_umem_release(region);
  2417		return ERR_PTR(-ENOSYS);
  2418	}
  2419	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Saleem, Shiraz Feb. 12, 2019, 4:41 p.m. UTC | #4

>Subject: Re: [rdma-next 10/12] RDMA/nes: Use for_each_sg_dma_page iterator
>on umem SGL
>
>On Mon, Feb 11, 2019 at 09:25:06AM -0600, Shiraz Saleem wrote:
>> From: "Shiraz, Saleem" <[e-mail]>
>>
>> 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.
>>
[.....]

>> -			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); ?

In retrospect, I think so.

I was trying to mirror what the original code was doing which is
checking if the dma addr of the sge is page aligned and piter variable is used
to get the sg holding the page.  But the original code assumed a 1:1 mapping
between sge and system page in umem sgl. So probably was really checking
if the dma addr of pages in the SGL are page aligned.
>
>
>>  					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.

Makes sense.

>>  					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.

OK.

Saleem, Shiraz Feb. 12, 2019, 4:41 p.m. UTC | #5

>Subject: Re: [rdma-next 10/12] RDMA/nes: Use for_each_sg_dma_page iterator
>on umem SGL
>
>Hi Saleem,
>
>Thank you for the patch! Yet something to improve:
>
>[auto build test ERROR on rdma/for-next] [also build test ERROR on v5.0-rc4
>next-20190211] [if your patch is applied to the wrong git tree, please drop us a
>note to help improve the system]
>
>url:    https://github.com/0day-ci/linux/commits/Shiraz-Saleem/Adapt-drivers-to-
>handle-page-combining-on-umem-SGEs/20190212-104247
>base:   https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git for-next
>config: x86_64-randconfig-x005-201906 (attached as .config)
>compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
>reproduce:
>        # save the attached .config to linux build tree
>        make ARCH=x86_64
>
>All errors (new ones prefixed by >>):
>
>   drivers/infiniband//hw/nes/nes_verbs.c: In function 'nes_reg_user_mr':
>>> drivers/infiniband//hw/nes/nes_verbs.c:2101:26: error: storage size
>>> of 'dma_iter' isn't known
>     struct sg_dma_page_iter dma_iter;
>                             ^~~~~~~~

Jason - Perhaps this patch was applied to the branch without its dependent patch?
https://www.spinics.net/lists/linux-rdma/msg75195.html?


>>> drivers/infiniband//hw/nes/nes_verbs.c:2173:4: error: implicit
>>> declaration of function 'for_each_sg_dma_page'; did you mean
>>> 'for_each_sg_page'? [-Werror=implicit-function-declaration]
>       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
>       ^~~~~~~~~~~~~~~~~~~~
>       for_each_sg_page
>>> drivers/infiniband//hw/nes/nes_verbs.c:2173:73: error: expected ';'
>>> before '{' token
>       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
>                                                                            ^~
>                                                                            ;
>   drivers/infiniband//hw/nes/nes_verbs.c:2396:73: error: expected ';' before '{'
>token
>       for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
>                                                                            ^~
>                                                                            ;
>   drivers/infiniband//hw/nes/nes_verbs.c:2328:4: warning: label 'reg_user_mr_err'
>defined but not used [-Wunused-label]
>       reg_user_mr_err:
>       ^~~~~~~~~~~~~~~
>   drivers/infiniband//hw/nes/nes_verbs.c:2110:6: warning: unused variable
>'page_count' [-Wunused-variable]
>     int page_count = 0;
>         ^~~~~~~~~~
>   drivers/infiniband//hw/nes/nes_verbs.c:2101:26: warning: unused variable
>'dma_iter' [-Wunused-variable]
>     struct sg_dma_page_iter dma_iter;
>                             ^~~~~~~~
>   drivers/infiniband//hw/nes/nes_verbs.c:2094:13: warning: unused variable
>'last_dma_addr' [-Wunused-variable]
>     dma_addr_t last_dma_addr = 0;
>                ^~~~~~~~~~~~~
>   cc1: some warnings being treated as errors
>
>vim +2101 drivers/infiniband//hw/nes/nes_verbs.c
>
>  2084
>  2085	/**
>  2086	 * nes_reg_user_mr
>  2087	 */
>  2088	static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64
>length,
>  2089			u64 virt, int acc, struct ib_udata *udata)
>  2090	{
>  2091		u64 iova_start;
>  2092		__le64 *pbl;
>  2093		u64 region_length;
>  2094		dma_addr_t last_dma_addr = 0;
>  2095		dma_addr_t first_dma_addr = 0;
>  2096		struct nes_pd *nespd = to_nespd(pd);
>  2097		struct nes_vnic *nesvnic = to_nesvnic(pd->device);
>  2098		struct nes_device *nesdev = nesvnic->nesdev;
>  2099		struct nes_adapter *nesadapter = nesdev->nesadapter;
>  2100		struct ib_mr *ibmr = ERR_PTR(-EINVAL);
>> 2101		struct sg_dma_page_iter dma_iter;
>  2102		struct nes_ucontext *nes_ucontext;
>  2103		struct nes_pbl *nespbl;
>  2104		struct nes_mr *nesmr;
>  2105		struct ib_umem *region;
>  2106		struct nes_mem_reg_req req;
>  2107		struct nes_vpbl vpbl;
>  2108		struct nes_root_vpbl root_vpbl;
>  2109		int page_index;
>  2110		int page_count = 0;
>  2111		int err, pbl_depth = 0;
>  2112		int ret;
>  2113		u32 stag;
>  2114		u32 stag_index = 0;
>  2115		u32 next_stag_index;
>  2116		u32 driver_key;
>  2117		u32 root_pbl_index = 0;
>  2118		u32 cur_pbl_index = 0;
>  2119		u32 skip_pages;
>  2120		u16 pbl_count;
>  2121		u8 single_page = 1;
>  2122		u8 stag_key;
>  2123
>  2124		region = ib_umem_get(udata, start, length, acc, 0);
>  2125		if (IS_ERR(region)) {
>  2126			return (struct ib_mr *)region;
>  2127		}
>  2128
>  2129		nes_debug(NES_DBG_MR, "User base = 0x%lX, Virt base =
>0x%lX, length = %u,"
>  2130				" offset = %u, page size = %lu.\n",
>  2131				(unsigned long int)start, (unsigned long int)virt,
>(u32)length,
>  2132				ib_umem_offset(region), BIT(region-
>>page_shift));
>  2133
>  2134		skip_pages = ((u32)ib_umem_offset(region)) >> 12;
>  2135
>  2136		if (ib_copy_from_udata(&req, udata, sizeof(req))) {
>  2137			ib_umem_release(region);
>  2138			return ERR_PTR(-EFAULT);
>  2139		}
>  2140		nes_debug(NES_DBG_MR, "Memory Registration type =
>%08X.\n", req.reg_type);
>  2141
>  2142		switch (req.reg_type) {
>  2143			case IWNES_MEMREG_TYPE_MEM:
>  2144				pbl_depth = 0;
>  2145				region_length = 0;
>  2146				vpbl.pbl_vbase = NULL;
>  2147				root_vpbl.pbl_vbase = NULL;
>  2148				root_vpbl.pbl_pbase = 0;
>  2149
>  2150				get_random_bytes(&next_stag_index,
>sizeof(next_stag_index));
>  2151				stag_key = (u8)next_stag_index;
>  2152
>  2153				driver_key = next_stag_index & 0x70000000;
>  2154
>  2155				next_stag_index >>= 8;
>  2156				next_stag_index %= nesadapter->max_mr;
>  2157
>  2158				err = nes_alloc_resource(nesadapter,
>nesadapter->allocated_mrs,
>  2159						nesadapter->max_mr,
>&stag_index, &next_stag_index, NES_RESOURCE_USER_MR);
>  2160				if (err) {
>  2161					ib_umem_release(region);
>  2162					return ERR_PTR(err);
>  2163				}
>  2164
>  2165				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
>  2166				if (!nesmr) {
>  2167					ib_umem_release(region);
>  2168					nes_free_resource(nesadapter,
>nesadapter->allocated_mrs, stag_index);
>  2169					return ERR_PTR(-ENOMEM);
>  2170				}
>  2171				nesmr->region = region;
>  2172
>> 2173				for_each_sg_dma_page(region->sg_head.sgl,
>&dma_iter, region->nmap, 0) {
>  2174					struct sg_page_iter *piter =
>&dma_iter.base;
>  2175
>  2176					if (sg_dma_address(piter->sg) &
>~PAGE_MASK) {
>  2177						ib_umem_release(region);
>  2178						nes_free_resource(nesadapter,
>nesadapter->allocated_mrs, stag_index);
>  2179						nes_debug(NES_DBG_MR,
>"Unaligned Memory Buffer: 0x%x\n",
>  2180							  (unsigned
>int)sg_dma_address(piter->sg));
>  2181						ibmr = ERR_PTR(-EINVAL);
>  2182						kfree(nesmr);
>  2183						goto reg_user_mr_err;
>  2184					}
>  2185
>  2186					if (!sg_dma_len(piter->sg)) {
>  2187						ib_umem_release(region);
>  2188						nes_free_resource(nesadapter,
>nesadapter->allocated_mrs,
>  2189								  stag_index);
>  2190						nes_debug(NES_DBG_MR,
>"Invalid Buffer Size\n");
>  2191						ibmr = ERR_PTR(-EINVAL);
>  2192						kfree(nesmr);
>  2193						goto reg_user_mr_err;
>  2194					}
>  2195
>  2196					region_length += PAGE_SIZE;
>  2197					region_length -= skip_pages << 12;
>  2198					skip_pages = 0;
>  2199					if ((page_count != 0) && (page_count <<
>12) - (ib_umem_offset(region) & (4096 - 1)) >= region->length)
>  2200						goto enough_pages;
>  2201					if ((page_count & 0x01FF) == 0) {
>  2202						if (page_count >= 1024 * 512) {
>  2203							ib_umem_release(region);
>  2204
>	nes_free_resource(nesadapter,
>  2205
>nesadapter->allocated_mrs, stag_index);
>  2206							kfree(nesmr);
>  2207							ibmr = ERR_PTR(-E2BIG);
>  2208							goto reg_user_mr_err;
>  2209						}
>  2210						if (root_pbl_index == 1) {
>  2211							root_vpbl.pbl_vbase =
>pci_alloc_consistent(nesdev->pcidev,
>  2212									8192,
>&root_vpbl.pbl_pbase);
>  2213
>	nes_debug(NES_DBG_MR, "Allocating root PBL, va = %p, pa =
>0x%08X\n",
>  2214
>root_vpbl.pbl_vbase, (unsigned int)root_vpbl.pbl_pbase);
>  2215							if (!root_vpbl.pbl_vbase) {
>  2216
>	ib_umem_release(region);
>  2217
>	pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
>  2218
>vpbl.pbl_pbase);
>  2219
>	nes_free_resource(nesadapter, nesadapter->allocated_mrs,
>  2220
>stag_index);
>  2221								kfree(nesmr);
>  2222								ibmr = ERR_PTR(-
>ENOMEM);
>  2223								goto
>reg_user_mr_err;
>  2224							}
>  2225							root_vpbl.leaf_vpbl =
>kcalloc(1024,
>  2226
>sizeof(*root_vpbl.leaf_vpbl),
>  2227
>GFP_KERNEL);
>  2228							if (!root_vpbl.leaf_vpbl) {
>  2229
>	ib_umem_release(region);
>  2230
>	pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
>  2231
>root_vpbl.pbl_pbase);
>  2232
>	pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
>  2233
>vpbl.pbl_pbase);
>  2234
>	nes_free_resource(nesadapter, nesadapter->allocated_mrs,
>  2235
>stag_index);
>  2236								kfree(nesmr);
>  2237								ibmr = ERR_PTR(-
>ENOMEM);
>  2238								goto
>reg_user_mr_err;
>  2239							}
>  2240
>	root_vpbl.pbl_vbase[0].pa_low =
>  2241
>	cpu_to_le32((u32)vpbl.pbl_pbase);
>  2242
>	root_vpbl.pbl_vbase[0].pa_high =
>  2243
>	cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
>  2244							root_vpbl.leaf_vpbl[0] =
>vpbl;
>  2245						}
>  2246						vpbl.pbl_vbase =
>pci_alloc_consistent(nesdev->pcidev, 4096,
>  2247								&vpbl.pbl_pbase);
>  2248						nes_debug(NES_DBG_MR,
>"Allocating leaf PBL, va = %p, pa = 0x%08X\n",
>  2249							  vpbl.pbl_vbase,
>(unsigned int)vpbl.pbl_pbase);
>  2250						if (!vpbl.pbl_vbase) {
>  2251							ib_umem_release(region);
>  2252
>	nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
>  2253							ibmr = ERR_PTR(-
>ENOMEM);
>  2254							kfree(nesmr);
>  2255							goto reg_user_mr_err;
>  2256						}
>  2257						if (1 <= root_pbl_index) {
>  2258
>	root_vpbl.pbl_vbase[root_pbl_index].pa_low =
>  2259
>	cpu_to_le32((u32)vpbl.pbl_pbase);
>  2260
>	root_vpbl.pbl_vbase[root_pbl_index].pa_high =
>  2261
>	cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
>  2262
>	root_vpbl.leaf_vpbl[root_pbl_index] = vpbl;
>  2263						}
>  2264						root_pbl_index++;
>  2265						cur_pbl_index = 0;
>  2266					}
>  2267					if (single_page) {
>  2268						if (page_count != 0) {
>  2269							if ((last_dma_addr + 4096)
>!= sg_page_iter_dma_address(&dma_iter))
>  2270								single_page = 0;
>  2271							last_dma_addr =
>sg_page_iter_dma_address(&dma_iter);
>  2272						} else {
>  2273							first_dma_addr =
>sg_page_iter_dma_address(&dma_iter);
>  2274							last_dma_addr =
>first_dma_addr;
>  2275						}
>  2276					}
>  2277
>  2278					vpbl.pbl_vbase[cur_pbl_index].pa_low =
>  2279
>	cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
>  2280					vpbl.pbl_vbase[cur_pbl_index].pa_high =
>  2281
>	cpu_to_le32((u32)((u64)(sg_page_iter_dma_address(&dma_iter))));
>  2282					cur_pbl_index++;
>  2283					page_count++;
>  2284				}
>  2285
>  2286				enough_pages:
>  2287				nes_debug(NES_DBG_MR, "calculating stag,
>stag_index=0x%08x, driver_key=0x%08x,"
>  2288						" stag_key=0x%08x\n",
>  2289						stag_index, driver_key, stag_key);
>  2290				stag = stag_index << 8;
>  2291				stag |= driver_key;
>  2292				stag += (u32)stag_key;
>  2293
>  2294				iova_start = virt;
>  2295				/* Make the leaf PBL the root if only one PBL */
>  2296				if (root_pbl_index == 1) {
>  2297					root_vpbl.pbl_pbase = vpbl.pbl_pbase;
>  2298				}
>  2299
>  2300				if (single_page) {
>  2301					pbl_count = 0;
>  2302				} else {
>  2303					pbl_count = root_pbl_index;
>  2304					first_dma_addr = 0;
>  2305				}
>  2306				nes_debug(NES_DBG_MR, "Registering STag
>0x%08X, VA = 0x%08X, length = 0x%08X,"
>  2307						" index = 0x%08X, region-
>>length=0x%08llx, pbl_count = %u\n",
>  2308						stag, (unsigned int)iova_start,
>  2309						(unsigned int)region_length,
>stag_index,
>  2310						(unsigned long long)region-
>>length, pbl_count);
>  2311				ret = nes_reg_mr(nesdev, nespd, stag, region-
>>length, &root_vpbl,
>  2312						 first_dma_addr, pbl_count,
>(u16)cur_pbl_index, acc,
>  2313						 &iova_start, &nesmr->pbls_used,
>&nesmr->pbl_4k);
>  2314
>  2315				nes_debug(NES_DBG_MR, "ret=%d\n", ret);
>  2316
>  2317				if (ret == 0) {
>  2318					nesmr->ibmr.rkey = stag;
>  2319					nesmr->ibmr.lkey = stag;
>  2320					nesmr->mode =
>IWNES_MEMREG_TYPE_MEM;
>  2321					ibmr = &nesmr->ibmr;
>  2322				} else {
>  2323					ib_umem_release(region);
>  2324					kfree(nesmr);
>  2325					ibmr = ERR_PTR(-ENOMEM);
>  2326				}
>  2327
>  2328				reg_user_mr_err:
>  2329				/* free the resources */
>  2330				if (root_pbl_index == 1) {
>  2331					pci_free_consistent(nesdev->pcidev,
>4096, vpbl.pbl_vbase,
>  2332							vpbl.pbl_pbase);
>  2333				} else {
>  2334					for (page_index=0;
>page_index<root_pbl_index; page_index++) {
>  2335						pci_free_consistent(nesdev-
>>pcidev, 4096,
>  2336
>	root_vpbl.leaf_vpbl[page_index].pbl_vbase,
>  2337
>	root_vpbl.leaf_vpbl[page_index].pbl_pbase);
>  2338					}
>  2339					kfree(root_vpbl.leaf_vpbl);
>  2340					pci_free_consistent(nesdev->pcidev,
>8192, root_vpbl.pbl_vbase,
>  2341							root_vpbl.pbl_pbase);
>  2342				}
>  2343
>  2344				nes_debug(NES_DBG_MR, "Leaving, ibmr=%p",
>ibmr);
>  2345
>  2346				return ibmr;
>  2347			case IWNES_MEMREG_TYPE_QP:
>  2348			case IWNES_MEMREG_TYPE_CQ:
>  2349				if (!region->length) {
>  2350					nes_debug(NES_DBG_MR, "Unable to
>register zero length region for CQ\n");
>  2351					ib_umem_release(region);
>  2352					return ERR_PTR(-EINVAL);
>  2353				}
>  2354				nespbl = kzalloc(sizeof(*nespbl), GFP_KERNEL);
>  2355				if (!nespbl) {
>  2356					ib_umem_release(region);
>  2357					return ERR_PTR(-ENOMEM);
>  2358				}
>  2359				nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
>  2360				if (!nesmr) {
>  2361					ib_umem_release(region);
>  2362					kfree(nespbl);
>  2363					return ERR_PTR(-ENOMEM);
>  2364				}
>  2365				nesmr->region = region;
>  2366				nes_ucontext = to_nesucontext(pd->uobject-
>>context);
>  2367				pbl_depth = region->length >> 12;
>  2368				pbl_depth += (region->length & (4096-1)) ? 1 : 0;
>  2369				nespbl->pbl_size = pbl_depth*sizeof(u64);
>  2370				if (req.reg_type == IWNES_MEMREG_TYPE_QP)
>{
>  2371					nes_debug(NES_DBG_MR, "Attempting
>to allocate QP PBL memory");
>  2372				} else {
>  2373					nes_debug(NES_DBG_MR, "Attempting
>to allocate CP PBL memory");
>  2374				}
>  2375
>  2376				nes_debug(NES_DBG_MR, " %u bytes, %u
>entries.\n",
>  2377						nespbl->pbl_size, pbl_depth);
>  2378				pbl = pci_alloc_consistent(nesdev->pcidev,
>nespbl->pbl_size,
>  2379						&nespbl->pbl_pbase);
>  2380				if (!pbl) {
>  2381					ib_umem_release(region);
>  2382					kfree(nesmr);
>  2383					kfree(nespbl);
>  2384					nes_debug(NES_DBG_MR, "Unable to
>allocate PBL memory\n");
>  2385					return ERR_PTR(-ENOMEM);
>  2386				}
>  2387
>  2388				nespbl->pbl_vbase = (u64 *)pbl;
>  2389				nespbl->user_base = start;
>  2390				nes_debug(NES_DBG_MR, "Allocated PBL
>memory, %u bytes, pbl_pbase=%lx,"
>  2391						" pbl_vbase=%p
>user_base=0x%lx\n",
>  2392					  nespbl->pbl_size, (unsigned long)
>nespbl->pbl_pbase,
>  2393					  (void *) nespbl->pbl_vbase, nespbl-
>>user_base);
>  2394
>  2395				nespbl->page = sg_page(region->sg_head.sgl);
>  2396				for_each_sg_dma_page(region->sg_head.sgl,
>&dma_iter, region->nmap, 0) {
>  2397					((__le32 *)pbl)[0] =
>cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
>  2398					((__le32 *)pbl)[1] =
>cpu_to_le32(((u64)(sg_page_iter_dma_address(&dma_iter)))>>32);
>  2399					nes_debug(NES_DBG_MR, "pbl=%p,
>*pbl=0x%016llx, 0x%08x%08x\n", pbl,
>  2400						  (unsigned long long)*pbl,
>  2401						  le32_to_cpu(((__le32 *)pbl)[1]),
>le32_to_cpu(((__le32 *)pbl)[0]));
>  2402					pbl++;
>  2403				}
>  2404
>  2405				if (req.reg_type == IWNES_MEMREG_TYPE_QP)
>{
>  2406					list_add_tail(&nespbl->list,
>&nes_ucontext->qp_reg_mem_list);
>  2407				} else {
>  2408					list_add_tail(&nespbl->list,
>&nes_ucontext->cq_reg_mem_list);
>  2409				}
>  2410				nesmr->ibmr.rkey = -1;
>  2411				nesmr->ibmr.lkey = -1;
>  2412				nesmr->mode = req.reg_type;
>  2413				return &nesmr->ibmr;
>  2414		}
>  2415
>  2416		ib_umem_release(region);
>  2417		return ERR_PTR(-ENOSYS);
>  2418	}
>  2419
>
>---
>0-DAY kernel test infrastructure                Open Source Technology Center
>https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Patch

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 6eb991d..0a766ae 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ 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) {
 					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)) {
 					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;
 				region_length -= skip_pages << 12;
-				for (page_index = skip_pages; page_index < chunk_pages; page_index++) {
-					skip_pages = 0;
-					if ((page_count != 0) && (page_count << 12) - (ib_umem_offset(region) & (4096 - 1)) >= region->length)
-						goto enough_pages;
-					if ((page_count&0x01FF) == 0) {
-						if (page_count >= 1024 * 512) {
+				skip_pages = 0;
+				if ((page_count != 0) && (page_count << 12) - (ib_umem_offset(region) & (4096 - 1)) >= region->length)
+					goto enough_pages;
+				if ((page_count & 0x01FF) == 0) {
+					if (page_count >= 1024 * 512) {
+						ib_umem_release(region);
+						nes_free_resource(nesadapter,
+								  nesadapter->allocated_mrs, stag_index);
+						kfree(nesmr);
+						ibmr = ERR_PTR(-E2BIG);
+						goto reg_user_mr_err;
+					}
+					if (root_pbl_index == 1) {
+						root_vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev,
+								8192, &root_vpbl.pbl_pbase);
+						nes_debug(NES_DBG_MR, "Allocating root PBL, va = %p, pa = 0x%08X\n",
+							  root_vpbl.pbl_vbase, (unsigned int)root_vpbl.pbl_pbase);
+						if (!root_vpbl.pbl_vbase) {
 							ib_umem_release(region);
-							nes_free_resource(nesadapter,
-									  nesadapter->allocated_mrs, stag_index);
+							pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
+									    vpbl.pbl_pbase);
+							nes_free_resource(nesadapter, nesadapter->allocated_mrs,
+									  stag_index);
 							kfree(nesmr);
-							ibmr = ERR_PTR(-E2BIG);
+							ibmr = ERR_PTR(-ENOMEM);
 							goto reg_user_mr_err;
 						}
-						if (root_pbl_index == 1) {
-							root_vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev,
-									8192, &root_vpbl.pbl_pbase);
-							nes_debug(NES_DBG_MR, "Allocating root PBL, va = %p, pa = 0x%08X\n",
-								  root_vpbl.pbl_vbase, (unsigned int)root_vpbl.pbl_pbase);
-							if (!root_vpbl.pbl_vbase) {
-								ib_umem_release(region);
-								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
-										    vpbl.pbl_pbase);
-								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
-										  stag_index);
-								kfree(nesmr);
-								ibmr = ERR_PTR(-ENOMEM);
-								goto reg_user_mr_err;
-							}
-							root_vpbl.leaf_vpbl = kcalloc(1024,
-										      sizeof(*root_vpbl.leaf_vpbl),
-										      GFP_KERNEL);
-							if (!root_vpbl.leaf_vpbl) {
-								ib_umem_release(region);
-								pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
-										    root_vpbl.pbl_pbase);
-								pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
-										    vpbl.pbl_pbase);
-								nes_free_resource(nesadapter, nesadapter->allocated_mrs,
-										  stag_index);
-								kfree(nesmr);
-								ibmr = ERR_PTR(-ENOMEM);
-								goto reg_user_mr_err;
-							}
-							root_vpbl.pbl_vbase[0].pa_low =
-									cpu_to_le32((u32)vpbl.pbl_pbase);
-							root_vpbl.pbl_vbase[0].pa_high =
-									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
-							root_vpbl.leaf_vpbl[0] = vpbl;
-						}
-						vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev, 4096,
-								&vpbl.pbl_pbase);
-						nes_debug(NES_DBG_MR, "Allocating leaf PBL, va = %p, pa = 0x%08X\n",
-							  vpbl.pbl_vbase, (unsigned int)vpbl.pbl_pbase);
-						if (!vpbl.pbl_vbase) {
+						root_vpbl.leaf_vpbl = kcalloc(1024,
+									      sizeof(*root_vpbl.leaf_vpbl),
+									      GFP_KERNEL);
+						if (!root_vpbl.leaf_vpbl) {
 							ib_umem_release(region);
-							nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
-							ibmr = ERR_PTR(-ENOMEM);
+							pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
+									    root_vpbl.pbl_pbase);
+							pci_free_consistent(nesdev->pcidev, 4096, vpbl.pbl_vbase,
+									    vpbl.pbl_pbase);
+							nes_free_resource(nesadapter, nesadapter->allocated_mrs,
+									  stag_index);
 							kfree(nesmr);
+							ibmr = ERR_PTR(-ENOMEM);
 							goto reg_user_mr_err;
 						}
-						if (1 <= root_pbl_index) {
-							root_vpbl.pbl_vbase[root_pbl_index].pa_low =
-									cpu_to_le32((u32)vpbl.pbl_pbase);
-							root_vpbl.pbl_vbase[root_pbl_index].pa_high =
-									cpu_to_le32((u32)((((u64)vpbl.pbl_pbase)>>32)));
-							root_vpbl.leaf_vpbl[root_pbl_index] = vpbl;
-						}
-						root_pbl_index++;
-						cur_pbl_index = 0;
+						root_vpbl.pbl_vbase[0].pa_low =
+								cpu_to_le32((u32)vpbl.pbl_pbase);
+						root_vpbl.pbl_vbase[0].pa_high =
+								cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
+						root_vpbl.leaf_vpbl[0] = vpbl;
 					}
-					if (single_page) {
-						if (page_count != 0) {
-							if ((last_dma_addr+4096) !=
-									(sg_dma_address(sg)+
-									(page_index*4096)))
-								single_page = 0;
-							last_dma_addr = sg_dma_address(sg)+
-									(page_index*4096);
-						} else {
-							first_dma_addr = sg_dma_address(sg)+
-									(page_index*4096);
-							last_dma_addr = first_dma_addr;
-						}
+					vpbl.pbl_vbase = pci_alloc_consistent(nesdev->pcidev, 4096,
+							&vpbl.pbl_pbase);
+					nes_debug(NES_DBG_MR, "Allocating leaf PBL, va = %p, pa = 0x%08X\n",
+						  vpbl.pbl_vbase, (unsigned int)vpbl.pbl_pbase);
+					if (!vpbl.pbl_vbase) {
+						ib_umem_release(region);
+						nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
+						ibmr = ERR_PTR(-ENOMEM);
+						kfree(nesmr);
+						goto reg_user_mr_err;
+					}
+					if (1 <= root_pbl_index) {
+						root_vpbl.pbl_vbase[root_pbl_index].pa_low =
+								cpu_to_le32((u32)vpbl.pbl_pbase);
+						root_vpbl.pbl_vbase[root_pbl_index].pa_high =
+								cpu_to_le32((u32)((((u64)vpbl.pbl_pbase) >> 32)));
+						root_vpbl.leaf_vpbl[root_pbl_index] = vpbl;
+					}
+					root_pbl_index++;
+					cur_pbl_index = 0;
+				}
+				if (single_page) {
+					if (page_count != 0) {
+						if ((last_dma_addr + 4096) != sg_page_iter_dma_address(&dma_iter))
+							single_page = 0;
+						last_dma_addr = sg_page_iter_dma_address(&dma_iter);
+					} else {
+						first_dma_addr = sg_page_iter_dma_address(&dma_iter);
+						last_dma_addr = first_dma_addr;
 					}
-
-					vpbl.pbl_vbase[cur_pbl_index].pa_low =
-							cpu_to_le32((u32)(sg_dma_address(sg)+
-							(page_index*4096)));
-					vpbl.pbl_vbase[cur_pbl_index].pa_high =
-							cpu_to_le32((u32)((((u64)(sg_dma_address(sg)+
-							(page_index*4096))) >> 32)));
-					cur_pbl_index++;
-					page_count++;
 				}
+
+				vpbl.pbl_vbase[cur_pbl_index].pa_low =
+						cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
+				vpbl.pbl_vbase[cur_pbl_index].pa_high =
+						cpu_to_le32((u32)((u64)(sg_page_iter_dma_address(&dma_iter))));
+				cur_pbl_index++;
+				page_count++;
 			}
 
 			enough_pages:
@@ -2412,26 +2403,14 @@  static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 				  nespbl->pbl_size, (unsigned long) nespbl->pbl_pbase,
 				  (void *) nespbl->pbl_vbase, nespbl->user_base);
 
-			for_each_sg(region->sg_head.sgl, sg, region->nmap, entry) {
-				chunk_pages = sg_dma_len(sg) >> 12;
-				chunk_pages += (sg_dma_len(sg) & (4096-1)) ? 1 : 0;
-				if (first_page) {
-					nespbl->page = sg_page(sg);
-					first_page = 0;
-				}
-
-				for (page_index = 0; page_index < chunk_pages; page_index++) {
-					((__le32 *)pbl)[0] = cpu_to_le32((u32)
-							(sg_dma_address(sg)+
-							(page_index*4096)));
-					((__le32 *)pbl)[1] = cpu_to_le32(((u64)
-							(sg_dma_address(sg)+
-							(page_index*4096)))>>32);
-					nes_debug(NES_DBG_MR, "pbl=%p, *pbl=0x%016llx, 0x%08x%08x\n", pbl,
-						  (unsigned long long)*pbl,
-						  le32_to_cpu(((__le32 *)pbl)[1]), le32_to_cpu(((__le32 *)pbl)[0]));
-					pbl++;
-				}
+			nespbl->page = sg_page(region->sg_head.sgl);
+			for_each_sg_dma_page(region->sg_head.sgl, &dma_iter, region->nmap, 0) {
+				((__le32 *)pbl)[0] = cpu_to_le32((u32)(sg_page_iter_dma_address(&dma_iter)));
+				((__le32 *)pbl)[1] = cpu_to_le32(((u64)(sg_page_iter_dma_address(&dma_iter)))>>32);
+				nes_debug(NES_DBG_MR, "pbl=%p, *pbl=0x%016llx, 0x%08x%08x\n", pbl,
+					  (unsigned long long)*pbl,
+					  le32_to_cpu(((__le32 *)pbl)[1]), le32_to_cpu(((__le32 *)pbl)[0]));
+				pbl++;
 			}
 
 			if (req.reg_type == IWNES_MEMREG_TYPE_QP) {