On 06-Mar-19 20:34, Jason Gunthorpe wrote: > On Wed, Mar 06, 2019 at 05:55:11PM +0200, Gal Pressman wrote: >> Also, EFA is special in the sense that all sg functions use PAGE_SIZE while EFA >> needs EFA_PAGE_SIZE. I guess it could be solved by passing that as a parameter, >> but that'll probably require some changes in existing API functions >> (__sg_alloc_table_from_pages for example)? > > No, you are supposed to build up the sgl in large chunks and then > fragment it into whatever the HW requires after DMA mapping. That's fine, I'll change the stride to PAGE_SIZE. > > The code as written is wrong, as the IOMMU is free to consolidate the > carefully broken up SGL into a single entry after doing dma_map. > > Fragementation for HW page size limitations must be done after > mapping to use the APIs correctly. It does. When building the chunk list (pbl_chunk_list_create) we iterate each sg element in EFA_PAGE_SIZE strides. To be honest, the name EFA_PAGE_SIZE is misleading as it has nothing to do with the device' page sizes, EFA_CHUNK_PAYLOAD_SIZE is more appropriate. I'll rename it. > > Shiraz is working on a function to iterate over a sgl in specified > fixed size blocks (ie EFA_PAGE_SIZE), you should work with him as it > is exactly what is needed here as well. > >> Would you consider keeping efa_vmalloc_buf_to_sg() as a temporary solution? >> The scope of this change is a bit bigger than just RDMA and I'd like to split >> these changes. > > You still have to make what you have right.. > > I'd suggest you write the full function you'd like to propose for > lib/scatterlist.c in EFA and commit to move it out later. > > Jason >