2024-02-20 17:01:20

by Vlastimil Babka

[permalink] [raw]
Subject: [PATCH 3/3] mm, slab, kasan: replace kasan_never_merge() with SLAB_NO_MERGE

The SLAB_KASAN flag prevents merging of caches in some configurations,
which is handled in a rather complicated way via kasan_never_merge().
Since we now have a generic SLAB_NO_MERGE flag, we can instead use it
for KASAN caches in addition to SLAB_KASAN in those configurations,
and simplify the SLAB_NEVER_MERGE handling.

Signed-off-by: Vlastimil Babka <[email protected]>
---
include/linux/kasan.h | 6 ------
mm/kasan/generic.c | 16 ++++------------
mm/slab_common.c | 2 +-
3 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index dbb06d789e74..70d6a8f6e25d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -429,7 +429,6 @@ struct kasan_cache {
};

size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object);
-slab_flags_t kasan_never_merge(void);
void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
slab_flags_t *flags);

@@ -446,11 +445,6 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache,
{
return 0;
}
-/* And thus nothing prevents cache merging. */
-static inline slab_flags_t kasan_never_merge(void)
-{
- return 0;
-}
/* And no cache-related metadata initialization is required. */
static inline void kasan_cache_create(struct kmem_cache *cache,
unsigned int *size,
diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
index df6627f62402..d8b78d273b9f 100644
--- a/mm/kasan/generic.c
+++ b/mm/kasan/generic.c
@@ -334,14 +334,6 @@ DEFINE_ASAN_SET_SHADOW(f3);
DEFINE_ASAN_SET_SHADOW(f5);
DEFINE_ASAN_SET_SHADOW(f8);

-/* Only allow cache merging when no per-object metadata is present. */
-slab_flags_t kasan_never_merge(void)
-{
- if (!kasan_requires_meta())
- return 0;
- return SLAB_KASAN;
-}
-
/*
* Adaptive redzone policy taken from the userspace AddressSanitizer runtime.
* For larger allocations larger redzones are used.
@@ -372,13 +364,13 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
/*
* SLAB_KASAN is used to mark caches that are sanitized by KASAN
* and that thus have per-object metadata.
- * Currently this flag is used in two places:
+ * Currently this flag is used in one place:
* 1. In slab_ksize() to account for per-object metadata when
* calculating the size of the accessible memory within the object.
- * 2. In slab_common.c via kasan_never_merge() to prevent merging of
- * caches with per-object metadata.
+ * Additionally, we use SLAB_NO_MERGE to prevent merging of caches
+ * with per-object metadata.
*/
- *flags |= SLAB_KASAN;
+ *flags |= SLAB_KASAN | SLAB_NO_MERGE;

ok_size = *size;

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 238293b1dbe1..7cfa2f1ce655 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -50,7 +50,7 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
*/
#define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \
- SLAB_FAILSLAB | SLAB_NO_MERGE | kasan_never_merge())
+ SLAB_FAILSLAB | SLAB_NO_MERGE)

#define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
SLAB_CACHE_DMA32 | SLAB_ACCOUNT)

--
2.43.1



2024-02-21 02:36:51

by Song, Xiongwei

[permalink] [raw]
Subject: RE: [PATCH 3/3] mm, slab, kasan: replace kasan_never_merge() with SLAB_NO_MERGE


> The SLAB_KASAN flag prevents merging of caches in some configurations,
> which is handled in a rather complicated way via kasan_never_merge().
> Since we now have a generic SLAB_NO_MERGE flag, we can instead use it
> for KASAN caches in addition to SLAB_KASAN in those configurations,
> and simplify the SLAB_NEVER_MERGE handling.
>
> Signed-off-by: Vlastimil Babka <[email protected]>

Ran a rough test with build and bootup with CONFIG_KASAN_GENERIC enabled,
feel free to add

Tested-by: Xiongwei Song <[email protected]>

Thanks,
Xiongwei

