RE: [PATCH 04/11] ia64: make dma_cache_sync a no-op

From: David Laight
Date: Wed Oct 04 2017 - 05:01:10 EST


From: Christoph Hellwig
> Sent: 03 October 2017 11:43
>
> ia64 does not implement DMA_ATTR_NON_CONSISTENT allocations, so it doesn't
> make any sense to do any work in dma_cache_sync given that it must be a
> no-op when dma_alloc_attrs returns coherent memory.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> arch/ia64/include/asm/dma-mapping.h | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/arch/ia64/include/asm/dma-mapping.h b/arch/ia64/include/asm/dma-mapping.h
> index 3ce5ab4339f3..99dfc1aa9d3c 100644
> --- a/arch/ia64/include/asm/dma-mapping.h
> +++ b/arch/ia64/include/asm/dma-mapping.h
> @@ -48,11 +48,6 @@ static inline void
> dma_cache_sync (struct device *dev, void *vaddr, size_t size,
> enum dma_data_direction dir)
> {
> - /*
> - * IA-64 is cache-coherent, so this is mostly a no-op. However, we do need to
> - * ensure that dma_cache_sync() enforces order, hence the mb().
> - */
> - mb();
> }

Are you sure about this one?
It looks as though you are doing a mechanical change for all architectures.
Some of them are probably stranger than you realise.

Even with cache coherent memory any cpu 'store/write buffer' may not
be snooped by dma reads.

Something needs to flush the store buffer between the last cpu write
to the dma buffer and the write (probably a device register) that
tells the device it can read the memory.

My guess from the comment is that dma_cache_synch() is expected to
include that barrier - and it might not be anywhere else.

David