| Message ID | 20220405135758.774016-3-catalin.marinas@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm, arm64: Reduce ARCH_KMALLOC_MINALIGN below the cache line size | expand |
On Wed, Apr 6, 2022 at 2:30 PM Catalin Marinas <catalin.marinas@arm.com> 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.
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@arm.com> 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).
diff --git a/drivers/base/devres.c b/drivers/base/devres.c index 864d0b3f566e..0ed39464ad08 100644 --- a/drivers/base/devres.c +++ b/drivers/base/devres.c @@ -29,10 +29,10 @@ struct devres { * Some archs want to perform DMA into kmalloc caches * and need a guaranteed alignment larger than * the alignment of a 64-bit integer. - * 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(). */ - u8 __aligned(ARCH_KMALLOC_MINALIGN) data[]; + u8 __aligned(ARCH_DMA_MINALIGN) data[]; }; struct devres_group {
ARCH_DMA_MINALIGN represents the minimum (static) alignment for safe DMA operations while ARCH_KMALLOC_MINALIGN is the minimum kmalloc() objects alignment. Signed-off-by: Catalin Marinas <catalin.marinas@arm.com> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> --- drivers/base/devres.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)