> ---
> include/linux/kasan.h | 6 ------
> mm/kasan/generic.c | 16 ++++------------
> mm/slab_common.c | 2 +-
> 3 files changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index dbb06d789e74..70d6a8f6e25d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -429,7 +429,6 @@ struct kasan_cache {
> };
>
> size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object);
> -slab_flags_t kasan_never_merge(void);
> void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> slab_flags_t *flags);
>
> @@ -446,11 +445,6 @@ static inline size_t kasan_metadata_size(struct kmem_cache
> *cache,
> {
> return 0;
> }
> -/* And thus nothing prevents cache merging. */
> -static inline slab_flags_t kasan_never_merge(void)
> -{
> - return 0;
> -}
> /* And no cache-related metadata initialization is required. */
> static inline void kasan_cache_create(struct kmem_cache *cache,
> unsigned int *size,
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index df6627f62402..d8b78d273b9f 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -334,14 +334,6 @@ DEFINE_ASAN_SET_SHADOW(f3);
> DEFINE_ASAN_SET_SHADOW(f5);
> DEFINE_ASAN_SET_SHADOW(f8);
>
> -/* Only allow cache merging when no per-object metadata is present. */
> -slab_flags_t kasan_never_merge(void)
> -{
> - if (!kasan_requires_meta())
> - return 0;
> - return SLAB_KASAN;
> -}
> -
> /*
> * Adaptive redzone policy taken from the userspace AddressSanitizer runtime.
> * For larger allocations larger redzones are used.
> @@ -372,13 +364,13 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned
> int *size,
> /*
> * SLAB_KASAN is used to mark caches that are sanitized by KASAN
> * and that thus have per-object metadata.
> - * Currently this flag is used in two places:
> + * Currently this flag is used in one place:
> * 1. In slab_ksize() to account for per-object metadata when
> * calculating the size of the accessible memory within the object.
> - * 2. In slab_common.c via kasan_never_merge() to prevent merging of
> - * caches with per-object metadata.
> + * Additionally, we use SLAB_NO_MERGE to prevent merging of caches
> + * with per-object metadata.
> */
> - *flags |= SLAB_KASAN;
> + *flags |= SLAB_KASAN | SLAB_NO_MERGE;
>
> ok_size = *size;
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 238293b1dbe1..7cfa2f1ce655 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -50,7 +50,7 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
> */
> #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
> SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \
> - SLAB_FAILSLAB | SLAB_NO_MERGE | kasan_never_merge())
> + SLAB_FAILSLAB | SLAB_NO_MERGE)
>
> #define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
> SLAB_CACHE_DMA32 | SLAB_ACCOUNT)
>
> --
> 2.43.1

2024-02-21 07:22:23

by Chengming Zhou

[permalink] [raw]
Subject: Re: [PATCH 3/3] mm, slab, kasan: replace kasan_never_merge() with SLAB_NO_MERGE

On 2024/2/21 00:58, Vlastimil Babka wrote:
> The SLAB_KASAN flag prevents merging of caches in some configurations,
> which is handled in a rather complicated way via kasan_never_merge().
> Since we now have a generic SLAB_NO_MERGE flag, we can instead use it
> for KASAN caches in addition to SLAB_KASAN in those configurations,
> and simplify the SLAB_NEVER_MERGE handling.
>
> Signed-off-by: Vlastimil Babka <[email protected]>

Reviewed-by: Chengming Zhou <[email protected]>

Thanks!

> ---
> include/linux/kasan.h | 6 ------
> mm/kasan/generic.c | 16 ++++------------
> mm/slab_common.c | 2 +-
> 3 files changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index dbb06d789e74..70d6a8f6e25d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -429,7 +429,6 @@ struct kasan_cache {
> };
>
> size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object);
> -slab_flags_t kasan_never_merge(void);
> void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> slab_flags_t *flags);
>
> @@ -446,11 +445,6 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache,
> {
> return 0;
> }
> -/* And thus nothing prevents cache merging. */
> -static inline slab_flags_t kasan_never_merge(void)
> -{
> - return 0;
> -}
> /* And no cache-related metadata initialization is required. */
> static inline void kasan_cache_create(struct kmem_cache *cache,
> unsigned int *size,
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index df6627f62402..d8b78d273b9f 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -334,14 +334,6 @@ DEFINE_ASAN_SET_SHADOW(f3);
> DEFINE_ASAN_SET_SHADOW(f5);
> DEFINE_ASAN_SET_SHADOW(f8);
>
> -/* Only allow cache merging when no per-object metadata is present. */
> -slab_flags_t kasan_never_merge(void)
> -{
> - if (!kasan_requires_meta())
> - return 0;
> - return SLAB_KASAN;
> -}
> -
> /*
> * Adaptive redzone policy taken from the userspace AddressSanitizer runtime.
> * For larger allocations larger redzones are used.
> @@ -372,13 +364,13 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> /*
> * SLAB_KASAN is used to mark caches that are sanitized by KASAN
> * and that thus have per-object metadata.
> - * Currently this flag is used in two places:
> + * Currently this flag is used in one place:
> * 1. In slab_ksize() to account for per-object metadata when
> * calculating the size of the accessible memory within the object.
> - * 2. In slab_common.c via kasan_never_merge() to prevent merging of
> - * caches with per-object metadata.
> + * Additionally, we use SLAB_NO_MERGE to prevent merging of caches
> + * with per-object metadata.
> */
> - *flags |= SLAB_KASAN;
> + *flags |= SLAB_KASAN | SLAB_NO_MERGE;
>
> ok_size = *size;
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 238293b1dbe1..7cfa2f1ce655 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -50,7 +50,7 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
> */
> #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
> SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \
> - SLAB_FAILSLAB | SLAB_NO_MERGE | kasan_never_merge())
> + SLAB_FAILSLAB | SLAB_NO_MERGE)
>
> #define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
> SLAB_CACHE_DMA32 | SLAB_ACCOUNT)
>

