Re: [PATCH 02/10] drivers/base: Use ARCH_DMA_MINALIGN instead of ARCH_KMALLOC_MINALIGN

[Catalin Marinas]

On Mon, Apr 11, 2022 at 05:57:08PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 6, 2022 at 2:30 PM Catalin Marinas <catalin.marinas@xxxxxxx> wrote:
> > ARCH_DMA_MINALIGN represents the minimum (static) alignment for safe DMA
> > operations while ARCH_KMALLOC_MINALIGN is the minimum kmalloc() objects
> > alignment.
> 
> ...
> 
> > -        * Thus we use ARCH_KMALLOC_MINALIGN here and get exactly the same
> > +        * Thus we use ARCH_DMA_MINALIGN here and get at least the same
> >          * buffer alignment as if it was allocated by plain kmalloc().
> 
> But then it becomes not true either, because the kmalloc() has other
> alignment constraints.

Maybe the comment could be improved a bit but I think it's still valid.
After this patch, struct devres becomes:

struct devres {
	struct devres_node		node;
	u8 __aligned(ARCH_DMA_MINALIGN) data[];
};

While we no longer guarantee the ARCH_DMA_MINALIGN alignment (which is
too big on most arm64 SoCs), what we need is for devres.data[] to be
aligned to the newly introduced arch_kmalloc_minalign(). This would give
us the DMA safety guarantees.

Since devres.data[] is at an offset multiple of ARCH_DMA_MINALIGN, in
order for the array to be aligned to arch_kmalloc_minalign(), all we
need is for ARCH_DMA_MINALIGN to be a multiple of
arch_kmalloc_minalign(). I actually had to write down some simple
equations to convince myself.

devres.data[] is at an offset multiple of ARCH_DMA_MINALIGN (after this
patch), even when struct devres is included in another structure, so we
have:

  offsetof(struct devres, data) = m * ARCH_DMA_MINALIGN

ARCH_DMA_MINALIGN is a power of two while arch_kmalloc_minalign() is
also a power of two, equal to or less than ARCH_DMA_MINALIGN:

  ARCH_DMA_MINALIGN = n * arch_kmalloc_minalign()

A kmalloc()'ed object of struct devres (or a container of) is aligned to
arch_kmalloc_minalign() by definition so:

  kmalloc() = p * arch_kmalloc_minalign()

>From the above, we can conclude that the data[] pointer is at a multiple
of arch_kmalloc_minalign():

  devres.data = (p + m * n) * arch_kmalloc_minalign()

Where m, n, p are all positive integers (n is also power of two).

If we did not change the devres structure, the alignment of
ARCH_KMALLOC_MINALIGN would no no be longer sufficient since the dynamic
arch_kmalloc_minalign() can be greater than ARCH_KMALLOC_MINALIGN on
specific SoCs (the first offsetof equation is no longer true).

-- 
Catalin