Re: [PATCH] swiotlb: sync buffer when mapping FROM_DEVICE
Hi Robin, On 2019-05-22 15:55, Robin Murphy wrote: > On 22/05/2019 14:34, Christoph Hellwig wrote: >> On Wed, May 22, 2019 at 02:25:38PM +0100, Robin Murphy wrote: >>> Sure, but that should be irrelevant since the effective problem here >>> is in >>> the sync_*_for_cpu direction, and it's the unmap which nobbles the >>> buffer. >>> If the driver does this: >>> >>> dma_map_single(whole buffer); >>> >>> dma_unmap_single(whole buffer); >>> >>> >>> then it could instead do this and be happy: >>> >>> dma_map_single(whole buffer, SKIP_CPU_SYNC); >>> >>> dma_sync_single_for_cpu(updated part of buffer); >>> dma_unmap_single(whole buffer, SKIP_CPU_SYNC); >>> >> >> Assuming the driver knows how much was actually DMAed this would >> solve the issue. Horia, does this work for you? > > Ohhh, and now I've just twigged what you were suggesting - your > DMA_ATTR_PARTIAL flag would mean "treat this as a read-modify-write of > the buffer because we *don't* know exactly which parts the device may > write to". So indeed if we did go down that route we wouldn't need any > of the sync stuff I was worrying about (but I might suggest naming it > DMA_ATTR_UPDATE instead). Apologies for being slow :) Don't we have DMA_BIDIRECTIONAL for such case? Maybe we should update documentation a bit to point that DMA_FROM_DEVICE expects the whole buffer to be filled by the device? Best regards -- Marek Szyprowski, PhD Samsung R Institute Poland
Re: [PATCH] swiotlb: sync buffer when mapping FROM_DEVICE
On 2019-05-22 2:09 pm, Christoph Hellwig wrote: On Wed, May 22, 2019 at 01:50:47PM +0100, Robin Murphy wrote: Would that work out any different from the existing DMA_ATTR_SKIP_CPU_SYNC? If drivers are prepared to handle this issue from their end, they can already do so for single mappings by using that attr along with explicit partial syncs via dma_sync_single(). For page/sg mappings we'd still have the problem of identifying what part of "partial" actually matters, and probably having to add some additional new sync operations to cope. Except that the same optimization we are tripping over here is also present in dma_sync_* - dma_sync_*_to_device with DMA_FROM_DEVICE is a no-op in swiotlb. Sure, but that should be irrelevant since the effective problem here is in the sync_*_for_cpu direction, and it's the unmap which nobbles the buffer. If the driver does this: dma_map_single(whole buffer); dma_unmap_single(whole buffer); then it could instead do this and be happy: dma_map_single(whole buffer, SKIP_CPU_SYNC); dma_sync_single_for_cpu(updated part of buffer); dma_unmap_single(whole buffer, SKIP_CPU_SYNC); Robin.
Re: [PATCH] swiotlb: sync buffer when mapping FROM_DEVICE
On Wed, May 22, 2019 at 01:50:47PM +0100, Robin Murphy wrote: > Would that work out any different from the existing DMA_ATTR_SKIP_CPU_SYNC? > > If drivers are prepared to handle this issue from their end, they can > already do so for single mappings by using that attr along with explicit > partial syncs via dma_sync_single(). For page/sg mappings we'd still have > the problem of identifying what part of "partial" actually matters, and > probably having to add some additional new sync operations to cope. Except that the same optimization we are tripping over here is also present in dma_sync_* - dma_sync_*_to_device with DMA_FROM_DEVICE is a no-op in swiotlb.
