On Tue, Apr 18, 2017 at 03:31:58PM -0600, Logan Gunthorpe wrote: > 1) It means that sg_has_p2p has to walk the entire sg and check every > page. Then map_sg_p2p/map_sg has to walk it again and repeat the check > then do some operation per page. If anyone is concerned about the > dma_map performance this could be an issue. dma_map performance is a concern, this is why I suggest this as an interm solution until all dma_ops are migrated. Ideally sg_has_p2p would be a fast path that checked some kind of flags bit set during sg_assign_page... This would probably all have to be protected with CONFIG_P2P until it becomes performance neutral. People without an iommu are not going to want to walk the sg list at all.. > 2) Without knowing exactly what the arch specific code may need to do > it's hard to say that this is exactly the right approach. If every > dma_ops provider has to do exactly this on every page it may lead to a > lot of duplicate code: I think someone would have to start to look at it to make a determination.. I suspect the main server oriented iommu dma op will want to have proper p2p support anyhow and will probably have their unique control flow.. > The only thing I'm presently aware of is the segment check and applying > the offset to the physical address Well, I called the function p2p_same_segment_map_page() in my last suggestion for a reason - that is all the helper does. The intention would be for real iommu drivers to call that helper for the one simple case and if it fails then use their own routines to figure out if cross-segment P2P is possible and configure the iommu as needed. > bus specific and not arch specific which I think is what Dan may be > getting at. So it may make sense to just have a pci_map_sg_p2p() which > takes a dma_ops struct it would use for any page that isn't a p2p page. Like I keep saying, dma_ops are not really designed to be stacked. Try and write a stacked map_sg function like you describe and you will see how horrible it quickly becomes. Setting up an iommu is very expensive, so we need to batch it for the entire sg list. Thus a trivial implementation to iterate over all sg list entries is not desired. So first a sg list without p2p memory would have to be created, pass to the lower level ops, then brought back. Remember, the returned sg list will have a different number of entries than the original. Now another complex loop is needed to split/merge back in the p2p sg elements to get a return result. Finally, we have to undo all of this when doing unmap. Basically, all this list processing is a huge overhead compared to just putting a helper call in the existing sg iteration loop of the actual op. Particularly if the actual op is a no-op like no-mmu x86 would use. Since dma mapping is a performance path we must be careful not to create intrinsic inefficiencies with otherwise nice layering :) Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
