From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 06485C433F5 for ; Fri, 22 Apr 2022 19:46:04 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8FBD26B0074; Fri, 22 Apr 2022 15:46:03 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8AAA86B0075; Fri, 22 Apr 2022 15:46:03 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7234E6B0078; Fri, 22 Apr 2022 15:46:03 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.hostedemail.com [64.99.140.25]) by kanga.kvack.org (Postfix) with ESMTP id 660D96B0074 for ; Fri, 22 Apr 2022 15:46:03 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 38471208F1 for ; Fri, 22 Apr 2022 19:46:03 +0000 (UTC) X-FDA: 79385545806.18.A90D917 Received: from mail-vk1-f169.google.com (mail-vk1-f169.google.com [209.85.221.169]) by imf26.hostedemail.com (Postfix) with ESMTP id 45D12140028 for ; Fri, 22 Apr 2022 19:46:01 +0000 (UTC) Received: by mail-vk1-f169.google.com with SMTP id 80so4332773vkw.0 for ; Fri, 22 Apr 2022 12:46:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NDEqcBhWfl+VEeADqX1cuVP35Bwxo39I117kMXuRP20=; b=TLtW1qg60rSGVQeGek0tIZbyfjQkLJWNt2ngGVAqGrQrV73U7QZtPSFld4Uyi0B6RP J9GPkHNV3XuQhGBYiytSnqd/7l1Gv/e1iTmShfjHJd7aH2wgp+/pbA9v+dtWis78Jnpu shzDMOEg9NWFbY6b1iewLe00f4U+La5RPbyk6Ie0Ag7oryOkMlVRhjEkwLlPuhZUcxwB eg3NNw7urj70jyvLtzGbE31gCm4Plz6qEPQs58pVl4HHSt2yZDX2K7lXm3NSPByh6cky 4Fh3WvhbaSpX6tmGDaUcBj72izF712SRHKPdmbs3TTcHsNQuJCdhoWYA9kih0BVMMGnN kH1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NDEqcBhWfl+VEeADqX1cuVP35Bwxo39I117kMXuRP20=; b=HTiwjM5wOHHN6FuiTZSszWDNdqcfl3StGSqNW6bmaVbyXhVezylpXaQuLJkUPktG5V 1NjHp7SV+yn/+sNU1zCCd3oiYtMnTN1XSjaMw5D+Mwxdhdzcoioa7iOwcdkoipBvJG1y MF/+H+W3xoqzsXfpG/p7KnH1eu0VbghN9ayiQZPjNnpcrDBsta+OCWmnbB8U1dLNRCWJ tBmfdliFJSqyT45DMLlq3BDjkxe6EFucJpXQi6D7Lc4sRwOlwrHiDyxlAP2XPLE3R3Zr dWafnF0qk2/kQwgpTZ/SH+ZOSNXbs0GcJGrV9tgeXKRGsY0WfieFx/HCIEG910nQeUaq ClaQ== X-Gm-Message-State: AOAM532wiLhk6yZpjrxUh4w3U041lwD6+lMVmK8Tmslkfkddrf1Oqs0A GMxjU5WDMT/TA7y9m58IuULks6e3SMsLDGY2Y8/IGg== X-Google-Smtp-Source: ABdhPJzFRaF/faHnVzvRvbLroYvArL300ra3ebJiuClesrr1sjxw+ZwLuwVyHz2uC4w6FbE23qJp/8nA5BlJJS6geDs= X-Received: by 2002:a1f:314b:0:b0:331:fff6:a89e with SMTP id x72-20020a1f314b000000b00331fff6a89emr2323548vkx.26.1650656761610; Fri, 22 Apr 2022 12:46:01 -0700 (PDT) MIME-Version: 1.0 References: <20220421211549.3884453-1-pcc@google.com> In-Reply-To: From: Peter Collingbourne Date: Fri, 22 Apr 2022 12:45:50 -0700 Message-ID: Subject: Re: [PATCH v2] mm: make minimum slab alignment a runtime property To: Andrey Konovalov Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>, Andrew Morton , Catalin Marinas , Linux ARM , Linux Memory Management List , Linux Kernel Mailing List , Vlastimil Babka , Pekka Enberg , roman.gushchin@linux.dev, Joonsoo Kim , David Rientjes , Herbert Xu , Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , kasan-dev , Eric Biederman , Kees Cook Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: msu9qpzmxioung8csqqt8yiwf8qtahuh X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 45D12140028 X-Rspam-User: Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=TLtW1qg6; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf26.hostedemail.com: domain of pcc@google.com designates 209.85.221.169 as permitted sender) smtp.mailfrom=pcc@google.com X-HE-Tag: 1650656761-261986 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Apr 22, 2022 at 9:04 AM Andrey Konovalov wrote: > > On Thu, Apr 21, 2022 at 11:16 PM Peter Collingbourne wrote: > > > > When CONFIG_KASAN_HW_TAGS is enabled we currently increase the minimum > > slab alignment to 16. This happens even if MTE is not supported in > > hardware or disabled via kasan=off, which creates an unnecessary > > memory overhead in those cases. Eliminate this overhead by making > > the minimum slab alignment a runtime property and only aligning to > > 16 if KASAN is enabled at runtime. > > > > On a DragonBoard 845c (non-MTE hardware) with a kernel built with > > CONFIG_KASAN_HW_TAGS, waiting for quiescence after a full Android > > boot I see the following Slab measurements in /proc/meminfo (median > > of 3 reboots): > > > > Before: 169020 kB > > After: 167304 kB > > Thanks for the improvement, Peter! > > Overall, the patch looks good to me: > > Reviewed-by: Andrey Konovalov Thanks! > While looking at the code, I noticed a couple of issues in the already > existing comments. Not sure if they are worth fixing, but I'll point > them out just in case. > > > Link: https://linux-review.googlesource.com/id/I752e725179b43b144153f4b6f584ceb646473ead > > Signed-off-by: Peter Collingbourne > > --- > > v2: > > - use max instead of max_t in flat_stack_align() > > > > arch/arc/include/asm/cache.h | 4 ++-- > > arch/arm/include/asm/cache.h | 2 +- > > arch/arm64/include/asm/cache.h | 19 +++++++++++++------ > > arch/microblaze/include/asm/page.h | 2 +- > > arch/riscv/include/asm/cache.h | 2 +- > > arch/sparc/include/asm/cache.h | 2 +- > > arch/xtensa/include/asm/processor.h | 2 +- > > fs/binfmt_flat.c | 9 ++++++--- > > include/crypto/hash.h | 2 +- > > include/linux/slab.h | 22 +++++++++++++++++----- > > mm/slab.c | 7 +++---- > > mm/slab_common.c | 3 +-- > > mm/slob.c | 6 +++--- > > 13 files changed, 51 insertions(+), 31 deletions(-) > > > > diff --git a/arch/arc/include/asm/cache.h b/arch/arc/include/asm/cache.h > > index f0f1fc5d62b6..b6a7763fd5d6 100644 > > --- a/arch/arc/include/asm/cache.h > > +++ b/arch/arc/include/asm/cache.h > > @@ -55,11 +55,11 @@ > > * Make sure slab-allocated buffers are 64-bit aligned when atomic64_t uses > > * ARCv2 64-bit atomics (LLOCKD/SCONDD). This guarantess runtime 64-bit > > * alignment for any atomic64_t embedded in buffer. > > - * Default ARCH_SLAB_MINALIGN is __alignof__(long long) which has a relaxed > > + * Default ARCH_SLAB_MIN_MINALIGN is __alignof__(long long) which has a relaxed > > * value of 4 (and not 8) in ARC ABI. > > */ > > #if defined(CONFIG_ARC_HAS_LL64) && defined(CONFIG_ARC_HAS_LLSC) > > -#define ARCH_SLAB_MINALIGN 8 > > +#define ARCH_SLAB_MIN_MINALIGN 8 > > #endif > > > > extern int ioc_enable; > > diff --git a/arch/arm/include/asm/cache.h b/arch/arm/include/asm/cache.h > > index e3ea34558ada..3e1018bb9805 100644 > > --- a/arch/arm/include/asm/cache.h > > +++ b/arch/arm/include/asm/cache.h > > @@ -21,7 +21,7 @@ > > * With EABI on ARMv5 and above we must have 64-bit aligned slab pointers. > > */ > > #if defined(CONFIG_AEABI) && (__LINUX_ARM_ARCH__ >= 5) > > -#define ARCH_SLAB_MINALIGN 8 > > +#define ARCH_SLAB_MIN_MINALIGN 8 > > #endif > > > > #define __read_mostly __section(".data..read_mostly") > > diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h > > index a074459f8f2f..38f171591c3f 100644 > > --- a/arch/arm64/include/asm/cache.h > > +++ b/arch/arm64/include/asm/cache.h > > @@ -6,6 +6,7 @@ > > #define __ASM_CACHE_H > > > > #include > > +#include > > > > #define CTR_L1IP_SHIFT 14 > > #define CTR_L1IP_MASK 3 > > @@ -49,15 +50,21 @@ > > */ > > #define ARCH_DMA_MINALIGN (128) > > > > -#ifdef CONFIG_KASAN_SW_TAGS > > -#define ARCH_SLAB_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT) > > -#elif defined(CONFIG_KASAN_HW_TAGS) > > -#define ARCH_SLAB_MINALIGN MTE_GRANULE_SIZE > > -#endif > > - > > #ifndef __ASSEMBLY__ > > > > #include > > +#include > > + > > +#ifdef CONFIG_KASAN_SW_TAGS > > +#define ARCH_SLAB_MIN_MINALIGN (1ULL << KASAN_SHADOW_SCALE_SHIFT) > > +#elif defined(CONFIG_KASAN_HW_TAGS) > > +static inline size_t arch_slab_minalign(void) > > +{ > > + return kasan_hw_tags_enabled() ? MTE_GRANULE_SIZE : > > + __alignof__(unsigned long long); > > +} > > +#define arch_slab_minalign() arch_slab_minalign() > > +#endif > > > > #define ICACHEF_ALIASING 0 > > #define ICACHEF_VPIPT 1 > > diff --git a/arch/microblaze/include/asm/page.h b/arch/microblaze/include/asm/page.h > > index 4b8b2fa78fc5..ccdbc1da3c3e 100644 > > --- a/arch/microblaze/include/asm/page.h > > +++ b/arch/microblaze/include/asm/page.h > > @@ -33,7 +33,7 @@ > > /* MS be sure that SLAB allocates aligned objects */ > > #define ARCH_DMA_MINALIGN L1_CACHE_BYTES > > > > -#define ARCH_SLAB_MINALIGN L1_CACHE_BYTES > > +#define ARCH_SLAB_MIN_MINALIGN L1_CACHE_BYTES > > > > /* > > * PAGE_OFFSET -- the first address of the first page of memory. With MMU > > diff --git a/arch/riscv/include/asm/cache.h b/arch/riscv/include/asm/cache.h > > index 9b58b104559e..7beb3b5d27c7 100644 > > --- a/arch/riscv/include/asm/cache.h > > +++ b/arch/riscv/include/asm/cache.h > > @@ -16,7 +16,7 @@ > > * the flat loader aligns it accordingly. > > */ > > #ifndef CONFIG_MMU > > -#define ARCH_SLAB_MINALIGN 16 > > +#define ARCH_SLAB_MIN_MINALIGN 16 > > #endif > > > > #endif /* _ASM_RISCV_CACHE_H */ > > diff --git a/arch/sparc/include/asm/cache.h b/arch/sparc/include/asm/cache.h > > index e62fd0e72606..9d8cb4687b7e 100644 > > --- a/arch/sparc/include/asm/cache.h > > +++ b/arch/sparc/include/asm/cache.h > > @@ -8,7 +8,7 @@ > > #ifndef _SPARC_CACHE_H > > #define _SPARC_CACHE_H > > > > -#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) > > +#define ARCH_SLAB_MIN_MINALIGN __alignof__(unsigned long long) > > > > #define L1_CACHE_SHIFT 5 > > #define L1_CACHE_BYTES 32 > > diff --git a/arch/xtensa/include/asm/processor.h b/arch/xtensa/include/asm/processor.h > > index 4489a27d527a..e3ea278e3fcf 100644 > > --- a/arch/xtensa/include/asm/processor.h > > +++ b/arch/xtensa/include/asm/processor.h > > @@ -18,7 +18,7 @@ > > #include > > #include > > > > -#define ARCH_SLAB_MINALIGN XTENSA_STACK_ALIGNMENT > > +#define ARCH_SLAB_MIN_MINALIGN XTENSA_STACK_ALIGNMENT > > > > /* > > * User space process size: 1 GB. > > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > > index 626898150011..23ce3439eafa 100644 > > --- a/fs/binfmt_flat.c > > +++ b/fs/binfmt_flat.c > > @@ -64,7 +64,10 @@ > > * Here we can be a bit looser than the data sections since this > > * needs to only meet arch ABI requirements. > > */ > > -#define FLAT_STACK_ALIGN max_t(unsigned long, sizeof(void *), ARCH_SLAB_MINALIGN) > > +static size_t flat_stack_align(void) > > +{ > > + return max(sizeof(void *), arch_slab_minalign()); > > +} > > > > #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ > > #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ > > @@ -148,7 +151,7 @@ static int create_flat_tables(struct linux_binprm *bprm, unsigned long arg_start > > sp -= 2; /* argvp + envp */ > > sp -= 1; /* &argc */ > > > > - current->mm->start_stack = (unsigned long)sp & -FLAT_STACK_ALIGN; > > + current->mm->start_stack = (unsigned long)sp & -flat_stack_align(); > > sp = (unsigned long __user *)current->mm->start_stack; > > > > if (put_user(bprm->argc, sp++)) > > @@ -966,7 +969,7 @@ static int load_flat_binary(struct linux_binprm *bprm) > > #endif > > stack_len += (bprm->argc + 1) * sizeof(char *); /* the argv array */ > > stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ > > - stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN); > > + stack_len = ALIGN(stack_len, flat_stack_align()); > > > > res = load_flat_file(bprm, &libinfo, 0, &stack_len); > > if (res < 0) > > diff --git a/include/crypto/hash.h b/include/crypto/hash.h > > index f140e4643949..442c290f458c 100644 > > --- a/include/crypto/hash.h > > +++ b/include/crypto/hash.h > > @@ -149,7 +149,7 @@ struct ahash_alg { > > > > struct shash_desc { > > struct crypto_shash *tfm; > > - void *__ctx[] __aligned(ARCH_SLAB_MINALIGN); > > + void *__ctx[] __aligned(ARCH_SLAB_MIN_MINALIGN); > > }; > > > > #define HASH_MAX_DIGESTSIZE 64 > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index 373b3ef99f4e..80e517593372 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -201,21 +201,33 @@ void kmem_dump_obj(void *object); > > #endif > > > > /* > > - * Setting ARCH_SLAB_MINALIGN in arch headers allows a different alignment. > > + * Setting ARCH_SLAB_MIN_MINALIGN in arch headers allows a different alignment. > > * Intended for arches that get misalignment faults even for 64 bit integer > > * aligned buffers. > > */ > > -#ifndef ARCH_SLAB_MINALIGN > > -#define ARCH_SLAB_MINALIGN __alignof__(unsigned long long) > > +#ifndef ARCH_SLAB_MIN_MINALIGN > > +#define ARCH_SLAB_MIN_MINALIGN __alignof__(unsigned long long) > > +#endif > > + > > +/* > > + * Arches can define this function if they want to decide the minimum slab > > + * alignment at runtime. The value returned by the function must be > > + * >= ARCH_SLAB_MIN_MINALIGN. > > + */ > > +#ifndef arch_slab_minalign > > +static inline size_t arch_slab_minalign(void) > > +{ > > + return ARCH_SLAB_MIN_MINALIGN; > > +} > > #endif > > > > /* > > * kmalloc and friends return ARCH_KMALLOC_MINALIGN aligned > > - * pointers. kmem_cache_alloc and friends return ARCH_SLAB_MINALIGN > > + * pointers. kmem_cache_alloc and friends return ARCH_SLAB_MIN_MINALIGN > > * aligned pointers. > > This comment is not precise: kmalloc relies on kmem_cache_alloc, so > kmalloc technically returns pointers aligned to > max(ARCH_KMALLOC_MINALIGN, ARCH_SLAB_MIN_MINALIGN). See > create_kmalloc_cache()->create_boot_cache()->calculate_alignment() for > SLAB and SLUB and __do_kmalloc_node() for SLOB. This alignment is > stronger than the one is specified for __assume_kmalloc_alignment > below, so the code should be fine. However, the comment is confusing. > > Also, the comment next to the ARCH_KMALLOC_MINALIGN definition says > "Setting ARCH_KMALLOC_MINALIGN in arch headers" while it should say > "Setting ARCH_DMA_MINALIGN in arch headers". Good catches. Let's fix these issues separately (especially since I reverted the ARCH_SLAB_MIN_MINALIGN change so the patches shouldn't conflict). Would you like to send patches? I can if not. Peter