On 2018-04-12 06:33, Christian König wrote: > Am 12.04.2018 um 11:49 schrieb Lucas Stach: >> Am Donnerstag, den 12.04.2018, 11:35 +0200 schrieb Christian König: >>> Am 12.04.2018 um 11:11 schrieb Lucas Stach: >>>> Am Mittwoch, den 11.04.2018, 20:26 +0200 schrieb Christian König: >>>>> Am 11.04.2018 um 19:11 schrieb Robin Murphy: >>>>>> For dma_map_sg(), DMA API implementations are free to merge >>>>>> consecutive >>>>>> segments into a single DMA mapping if conditions are suitable, >>>>>> thus the >>>>>> resulting DMA addresses may be packed into fewer entries than >>>>>> ttm->sg->nents implies. >>>>>> >>>>>> drm_prime_sg_to_page_addr_arrays() does not account for this, >>>>>> meaning >>>>>> its callers either have to reject the 0 < count < nents case or >>>>>> risk >>>>>> getting bogus addresses back later. Fortunately this is relatively >>>>>> easy >>>>>> to deal with having to rejig structures to also store the mapped >>>>>> count, >>>>>> since the total DMA length should still be equal to the total >>>>>> buffer >>>>>> length. All we need is a separate scatterlist cursor to iterate >>>>>> the DMA >>>>>> addresses separately from the CPU addresses. >>>>> Mhm, I think I like Sinas approach better. >>>>> >>>>> See the hardware actually needs the dma_address on a page by page >>>>> basis. >>>>> >>>>> Joining multiple consecutive pages into one entry is just >>>>> additional >>>>> overhead which we don't need. >>>> But setting MAX_SEGMENT_SIZE will probably prevent an IOMMU that >>>> might >>>> be in front of your GPU to map large pages as such, causing >>>> additional >>>> overhead by means of additional TLB misses and page walks on the >>>> IOMMU >>>> side. >>>> >>>> And wouldn't you like to use huge pages at the GPU side, if the >>>> IOMMU >>>> already provides you the service of producing one large consecutive >>>> address range, rather than mapping them via a number of small pages? >>> No, I wouldn't like to use that. We're already using that :) >>> >>> But we use huge pages by allocating consecutive chunks of memory so >>> that >>> both the CPU as well as the GPU can increase their TLB hit rate. >> I'm not convinced that this is a universal win. Many GPU buffers >> aren't >> accessed by the CPU and allocating huge pages puts much more strain on >> the kernel MM subsystem. > > Yeah, we indeed see the extra overhead during allocation. > >>> What currently happens is that the DMA subsystem tries to coalesce >>> multiple pages into on SG entry and we de-coalesce that inside the >>> driver again to create our random access array. >> I guess the right thing would be to have a flag that tells the the DMA >> implementation to not coalesce the entries. But (ab-)using max segment >> size tells the DMA API to split the segments if they are larger than >> the given size, which is probably not what you want either as you now >> need to coalesce the segments again when you are mapping real huge >> pages. > > No, even with huge pages I need an array with every single dma address > for a 4K pages (or whatever the normal page size is). > > The problem is that I need a random access array for the DMA > addresses, even when they are continuously. > > I agree that max segment size is a bit ugly here, but at least for > radeon, amdgpu and pretty much TTM in general it is exactly what I > need. > > I could fix TTM to not need that, but for radeon and amdgpu it is the > hardware which needs this. I can implement i915 approach as Robin suggested as an alternative. Is that better? > > Christian. > >> >>> That is a huge waste of memory and CPU cycles and I actually wanted >>> to >>> get rid of it for quite some time now. Sinas approach seems to be a >>> good >>> step into the right direction to me to actually clean that up. >>> >>>> Detecting such a condition is much easier if the DMA map >>>> implementation >>>> gives you the coalesced scatter entries. >>> A way which preserves both path would be indeed nice to have, but >>> that >>> only allows for the TLB optimization for the GPU and not the CPU any >>> more. So I actually see that as really minor use case. >> Some of the Tegras have much larger TLBs and better page-walk >> performance on the system IOMMU compared to the GPU MMU, so they would >> probably benefit a good deal even if the hugepage optimization only >> targets the GPU side. >> >> Regards, >> Lucas
