Re: slub - extended kmalloc redzone and dma alignment

[Petr Tesarik <ptesarik@suse.com>]

On Mon, 7 Apr 2025 18:12:09 +0100
Catalin Marinas <catalin.marinas@arm.com> wrote:

> Hi Vlastimil, Feng,
> 
> Thanks for looping me in. I'm just catching up with this thread.
> 
> On Mon, Apr 07, 2025 at 09:54:41AM +0200, Vlastimil Babka wrote:
> > On 4/7/25 09:21, Feng Tang wrote:  
> > > On Sun, Apr 06, 2025 at 10:02:40PM +0800, Feng Tang wrote:
> > > [...]  
> > >> > I can remember this series, as well as my confusion why
> > >> > 192-byte kmalloc caches were missing on arm64.
> > >> > 
> > >> > Nevertheless, I believe ARCH_DMA_MINALIGN is required to avoid
> > >> > putting a DMA buffer on the same cache line as some other data
> > >> > that might be _written_ by the CPU while the corresponding
> > >> > main memory is modified by another bus-mastering device.
> > >> > 
> > >> > Consider this layout:
> > >> > 
> > >> >  ... | DMA buffer | other data | ...
> > >> >      ^                         ^
> > >> >      +-------------------------+-- cache line boundaries
> > >> > 
> > >> > When you prepare for DMA, you make sure that the DMA buffer is
> > >> > not cached by the CPU, so you flush the cache line (from all
> > >> > levels). Then you tell the device to write into the DMA
> > >> > buffer. However, before the device finishes the DMA
> > >> > transaction, the CPU accesses "other data", loading this cache
> > >> > line from main memory with partial results. Worse, if the CPU
> > >> > writes to "other data", it may write the cache line back into
> > >> > main memory, racing with the device writing to DMA buffer, and
> > >> > you end up with corrupted data in DMA buffer.  
> 
> Yes, cache evictions from 'other data; can override the DMA. Another
> problem, when the DMA completed, the kernel does a cache invalidation
> to remove any speculatively loaded cache lines from the DMA buffer
> but that would also invalidate 'other data', potentially corrupting
> it if it was dirty.
> 
> So it's not safe to have DMA into buffers less than ARCH_DMA_MINALIGN
> (and unaligned).

It's not safe to DMA into buffers that share a CPU cache line with other
data, which could be before or after the DMA buffer, of course.

> What I did with reducing the minimum kmalloc()
> alignment was to force bouncing via swiotlb if the size passed to the
> DMA API is small. It may end up bouncing buffers that did not
> originate from kmalloc() or have proper alignment (with padding) but
> that's some heuristics we were willing to accept to be able to use
> small kmalloc() caches on arm64 - see dma_kmalloc_needs_bounce().
> 
> Does redzoning apply to kmalloc() or kmem_cache_create() (or both)? I
> haven't checked yet but if the red zone is within ARCH_DMA_MINALIGN
> (or rather dma_get_cache_alignment()), we could have issues with
> either corrupting the DMA buffer or the red zone. [...]

I'm sorry if I'm being thick, but IIUC the red zone does not have to be
protected. Yes, we might miss red zone corruption if it happens to race
with a DMA transaction, but I have assumed that this is permissible. I
regard the red zone as a useful debugging tool, not a safety measure
that is guaranteed to detect any write beyond the buffer end.

>[...]
> > > I'm not familiar with DMA stuff, but Vlastimil's idea does make it
> > > easier for driver developer to write a driver to be used on
> > > different ARCHs, which have different DMA alignment requirement.
> > > Say if the minimal safe size is 8 bytes, the driver can just
> > > request 8 bytes and ARCH_DMA_MINALIGN will automatically chose
> > > the right size for it, which can save memory for ARCHs with
> > > smaller alignment requirement. Meanwhile it does sacrifice part
> > > of the redzone check ability, so I don't have preference here :)  
> > 
> > Let's clarify first who's expected to ensure the word alignment for
> > DMA, if it's not kmalloc() then I'd rather resist moving it there
> > :)  
> 
> In theory, the DMA API should handle the alignment as I tried to
> remove it from the kmalloc() code.

Are we talking about the alignment of the starting address, or buffer
size, or both?

> With kmem_cache_create() (or kmalloc() as well), if the object size is
> not cacheline-aligned, is there risk of redzoning around the object
> without any alignment restrictions? The logic in
> dma_kmalloc_size_aligned() would fail for sufficiently large buffers
> but with unaligned red zone around the object.

This red zone is the extra memory that is normally wasted by kmalloc()
rounding up the requested size to the bucket size.
But dma_kmalloc_size_aligned() already uses kmalloc_size_roundup(size),
so it seems to be covered.

Petr T