On 28-Mar-19 20:12, Jason Gunthorpe wrote: > On Thu, Mar 28, 2019 at 08:09:23PM +0200, Gal Pressman wrote: > >>>> I'm not entirely up to date with Shiraz's work, wanna make sure I understand: >>>> Currently the umem sgl is built out of PAGE_SIZE elements. The reason we can't >>>> assume each element is PAGE_SIZE long is because the DMA mapping (iommu) might >>>> combine multiple elements or because Shiraz's work is going to combine the >>>> elements manually? >>> >>> That is the existing reason, but this patch: >>> >>> https://patchwork.kernel.org/patch/10875419/ >>> >>> Just directly makes larger SGL. >> >> Why do we want to combine pages if the iommu does that for us? Isn't >> the only thing we need here is Shiraz's iterator that uses the device >> supported page sizes? > > Only some iommu's do it (and sometimes only in some configurations), > and it doesn't happen if the iommu isn't used. We want a consistent > operation for IB drivers. Thanks for the explanation. > >>>> Reading the documentation, sg_page_iter kinda looks like it covers >>>> sg_dma_page_iter functionality? Don't they both retrieve the DMA >>>> address page by page? Looks like the only difference is that in >>>> sg_page_iter I can get the struct page as well? >>> >>> No. >>> >>> A SGL contains two lists of different size, a 'dma mapped' list and a >>> 'CPU page list'. The act of DMA mapping creates the first list. >>> >>> These lists cannot be intermixed, if you run over the CPU page list >>> you can't get DMA addresses, and vice versa. >> >> The sg_page_iter documentation: >> >> /* >> * sg page iterator >> * >> * Iterates over sg entries page-by-page. On each successful iteration, you >> * can call sg_page_iter_page(@piter) to get the current page and its dma >> * address. @piter->sg will point to the sg holding this page and >> * @piter->sg_pgoffset to the page's page offset within the sg. The iteration >> * will stop either when a maximum number of sg entries was reached or a >> * terminating sg (sg_last(sg) == true) was reached. >> */ >> >> It says you can call sg_page_iter_page() to get the current page and its dma >> address, should the documentation be updated to remove this part? > > I missed that one 'dma address' when I fixed this :\ Send a patch? Sure.
