Re: Bug in add_dma_entry()'s debugging code

From: Alan Stern
Date: Fri Dec 01 2023 - 11:21:45 EST


On Fri, Dec 01, 2023 at 01:17:43PM +0100, Ferry Toth wrote:
> > A non-cache coherent platform would either have the kmalloc()
> > allocations aligned or it would bounce those small, unaligned buffers.
> It would? Or it always has?
> > So it doesn't seem right to issue a warning on x86 where kmalloc()
> > minimum alignment is 8 and not getting the warning on a non-coherent
> > platform which forces the kmalloc() alignment.
>
> If *all*non-coherent platforms implement either correct alignment or bounce
> buffer, and on (coherent) x86 we get an WARN, then it is a false alarm
> right?
>
> That is exactly my question (because I have no idea which platforms have
> non-coherent caches).

Don't forget, not all DMA buffers are allocated by kmalloc(). A buffer
allocated by some other means might not be aligned properly and might
share a cache line with another buffer.

Or you might have a single data structure that was allocated by
kmalloc() and then create separate DMA mappings for two members of that
structure. If the two members are in the same cache line, that would be
an error.

Alan Stern

> > If we consider the kmalloc() aspect already covered by bouncing or force
> > alignment, the DMA API debug code can still detect other cache line
> > sharing situations.
>
> Consider? Or know?
>
> If we know, and we can not easily prevent false WARN's on x86 it could be
> sufficient to change the message to "possible cacheline sharing detected"
> and add a line that "DMA_API_DEBUG" should be disabled on production
> systems.
>
> And while at it, we might be able to fix the missed real cacheline sharing
> mentioned by Alan:
>
>  "As a separate issue, active_cacheline_insert() fails to detect
> overlap in situations where a mapping occupies more than one cache line.
> For example, if mapping A uses lines N and N+1 and mapping B uses line
> N+1, no overlap will be detected because the radix-tree keys for A and B
> will be different (N vs. N+1)."
>