Re: [PATCH] dma-direct: avoid redundant memory sync for swiotlb

[Robin Murphy]

On 2022-04-13 05:59, Christoph Hellwig wrote:
> On Wed, Apr 13, 2022 at 09:02:02AM +0800, Chao Gao wrote:
> > dma_direct_sync_single_for_cpu() also calls arch_sync_dma_for_cpu_all()
> > and arch_dma_mark_clean() in some cases. if SWIOTLB does sync internally,
> > should these two functions be called by SWIOTLB?
> >
> > Personally, it might be better if swiotlb can just focus on bounce buffer
> > alloc/free. Adding more DMA coherence logic into swiotlb will make it
> > a little complicated.
> >
> > How about an open-coded version of dma_direct_sync_single_for_cpu
> > in dma_direct_unmap_page with swiotlb_sync_single_for_cpu replaced by
> > swiotlb_tbl_unmap_single?
>
> I don't think the swiotlb and non-coherent case ever fully worked.
> Before the merge of swiotlb into dma-direct they obviously were
> mutally exclusive, and even now all the cache maintainance is done
> on the physical address of the original data, not the swiotlb buffer.

Are you sure? AFAICS swiotlb_map() does the right thing, and 
dma_direct_{sync,unmap} are working off the DMA address, which is that 
of the bounce slot when SWIOTLB is involved (not least, how would the 
is_swiotlb_buffer() checks work otherwise?)

> If we want to fix that properly all the arch dma calls will need to
> move into swiotlb, but that is a much bigger patch.
>
> So for now I'd be happy with the one liner presented here, but
> eventually the whole area could use an overhaul.

Sure, whoever gets round to tackling DMA_ATTR_NO_SNOOP first will need 
to go through all the cache maintenance hooks anyway, so happy to kick 
the can down the road until then.

Thanks,
Robin.