>Subject: Re: [PATCH v2 rdma-next 1/5] RDMA/umem: Add API to find best driver >supported page size in an MR > >On Fri, May 03, 2019 at 03:22:59PM +0000, Saleem, Shiraz wrote: >> >This is because mask shouldn't start as zero - the highest possible >> >mask is something like log2_ru(umem length) >> > >> >ie absent other considerations the page size at the NIC should be the >> >size of the umem. >> > >> >Then we scan the sgl and reduce that value based on the physical >> >address layout >> > >> >Then we reduce it again based on the uvirt vs address difference >> > >> >Oring a '0' into the mask means that step contributes no restriction. >> > >> >.. >> > >> >So I think the algorithm is just off as is, it should be more like >> > >> > // Page size can't be larger than the length of the MR mask = >> >log2_ru(umem length); >> > >> > // offset into the first SGL for umem->addr pgoff = umem->address & >> >PAGE_MASK; va = uvirt_addr; >> > >> >> Did you mean pgoff = umem->address & ~PAGE_MASK? > >Yes... OK. Don't we need something like this for zero based VA? va = uvirt_addr ? uvirt_addr : umem->addr; Also can we do this for all HW? Or do we keep the IB_UMEM_VA_BASED_OFFSET flag and OR in the dma_addr_end (for first SGL) and dma_addr_start (for last SGL) to the mask when the flag is not set? > for_each_sgl() > mask |= (sgl->dma_addr + pgoff) ^ va > if (not first or last) > // Interior SGLs limit by the length as well > mask |= sgl->length; > va += sgl->length - pgoff; > pgoff = 0; > > >But really even that is not what it should be, the 'pgoff' should simply be the >'offset' member of the first sgl and we should set it correctly based on the umem- >>address when building the sgl in the core code. > I can look to update it post this series. And the core code changes to not over allocate scatter table.