From: Hyeonggon Yoo <42.hyeyoo@gmail.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: David Rientjes <rientjes@google.com>,
Christoph Lameter <cl@linux.com>,
Pekka Enberg <penberg@kernel.org>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
Andrew Morton <akpm@linux-foundation.org>,
Roman Gushchin <roman.gushchin@linux.dev>,
Andrey Ryabinin <ryabinin.a.a@gmail.com>,
Alexander Potapenko <glider@google.com>,
Andrey Konovalov <andreyknvl@gmail.com>,
Dmitry Vyukov <dvyukov@google.com>,
Vincenzo Frascino <vincenzo.frascino@arm.com>,
Marco Elver <elver@google.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Michal Hocko <mhocko@kernel.org>,
Shakeel Butt <shakeelb@google.com>,
Muchun Song <muchun.song@linux.dev>,
Kees Cook <keescook@chromium.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
kasan-dev@googlegroups.com, cgroups@vger.kernel.org,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2 13/21] mm/slab: move pre/post-alloc hooks from slab.h to slub.c
Date: Thu, 7 Dec 2023 09:43:04 +0900 [thread overview]
Message-ID: <ZXEVGNxKTNC6v5NR@localhost.localdomain> (raw)
In-Reply-To: <20231120-slab-remove-slab-v2-13-9c9c70177183@suse.cz>
On Mon, Nov 20, 2023 at 07:34:24PM +0100, Vlastimil Babka wrote:
> We don't share the hooks between two slab implementations anymore so
> they can be moved away from the header. As part of the move, also move
> should_failslab() from slab_common.c as the pre_alloc hook uses it.
> This means slab.h can stop including fault-inject.h and kmemleak.h.
> Fix up some files that were depending on the includes transitively.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/kasan/report.c | 1 +
> mm/memcontrol.c | 1 +
> mm/slab.h | 72 -------------------------------------------------
> mm/slab_common.c | 8 +-----
> mm/slub.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 84 insertions(+), 79 deletions(-)
>
> diff --git a/mm/kasan/report.c b/mm/kasan/report.c
> index e77facb62900..011f727bfaff 100644
> --- a/mm/kasan/report.c
> +++ b/mm/kasan/report.c
> @@ -23,6 +23,7 @@
> #include <linux/stacktrace.h>
> #include <linux/string.h>
> #include <linux/types.h>
> +#include <linux/vmalloc.h>
> #include <linux/kasan.h>
> #include <linux/module.h>
> #include <linux/sched/task_stack.h>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 947fb50eba31..8a0603517065 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -64,6 +64,7 @@
> #include <linux/psi.h>
> #include <linux/seq_buf.h>
> #include <linux/sched/isolation.h>
> +#include <linux/kmemleak.h>
> #include "internal.h"
> #include <net/sock.h>
> #include <net/ip.h>
> diff --git a/mm/slab.h b/mm/slab.h
> index 1ac3a2f8d4c0..65ebf86b3fe9 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -9,8 +9,6 @@
> #include <linux/kobject.h>
> #include <linux/sched/mm.h>
> #include <linux/memcontrol.h>
> -#include <linux/fault-inject.h>
> -#include <linux/kmemleak.h>
> #include <linux/kfence.h>
> #include <linux/kasan.h>
>
> @@ -796,76 +794,6 @@ static inline size_t slab_ksize(const struct kmem_cache *s)
> return s->size;
> }
>
> -static inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
> - struct list_lru *lru,
> - struct obj_cgroup **objcgp,
> - size_t size, gfp_t flags)
> -{
> - flags &= gfp_allowed_mask;
> -
> - might_alloc(flags);
> -
> - if (should_failslab(s, flags))
> - return NULL;
> -
> - if (!memcg_slab_pre_alloc_hook(s, lru, objcgp, size, flags))
> - return NULL;
> -
> - return s;
> -}
> -
> -static inline void slab_post_alloc_hook(struct kmem_cache *s,
> - struct obj_cgroup *objcg, gfp_t flags,
> - size_t size, void **p, bool init,
> - unsigned int orig_size)
> -{
> - unsigned int zero_size = s->object_size;
> - bool kasan_init = init;
> - size_t i;
> -
> - flags &= gfp_allowed_mask;
> -
> - /*
> - * For kmalloc object, the allocated memory size(object_size) is likely
> - * larger than the requested size(orig_size). If redzone check is
> - * enabled for the extra space, don't zero it, as it will be redzoned
> - * soon. The redzone operation for this extra space could be seen as a
> - * replacement of current poisoning under certain debug option, and
> - * won't break other sanity checks.
> - */
> - if (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) &&
> - (s->flags & SLAB_KMALLOC))
> - zero_size = orig_size;
> -
> - /*
> - * When slub_debug is enabled, avoid memory initialization integrated
> - * into KASAN and instead zero out the memory via the memset below with
> - * the proper size. Otherwise, KASAN might overwrite SLUB redzones and
> - * cause false-positive reports. This does not lead to a performance
> - * penalty on production builds, as slub_debug is not intended to be
> - * enabled there.
> - */
> - if (__slub_debug_enabled())
> - kasan_init = false;
> -
> - /*
> - * As memory initialization might be integrated into KASAN,
> - * kasan_slab_alloc and initialization memset must be
> - * kept together to avoid discrepancies in behavior.
> - *
> - * As p[i] might get tagged, memset and kmemleak hook come after KASAN.
> - */
> - for (i = 0; i < size; i++) {
> - p[i] = kasan_slab_alloc(s, p[i], flags, kasan_init);
> - if (p[i] && init && (!kasan_init || !kasan_has_integrated_init()))
> - memset(p[i], 0, zero_size);
> - kmemleak_alloc_recursive(p[i], s->object_size, 1,
> - s->flags, flags);
> - kmsan_slab_alloc(s, p[i], flags);
> - }
> -
> - memcg_slab_post_alloc_hook(s, objcg, flags, size, p);
> -}
>
> /*
> * The slab lists for all objects.
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 63b8411db7ce..bbc2e3f061f1 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -21,6 +21,7 @@
> #include <linux/swiotlb.h>
> #include <linux/proc_fs.h>
> #include <linux/debugfs.h>
> +#include <linux/kmemleak.h>
> #include <linux/kasan.h>
> #include <asm/cacheflush.h>
> #include <asm/tlbflush.h>
> @@ -1470,10 +1471,3 @@ EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc);
> EXPORT_TRACEPOINT_SYMBOL(kfree);
> EXPORT_TRACEPOINT_SYMBOL(kmem_cache_free);
>
> -int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
> -{
> - if (__should_failslab(s, gfpflags))
> - return -ENOMEM;
> - return 0;
> -}
> -ALLOW_ERROR_INJECTION(should_failslab, ERRNO);
> diff --git a/mm/slub.c b/mm/slub.c
> index 979932d046fd..9eb6508152c2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -34,6 +34,7 @@
> #include <linux/memory.h>
> #include <linux/math64.h>
> #include <linux/fault-inject.h>
> +#include <linux/kmemleak.h>
> #include <linux/stacktrace.h>
> #include <linux/prefetch.h>
> #include <linux/memcontrol.h>
> @@ -3494,6 +3495,86 @@ static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s,
> 0, sizeof(void *));
> }
>
> +noinline int should_failslab(struct kmem_cache *s, gfp_t gfpflags)
> +{
> + if (__should_failslab(s, gfpflags))
> + return -ENOMEM;
> + return 0;
> +}
> +ALLOW_ERROR_INJECTION(should_failslab, ERRNO);
> +
> +static inline struct kmem_cache *slab_pre_alloc_hook(struct kmem_cache *s,
> + struct list_lru *lru,
> + struct obj_cgroup **objcgp,
> + size_t size, gfp_t flags)
> +{
> + flags &= gfp_allowed_mask;
> +
> + might_alloc(flags);
> +
> + if (should_failslab(s, flags))
> + return NULL;
> +
> + if (!memcg_slab_pre_alloc_hook(s, lru, objcgp, size, flags))
> + return NULL;
> +
> + return s;
> +}
> +
> +static inline void slab_post_alloc_hook(struct kmem_cache *s,
> + struct obj_cgroup *objcg, gfp_t flags,
> + size_t size, void **p, bool init,
> + unsigned int orig_size)
> +{
> + unsigned int zero_size = s->object_size;
> + bool kasan_init = init;
> + size_t i;
> +
> + flags &= gfp_allowed_mask;
> +
> + /*
> + * For kmalloc object, the allocated memory size(object_size) is likely
> + * larger than the requested size(orig_size). If redzone check is
> + * enabled for the extra space, don't zero it, as it will be redzoned
> + * soon. The redzone operation for this extra space could be seen as a
> + * replacement of current poisoning under certain debug option, and
> + * won't break other sanity checks.
> + */
> + if (kmem_cache_debug_flags(s, SLAB_STORE_USER | SLAB_RED_ZONE) &&
> + (s->flags & SLAB_KMALLOC))
> + zero_size = orig_size;
> +
> + /*
> + * When slub_debug is enabled, avoid memory initialization integrated
> + * into KASAN and instead zero out the memory via the memset below with
> + * the proper size. Otherwise, KASAN might overwrite SLUB redzones and
> + * cause false-positive reports. This does not lead to a performance
> + * penalty on production builds, as slub_debug is not intended to be
> + * enabled there.
> + */
> + if (__slub_debug_enabled())
> + kasan_init = false;
> +
> + /*
> + * As memory initialization might be integrated into KASAN,
> + * kasan_slab_alloc and initialization memset must be
> + * kept together to avoid discrepancies in behavior.
> + *
> + * As p[i] might get tagged, memset and kmemleak hook come after KASAN.
> + */
> + for (i = 0; i < size; i++) {
> + p[i] = kasan_slab_alloc(s, p[i], flags, kasan_init);
> + if (p[i] && init && (!kasan_init ||
> + !kasan_has_integrated_init()))
> + memset(p[i], 0, zero_size);
> + kmemleak_alloc_recursive(p[i], s->object_size, 1,
> + s->flags, flags);
> + kmsan_slab_alloc(s, p[i], flags);
> + }
> +
> + memcg_slab_post_alloc_hook(s, objcg, flags, size, p);
> +}
> +
> /*
> * Inlined fastpath so that allocation functions (kmalloc, kmem_cache_alloc)
> * have the fastpath folded into their functions. So no function call
>
> --
Looks good to me,
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> 2.42.1
>
>
next prev parent reply other threads:[~2023-12-07 0:43 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-20 18:34 [PATCH v2 00/21] remove the SLAB allocator Vlastimil Babka
2023-11-20 18:34 ` [PATCH v2 01/21] mm/slab, docs: switch mm-api docs generation from slab.c to slub.c Vlastimil Babka
2023-11-24 0:46 ` David Rientjes
2023-12-05 3:53 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 02/21] mm/slab: remove CONFIG_SLAB from all Kconfig and Makefile Vlastimil Babka
2023-12-05 4:15 ` Hyeonggon Yoo
2023-12-05 10:14 ` Vlastimil Babka
2023-12-06 0:08 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 03/21] KASAN: remove code paths guarded by CONFIG_SLAB Vlastimil Babka
2023-11-21 8:23 ` Hyeonggon Yoo
2023-11-21 16:47 ` Andrey Konovalov
2023-12-05 4:26 ` Hyeonggon Yoo
2023-12-05 4:48 ` Hyeonggon Yoo
2023-12-05 10:16 ` Vlastimil Babka
2023-11-20 18:34 ` [PATCH v2 04/21] KFENCE: cleanup kfence_guarded_alloc() after CONFIG_SLAB removal Vlastimil Babka
2023-12-06 8:01 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 05/21] mm/memcontrol: remove CONFIG_SLAB #ifdef guards Vlastimil Babka
2023-12-06 8:12 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 06/21] cpu/hotplug: remove CPUHP_SLAB_PREPARE hooks Vlastimil Babka
2023-12-01 11:28 ` Thomas Gleixner
2023-12-06 8:28 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 07/21] mm/slab: remove CONFIG_SLAB code from slab common code Vlastimil Babka
2023-12-06 9:05 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 08/21] mm/mempool/dmapool: remove CONFIG_DEBUG_SLAB ifdefs Vlastimil Babka
2023-12-06 9:10 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 09/21] mm/slab: remove mm/slab.c and slab_def.h Vlastimil Babka
2023-11-22 20:07 ` Christoph Lameter
2023-12-06 9:31 ` Hyeonggon Yoo
2023-12-06 9:37 ` Vlastimil Babka
2023-11-20 18:34 ` [PATCH v2 10/21] mm/slab: move struct kmem_cache_cpu declaration to slub.c Vlastimil Babka
2023-12-06 9:35 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 11/21] mm/slab: move the rest of slub_def.h to mm/slab.h Vlastimil Babka
2023-12-06 9:45 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 12/21] mm/slab: consolidate includes in the internal mm/slab.h Vlastimil Babka
2023-12-07 0:30 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 13/21] mm/slab: move pre/post-alloc hooks from slab.h to slub.c Vlastimil Babka
2023-12-07 0:43 ` Hyeonggon Yoo [this message]
2023-11-20 18:34 ` [PATCH v2 14/21] mm/slab: move memcg related functions " Vlastimil Babka
2023-12-07 0:59 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 15/21] mm/slab: move struct kmem_cache_node " Vlastimil Babka
2023-12-07 1:11 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 16/21] mm/slab: move kfree() from slab_common.c " Vlastimil Babka
2023-12-05 4:38 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 17/21] mm/slab: move kmalloc_slab() to mm/slab.h Vlastimil Babka
2023-12-07 1:28 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 18/21] mm/slab: move kmalloc() functions from slab_common.c to slub.c Vlastimil Babka
2023-12-07 1:30 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 19/21] mm/slub: remove slab_alloc() and __kmem_cache_alloc_lru() wrappers Vlastimil Babka
2023-12-07 1:35 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 20/21] mm/slub: optimize alloc fastpath code layout Vlastimil Babka
2023-12-07 2:32 ` Hyeonggon Yoo
2023-11-20 18:34 ` [PATCH v2 21/21] mm/slub: optimize free fast path " Vlastimil Babka
2023-12-07 2:40 ` Hyeonggon Yoo
2023-11-24 0:45 ` [PATCH v2 00/21] remove the SLAB allocator David Rientjes
2023-11-24 9:26 ` Vlastimil Babka
2023-12-07 2:45 ` Hyeonggon Yoo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZXEVGNxKTNC6v5NR@localhost.localdomain \
--to=42.hyeyoo@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andreyknvl@gmail.com \
--cc=cgroups@vger.kernel.org \
--cc=cl@linux.com \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=glider@google.com \
--cc=hannes@cmpxchg.org \
--cc=iamjoonsoo.kim@lge.com \
--cc=kasan-dev@googlegroups.com \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=muchun.song@linux.dev \
--cc=penberg@kernel.org \
--cc=rientjes@google.com \
--cc=roman.gushchin@linux.dev \
--cc=ryabinin.a.a@gmail.com \
--cc=shakeelb@google.com \
--cc=vbabka@suse.cz \
--cc=vincenzo.frascino@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.