Re: [PATCH] arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Christoph Hellwig <hch@lst.de>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	Mark Rutland <mark.rutland@arm.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Ard Biesheuvel <ardb@kernel.org>
Subject: Re: [PATCH] arm64: dma: Drop cache invalidation from arch_dma_prep_coherent()
Date: Wed, 7 Sep 2022 15:10:35 +0100	[thread overview]
Message-ID: <YximWx/g2FjA/Ypl@shell.armlinux.org.uk> (raw)
In-Reply-To: <20220907090434.GB30704@lst.de>

On Wed, Sep 07, 2022 at 11:04:34AM +0200, Christoph Hellwig wrote:
> On Wed, Aug 24, 2022 at 11:00:02PM +0100, Catalin Marinas wrote:
> > (I was wondering why not just invalidate without clean but it could be
> > that the allocated memory was zeroed and we want that to make it to the
> > PoC)
> 
> The memory is zerod after arch_dma_prep_coherent.  So a pure invalidate
> seems like the right thing to do here.

My thoughts on Christoph's comment...

That seems backwards to me.

If we allocate memory, clean (and maybe invalidate) the cache, and then
zero the memory _after_ the clean, then couldn't we be creating dirty
cache lines. There are caches out there that are write-allocate, which
means that by writing zeros to memory, you drag cache lines back into
the cache. These dirty cache lines could then be written back at some
random time later, corrupting the data that has been DMA'd to the
buffer or placed there via the uncached mapping (if the architecture
doesn't hit those cache lines.)

It has to be:
- allocate
- zero
- deal with the cache

not:
- allocate
- deal with the cache
- zero

Moreover, if one does:
- allocate
- invalidate
- zero
- clean+(invalidate?)

then aren't we making the zero operation potentially more expensive than
if we had cache lines already there - I'm thinking there about a
no-write-allocate cache here. Those cache lines could have been used to
buffer the zeroing writes and then written out later. Depending on the
architecture, having cached lines may mean that write merging happens,
meaning writebacks to memory occur in larger chunks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel