Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220405135758.774016-1-catalin.marinas@arm.com>
 <20220405135758.774016-2-catalin.marinas@arm.com> <YkzX1nM0Ca7apVBt@hyeyoo>
In-Reply-To: <YkzX1nM0Ca7apVBt@hyeyoo>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Wed, 6 Apr 2022 09:29:19 +0200
Message-ID: <CAK8P3a1K0=jwYEHVu=X7oAWk9dzaOYAdFsidwVRKCJVReSV3+g@mail.gmail.com>
Subject: Re: [PATCH 01/10] mm/slab: Decouple ARCH_KMALLOC_MINALIGN from ARCH_DMA_MINALIGN
To:     Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Linux-MM <linux-mm@kvack.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Christoph Lameter <cl@linux.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        David Rientjes <rientjes@google.com>,
        Pekka Enberg <penberg@kernel.org>, roman.gushchin@linux.dev
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Apr 6, 2022 at 1:59 AM Hyeonggon Yoo <42.hyeyoo@gmail.com> wrote:
>
> On Tue, Apr 05, 2022 at 02:57:49PM +0100, Catalin Marinas wrote:
> > In preparation for supporting a dynamic kmalloc() minimum alignment,
> > allow architectures to define ARCH_KMALLOC_MINALIGN independently of
> > ARCH_DMA_MINALIGN. In addition, always define ARCH_DMA_MINALIGN even if
> > an architecture does not override it.
> >
>
> [ +Cc slab maintainer/reviewers ]
>
> I get why you want to set minimum alignment of kmalloc() dynamically.
> That's because cache line size can be different and we cannot statically
> know that, right?
>
> But I don't get why you are trying to decouple ARCH_KMALLOC_MINALIGN
> from ARCH_DMA_MINALIGN. kmalloc'ed buffer is always supposed to be DMA-safe.
>
> I'm afraid this series may break some archs/drivers.
>
> in Documentation/dma-api-howto.rst:
> > 2) ARCH_DMA_MINALIGN
> >
> >   Architectures must ensure that kmalloc'ed buffer is
> >   DMA-safe. Drivers and subsystems depend on it. If an architecture
> >   isn't fully DMA-coherent (i.e. hardware doesn't ensure that data in
> >   the CPU cache is identical to data in main memory),
> >   ARCH_DMA_MINALIGN must be set so that the memory allocator
> >   makes sure that kmalloc'ed buffer doesn't share a cache line with
> >   the others. See arch/arm/include/asm/cache.h as an example.
> >
> >   Note that ARCH_DMA_MINALIGN is about DMA memory alignment
> >   constraints. You don't need to worry about the architecture data
> >   alignment constraints (e.g. the alignment constraints about 64-bit
> >   objects).
>
> If I'm missing something, please let me know :)

It helps in two ways:

- you can start with a relatively large hardcoded ARCH_DMA_MINALIGN
  of 128 or 256 bytes, depending on what the largest possible line size
  is for any machine you want to support, and then drop that down to
  32 or 64 bytes based on runtime detection. This should always be safe,
  and it means a very sizable chunk of wasted memory can be recovered.

- On systems that are fully cache coherent, there is no need to align
  kmallloc() allocations for DMA safety at all, on these, we can drop the
  size even below the cache line. This does not apply on most of the
  cheaper embedded or mobile SoCs, but it helps a lot on the machines
  you'd find in a data center.

        Arnd