Re: [PATCH v7 01/17] mm/slab: Decouple ARCH_KMALLOC_MINALIGN from ARCH_DMA_MINALIGN

[Catalin Marinas <catalin.marinas@arm.com>]

On Tue, Jun 13, 2023 at 11:46:31AM +0200, Vlastimil Babka wrote:
> On 6/12/23 17:31, Catalin Marinas wrote:
> > In preparation for supporting a kmalloc() minimum alignment smaller than
> > the arch DMA alignment, decouple the two definitions. This requires that
> > either the kmalloc() caches are aligned to a (run-time) cache-line size
> > or the DMA API bounces unaligned kmalloc() allocations. Subsequent
> > patches will implement both options.
> > 
> > After this patch, ARCH_DMA_MINALIGN is expected to be used in static
> > alignment annotations and defined by an architecture to be the maximum
> > alignment for all supported configurations/SoCs in a single Image.
> > Architectures opting in to a smaller ARCH_KMALLOC_MINALIGN will need to
> > define its value in the arch headers.
> > 
> > Since ARCH_DMA_MINALIGN is now always defined, adjust the #ifdef in
> > dma_get_cache_alignment() so that there is no change for architectures
> > not requiring a minimum DMA alignment.
> > 
> > Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Vlastimil Babka <vbabka@suse.cz>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Cc: Robin Murphy <robin.murphy@arm.com>
> > Tested-by: Isaac J. Manjarres <isaacmanjarres@google.com>
> 
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks for the ack.

It looks like moving the ARCH_DMA_MINALIGN definition to linux/cache.h
messes up powerpc32 -
https://lore.kernel.org/r/202306131053.1ybvRRhO-lkp@intel.com - possibly
a few other architecture that define this macro in a file other than
asm/cache.h (I only tested powerpc64 before posting).

At a quick grep, we have:

  arch/microblaze/include/asm/page.h:34: #define ARCH_DMA_MINALIGN       L1_CACHE_BYTES

This could be moved to asm/cache.h. It probably already relies on some
indirect includes.

  arch/mips/include/asm/mach-generic/kmalloc.h:10: #define ARCH_DMA_MINALIGN       128
  arch/mips/include/asm/mach-ip32/kmalloc.h:7: #define ARCH_DMA_MINALIGN       32
  arch/mips/include/asm/mach-ip32/kmalloc.h:9: #define ARCH_DMA_MINALIGN       128
  arch/mips/include/asm/mach-n64/kmalloc.h:6: #define ARCH_DMA_MINALIGN L1_CACHE_BYTES
  arch/mips/include/asm/mach-tx49xx/kmalloc.h:5: #define ARCH_DMA_MINALIGN L1_CACHE_BYTES

These are ok since kmalloc.h is included in asm/cache.h.

  arch/powerpc/include/asm/page_32.h:16: #define ARCH_DMA_MINALIGN       L1_CACHE_BYTES

I'd move this macro to asm/cache.h on powerpc. asm/page_32.h already
includes asm/cache.h.

  arch/sh/include/asm/page.h:181: #define ARCH_DMA_MINALIGN       L1_CACHE_BYTES

Same here. L1_CACHE_BYTES is defined in asm/cache.h but it's not
included in asm/page.h, so I guess it's relying on some indirect
includes.

I'll post three more patches to move the ARCH_DMA_MINALIGN to
asm/cache.h for microblaze, powerpc and sh (once I finished build
testing).

-- 
Catalin