2024-02-21 20:50:37

by Andrey Konovalov

[permalink] [raw]
Subject: Re: [PATCH 3/3] mm, slab, kasan: replace kasan_never_merge() with SLAB_NO_MERGE

On Tue, Feb 20, 2024 at 5:58 PM Vlastimil Babka <[email protected]> wrote:
>
> The SLAB_KASAN flag prevents merging of caches in some configurations,
> which is handled in a rather complicated way via kasan_never_merge().
> Since we now have a generic SLAB_NO_MERGE flag, we can instead use it
> for KASAN caches in addition to SLAB_KASAN in those configurations,
> and simplify the SLAB_NEVER_MERGE handling.
>
> Signed-off-by: Vlastimil Babka <[email protected]>
> ---
> include/linux/kasan.h | 6 ------
> mm/kasan/generic.c | 16 ++++------------
> mm/slab_common.c | 2 +-
> 3 files changed, 5 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index dbb06d789e74..70d6a8f6e25d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -429,7 +429,6 @@ struct kasan_cache {
> };
>
> size_t kasan_metadata_size(struct kmem_cache *cache, bool in_object);
> -slab_flags_t kasan_never_merge(void);
> void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> slab_flags_t *flags);
>
> @@ -446,11 +445,6 @@ static inline size_t kasan_metadata_size(struct kmem_cache *cache,
> {
> return 0;
> }
> -/* And thus nothing prevents cache merging. */
> -static inline slab_flags_t kasan_never_merge(void)
> -{
> - return 0;
> -}
> /* And no cache-related metadata initialization is required. */
> static inline void kasan_cache_create(struct kmem_cache *cache,
> unsigned int *size,
> diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c
> index df6627f62402..d8b78d273b9f 100644
> --- a/mm/kasan/generic.c
> +++ b/mm/kasan/generic.c
> @@ -334,14 +334,6 @@ DEFINE_ASAN_SET_SHADOW(f3);
> DEFINE_ASAN_SET_SHADOW(f5);
> DEFINE_ASAN_SET_SHADOW(f8);
>
> -/* Only allow cache merging when no per-object metadata is present. */
> -slab_flags_t kasan_never_merge(void)
> -{
> - if (!kasan_requires_meta())
> - return 0;
> - return SLAB_KASAN;
> -}
> -
> /*
> * Adaptive redzone policy taken from the userspace AddressSanitizer runtime.
> * For larger allocations larger redzones are used.
> @@ -372,13 +364,13 @@ void kasan_cache_create(struct kmem_cache *cache, unsigned int *size,
> /*
> * SLAB_KASAN is used to mark caches that are sanitized by KASAN
> * and that thus have per-object metadata.
> - * Currently this flag is used in two places:
> + * Currently this flag is used in one place:
> * 1. In slab_ksize() to account for per-object metadata when
> * calculating the size of the accessible memory within the object.
> - * 2. In slab_common.c via kasan_never_merge() to prevent merging of
> - * caches with per-object metadata.

Let's reword this to:

SLAB_KASAN is used to mark caches that are sanitized by KASAN and that
thus have per-object metadata. Currently, this flag is used in
slab_ksize() to account for per-object metadata when calculating the
size of the accessible memory within the object.

> + * Additionally, we use SLAB_NO_MERGE to prevent merging of caches
> + * with per-object metadata.
> */
> - *flags |= SLAB_KASAN;
> + *flags |= SLAB_KASAN | SLAB_NO_MERGE;
>
> ok_size = *size;
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 238293b1dbe1..7cfa2f1ce655 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -50,7 +50,7 @@ static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
> */
> #define SLAB_NEVER_MERGE (SLAB_RED_ZONE | SLAB_POISON | SLAB_STORE_USER | \
> SLAB_TRACE | SLAB_TYPESAFE_BY_RCU | SLAB_NOLEAKTRACE | \
> - SLAB_FAILSLAB | SLAB_NO_MERGE | kasan_never_merge())
> + SLAB_FAILSLAB | SLAB_NO_MERGE)
>
> #define SLAB_MERGE_SAME (SLAB_RECLAIM_ACCOUNT | SLAB_CACHE_DMA | \
> SLAB_CACHE_DMA32 | SLAB_ACCOUNT)
>
> --
> 2.43.1
>

Otherwise, looks good to me.

Reviewed-by: Andrey Konovalov <[email protected]>

Thanks!