diff mbox series

[mm,v3,28/38] kasan, page_alloc: allow skipping memory init for HW_TAGS

Message ID cd8667450f7a0daf6b4081276e11a5f7bed60128.1639432170.git.andreyknvl@google.com (mailing list archive)
State New
Headers show
Series kasan, vmalloc, arm64: add vmalloc tagging support for SW/HW_TAGS | expand

Commit Message

andrey.konovalov@linux.dev Dec. 13, 2021, 9:54 p.m. UTC 
From: Andrey Konovalov <andreyknvl@google.com>

Add a new GFP flag __GFP_SKIP_ZERO that allows to skip memory
initialization. The flag is only effective with HW_TAGS KASAN.

This flag will be used by vmalloc code for page_alloc allocations
backing vmalloc() mappings in a following patch. The reason to skip
memory initialization for these pages in page_alloc is because vmalloc
code will be initializing them instead.

With the current implementation, when __GFP_SKIP_ZERO is provided,
__GFP_ZEROTAGS is ignored. This doesn't matter, as these two flags are
never provided at the same time. However, if this is changed in the
future, this particular implementation detail can be changed as well.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>

---

Changes v2->v3:
- Update patch description.

Changes v1->v2:
- Add this patch.
---
 include/linux/gfp.h | 16 +++++++++++-----
 mm/page_alloc.c     | 13 ++++++++++++-
 2 files changed, 23 insertions(+), 6 deletions(-)

Comments

Marco Elver Dec. 14, 2021, 6 p.m. UTC | #1 
On Mon, Dec 13, 2021 at 10:54PM +0100, andrey.konovalov@linux.dev wrote:
> From: Andrey Konovalov <andreyknvl@google.com>
> 
> Add a new GFP flag __GFP_SKIP_ZERO that allows to skip memory
> initialization. The flag is only effective with HW_TAGS KASAN.
[...]
> - * is being zeroed (either via __GFP_ZERO or via init_on_alloc).
> + * is being zeroed (either via __GFP_ZERO or via init_on_alloc, provided that
> + * __GFP_SKIP_ZERO is not set).
> + *
> + * %__GFP_SKIP_ZERO makes page_alloc skip zeroing memory.
> + * Only effective when HW_TAGS KASAN is enabled.
>   *
>   * %__GFP_SKIP_KASAN_UNPOISON makes KASAN skip unpoisoning on page allocation.
>   * Only effective in HW_TAGS mode.
> @@ -242,6 +247,7 @@ struct vm_area_struct;
>  #define __GFP_COMP	((__force gfp_t)___GFP_COMP)
>  #define __GFP_ZERO	((__force gfp_t)___GFP_ZERO)
>  #define __GFP_ZEROTAGS	((__force gfp_t)___GFP_ZEROTAGS)
> +#define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
>  #define __GFP_SKIP_KASAN_UNPOISON ((__force gfp_t)___GFP_SKIP_KASAN_UNPOISON)
>  #define __GFP_SKIP_KASAN_POISON   ((__force gfp_t)___GFP_SKIP_KASAN_POISON)
>  
> @@ -249,7 +255,7 @@ struct vm_area_struct;
>  #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
>  
>  /* Room for N __GFP_FOO bits */
> -#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
> +#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))

You're adding several new flags, I think you should also make a
corresponding change to include/trace/events/mmflags.h?

At least __GFP_SKIP_KASAN_POISON is currently in there.
Andrey Konovalov Dec. 14, 2021, 6:28 p.m. UTC | #2 
On Tue, Dec 14, 2021 at 7:00 PM Marco Elver <elver@google.com> wrote:
>
> On Mon, Dec 13, 2021 at 10:54PM +0100, andrey.konovalov@linux.dev wrote:
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Add a new GFP flag __GFP_SKIP_ZERO that allows to skip memory
> > initialization. The flag is only effective with HW_TAGS KASAN.
> [...]
> > - * is being zeroed (either via __GFP_ZERO or via init_on_alloc).
> > + * is being zeroed (either via __GFP_ZERO or via init_on_alloc, provided that
> > + * __GFP_SKIP_ZERO is not set).
> > + *
> > + * %__GFP_SKIP_ZERO makes page_alloc skip zeroing memory.
> > + * Only effective when HW_TAGS KASAN is enabled.
> >   *
> >   * %__GFP_SKIP_KASAN_UNPOISON makes KASAN skip unpoisoning on page allocation.
> >   * Only effective in HW_TAGS mode.
> > @@ -242,6 +247,7 @@ struct vm_area_struct;
> >  #define __GFP_COMP   ((__force gfp_t)___GFP_COMP)
> >  #define __GFP_ZERO   ((__force gfp_t)___GFP_ZERO)
> >  #define __GFP_ZEROTAGS       ((__force gfp_t)___GFP_ZEROTAGS)
> > +#define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
> >  #define __GFP_SKIP_KASAN_UNPOISON ((__force gfp_t)___GFP_SKIP_KASAN_UNPOISON)
> >  #define __GFP_SKIP_KASAN_POISON   ((__force gfp_t)___GFP_SKIP_KASAN_POISON)
> >
> > @@ -249,7 +255,7 @@ struct vm_area_struct;
> >  #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
> >
> >  /* Room for N __GFP_FOO bits */
> > -#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
> > +#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))
>
> You're adding several new flags, I think you should also make a
> corresponding change to include/trace/events/mmflags.h?
>
> At least __GFP_SKIP_KASAN_POISON is currently in there.

Indeed, will fix in v4. Thanks!
Kuan-Ying Lee (李冠穎) Dec. 17, 2021, 1:50 a.m. UTC | #3 
On Tue, 2021-12-14 at 05:54 +0800, andrey.konovalov@linux.dev wrote:
> From: Andrey Konovalov <andreyknvl@google.com>
> 
> Add a new GFP flag __GFP_SKIP_ZERO that allows to skip memory
> initialization. The flag is only effective with HW_TAGS KASAN.
> 
> This flag will be used by vmalloc code for page_alloc allocations
> backing vmalloc() mappings in a following patch. The reason to skip
> memory initialization for these pages in page_alloc is because
> vmalloc
> code will be initializing them instead.
> 
> With the current implementation, when __GFP_SKIP_ZERO is provided,
> __GFP_ZEROTAGS is ignored. This doesn't matter, as these two flags
> are
> never provided at the same time. However, if this is changed in the
> future, this particular implementation detail can be changed as well.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> 
> ---
> 
> Changes v2->v3:
> - Update patch description.
> 
> Changes v1->v2:
> - Add this patch.
> ---
>  include/linux/gfp.h | 16 +++++++++++-----
>  mm/page_alloc.c     | 13 ++++++++++++-
>  2 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 6781f84345d1..b8b1a7198186 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -54,10 +54,11 @@ struct vm_area_struct;
>  #define ___GFP_THISNODE		0x200000u
>  #define ___GFP_ACCOUNT		0x400000u
>  #define ___GFP_ZEROTAGS		0x800000u
> -#define ___GFP_SKIP_KASAN_UNPOISON	0x1000000u
> -#define ___GFP_SKIP_KASAN_POISON	0x2000000u
> +#define ___GFP_SKIP_ZERO	0x1000000u
> +#define ___GFP_SKIP_KASAN_UNPOISON	0x2000000u
> +#define ___GFP_SKIP_KASAN_POISON	0x4000000u
>  #ifdef CONFIG_LOCKDEP
> -#define ___GFP_NOLOCKDEP	0x4000000u
> +#define ___GFP_NOLOCKDEP	0x8000000u
>  #else
>  #define ___GFP_NOLOCKDEP	0
>  #endif
> @@ -230,7 +231,11 @@ struct vm_area_struct;
>   * %__GFP_ZERO returns a zeroed page on success.
>   *
>   * %__GFP_ZEROTAGS zeroes memory tags at allocation time if the
> memory itself
> - * is being zeroed (either via __GFP_ZERO or via init_on_alloc).
> + * is being zeroed (either via __GFP_ZERO or via init_on_alloc,
> provided that
> + * __GFP_SKIP_ZERO is not set).
> + *
> + * %__GFP_SKIP_ZERO makes page_alloc skip zeroing memory.
> + * Only effective when HW_TAGS KASAN is enabled.
>   *
>   * %__GFP_SKIP_KASAN_UNPOISON makes KASAN skip unpoisoning on page
> allocation.
>   * Only effective in HW_TAGS mode.
> @@ -242,6 +247,7 @@ struct vm_area_struct;
>  #define __GFP_COMP	((__force gfp_t)___GFP_COMP)
>  #define __GFP_ZERO	((__force gfp_t)___GFP_ZERO)
>  #define __GFP_ZEROTAGS	((__force gfp_t)___GFP_ZEROTAGS)
> +#define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
>  #define __GFP_SKIP_KASAN_UNPOISON ((__force
> gfp_t)___GFP_SKIP_KASAN_UNPOISON)
>  #define __GFP_SKIP_KASAN_POISON   ((__force
> gfp_t)___GFP_SKIP_KASAN_POISON)
>  
> @@ -249,7 +255,7 @@ struct vm_area_struct;
>  #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
>  
>  /* Room for N __GFP_FOO bits */
> -#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
> +#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))
>  #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) -
> 1))
>  
>  /**
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f1d5b80591c4..af7516a2d5ea 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2409,10 +2409,21 @@ static inline bool
> should_skip_kasan_unpoison(gfp_t flags, bool init_tags)
>  	return init_tags || (flags & __GFP_SKIP_KASAN_UNPOISON);
>  }
>  
> +static inline bool should_skip_init(gfp_t flags)
> +{
> +	/* Don't skip if a software KASAN mode is enabled. */
> +	if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
> +		return false;
> +

Hi Andrey,

Should we use kasan_hw_tags_enabled() in should_skip_init() function
instead of checking the config?

I think we should handle the condition which is CONFIG_KASAN_HW_TAGS=y
and command line="kasan=off".


> +	/* For hardware tag-based KASAN, skip if requested. */
> +	return (flags & __GFP_SKIP_ZERO);
> +}
> +
>  inline void post_alloc_hook(struct page *page, unsigned int order,
>  				gfp_t gfp_flags)
>  {
> -	bool init = !want_init_on_free() &&
> want_init_on_alloc(gfp_flags);
> +	bool init = !want_init_on_free() &&
> want_init_on_alloc(gfp_flags) &&
> +			!should_skip_init(gfp_flags);
>  	bool init_tags = init && (gfp_flags & __GFP_ZEROTAGS);
>  
>  	set_page_private(page, 0);
> -- 
> 2.25.1
> 
>
Andrey Konovalov Dec. 20, 2021, 9:35 p.m. UTC | #4 
On Fri, Dec 17, 2021 at 2:50 AM Kuan-Ying Lee
<Kuan-Ying.Lee@mediatek.com> wrote:
>
> On Tue, 2021-12-14 at 05:54 +0800, andrey.konovalov@linux.dev wrote:
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Add a new GFP flag __GFP_SKIP_ZERO that allows to skip memory
> > initialization. The flag is only effective with HW_TAGS KASAN.
> >
> > This flag will be used by vmalloc code for page_alloc allocations
> > backing vmalloc() mappings in a following patch. The reason to skip
> > memory initialization for these pages in page_alloc is because
> > vmalloc
> > code will be initializing them instead.
> >
> > With the current implementation, when __GFP_SKIP_ZERO is provided,
> > __GFP_ZEROTAGS is ignored. This doesn't matter, as these two flags
> > are
> > never provided at the same time. However, if this is changed in the
> > future, this particular implementation detail can be changed as well.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> >
> > ---
> >
> > Changes v2->v3:
> > - Update patch description.
> >
> > Changes v1->v2:
> > - Add this patch.
> > ---
> >  include/linux/gfp.h | 16 +++++++++++-----
> >  mm/page_alloc.c     | 13 ++++++++++++-
> >  2 files changed, 23 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 6781f84345d1..b8b1a7198186 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -54,10 +54,11 @@ struct vm_area_struct;
> >  #define ___GFP_THISNODE              0x200000u
> >  #define ___GFP_ACCOUNT               0x400000u
> >  #define ___GFP_ZEROTAGS              0x800000u
> > -#define ___GFP_SKIP_KASAN_UNPOISON   0x1000000u
> > -#define ___GFP_SKIP_KASAN_POISON     0x2000000u
> > +#define ___GFP_SKIP_ZERO     0x1000000u
> > +#define ___GFP_SKIP_KASAN_UNPOISON   0x2000000u
> > +#define ___GFP_SKIP_KASAN_POISON     0x4000000u
> >  #ifdef CONFIG_LOCKDEP
> > -#define ___GFP_NOLOCKDEP     0x4000000u
> > +#define ___GFP_NOLOCKDEP     0x8000000u
> >  #else
> >  #define ___GFP_NOLOCKDEP     0
> >  #endif
> > @@ -230,7 +231,11 @@ struct vm_area_struct;
> >   * %__GFP_ZERO returns a zeroed page on success.
> >   *
> >   * %__GFP_ZEROTAGS zeroes memory tags at allocation time if the
> > memory itself
> > - * is being zeroed (either via __GFP_ZERO or via init_on_alloc).
> > + * is being zeroed (either via __GFP_ZERO or via init_on_alloc,
> > provided that
> > + * __GFP_SKIP_ZERO is not set).
> > + *
> > + * %__GFP_SKIP_ZERO makes page_alloc skip zeroing memory.
> > + * Only effective when HW_TAGS KASAN is enabled.
> >   *
> >   * %__GFP_SKIP_KASAN_UNPOISON makes KASAN skip unpoisoning on page
> > allocation.
> >   * Only effective in HW_TAGS mode.
> > @@ -242,6 +247,7 @@ struct vm_area_struct;
> >  #define __GFP_COMP   ((__force gfp_t)___GFP_COMP)
> >  #define __GFP_ZERO   ((__force gfp_t)___GFP_ZERO)
> >  #define __GFP_ZEROTAGS       ((__force gfp_t)___GFP_ZEROTAGS)
> > +#define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
> >  #define __GFP_SKIP_KASAN_UNPOISON ((__force
> > gfp_t)___GFP_SKIP_KASAN_UNPOISON)
> >  #define __GFP_SKIP_KASAN_POISON   ((__force
> > gfp_t)___GFP_SKIP_KASAN_POISON)
> >
> > @@ -249,7 +255,7 @@ struct vm_area_struct;
> >  #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
> >
> >  /* Room for N __GFP_FOO bits */
> > -#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
> > +#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))
> >  #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) -
> > 1))
> >
> >  /**
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index f1d5b80591c4..af7516a2d5ea 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -2409,10 +2409,21 @@ static inline bool
> > should_skip_kasan_unpoison(gfp_t flags, bool init_tags)
> >       return init_tags || (flags & __GFP_SKIP_KASAN_UNPOISON);
> >  }
> >
> > +static inline bool should_skip_init(gfp_t flags)
> > +{
> > +     /* Don't skip if a software KASAN mode is enabled. */
> > +     if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
> > +             return false;
> > +
>
> Hi Andrey,
>
> Should we use kasan_hw_tags_enabled() in should_skip_init() function
> instead of checking the config?
>
> I think we should handle the condition which is CONFIG_KASAN_HW_TAGS=y
> and command line="kasan=off".

Hi Kuan-Ying,

You are right! Will fix in v4.

Thanks!
diff mbox series

Patch

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6781f84345d1..b8b1a7198186 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -54,10 +54,11 @@  struct vm_area_struct;
 #define ___GFP_THISNODE		0x200000u
 #define ___GFP_ACCOUNT		0x400000u
 #define ___GFP_ZEROTAGS		0x800000u
-#define ___GFP_SKIP_KASAN_UNPOISON	0x1000000u
-#define ___GFP_SKIP_KASAN_POISON	0x2000000u
+#define ___GFP_SKIP_ZERO	0x1000000u
+#define ___GFP_SKIP_KASAN_UNPOISON	0x2000000u
+#define ___GFP_SKIP_KASAN_POISON	0x4000000u
 #ifdef CONFIG_LOCKDEP
-#define ___GFP_NOLOCKDEP	0x4000000u
+#define ___GFP_NOLOCKDEP	0x8000000u
 #else
 #define ___GFP_NOLOCKDEP	0
 #endif
@@ -230,7 +231,11 @@  struct vm_area_struct;
  * %__GFP_ZERO returns a zeroed page on success.
  *
  * %__GFP_ZEROTAGS zeroes memory tags at allocation time if the memory itself
- * is being zeroed (either via __GFP_ZERO or via init_on_alloc).
+ * is being zeroed (either via __GFP_ZERO or via init_on_alloc, provided that
+ * __GFP_SKIP_ZERO is not set).
+ *
+ * %__GFP_SKIP_ZERO makes page_alloc skip zeroing memory.
+ * Only effective when HW_TAGS KASAN is enabled.
  *
  * %__GFP_SKIP_KASAN_UNPOISON makes KASAN skip unpoisoning on page allocation.
  * Only effective in HW_TAGS mode.
@@ -242,6 +247,7 @@  struct vm_area_struct;
 #define __GFP_COMP	((__force gfp_t)___GFP_COMP)
 #define __GFP_ZERO	((__force gfp_t)___GFP_ZERO)
 #define __GFP_ZEROTAGS	((__force gfp_t)___GFP_ZEROTAGS)
+#define __GFP_SKIP_ZERO ((__force gfp_t)___GFP_SKIP_ZERO)
 #define __GFP_SKIP_KASAN_UNPOISON ((__force gfp_t)___GFP_SKIP_KASAN_UNPOISON)
 #define __GFP_SKIP_KASAN_POISON   ((__force gfp_t)___GFP_SKIP_KASAN_POISON)
 
@@ -249,7 +255,7 @@  struct vm_area_struct;
 #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
 
 /* Room for N __GFP_FOO bits */
-#define __GFP_BITS_SHIFT (26 + IS_ENABLED(CONFIG_LOCKDEP))
+#define __GFP_BITS_SHIFT (27 + IS_ENABLED(CONFIG_LOCKDEP))
 #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
 
 /**
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f1d5b80591c4..af7516a2d5ea 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2409,10 +2409,21 @@  static inline bool should_skip_kasan_unpoison(gfp_t flags, bool init_tags)
 	return init_tags || (flags & __GFP_SKIP_KASAN_UNPOISON);
 }
 
+static inline bool should_skip_init(gfp_t flags)
+{
+	/* Don't skip if a software KASAN mode is enabled. */
+	if (!IS_ENABLED(CONFIG_KASAN_HW_TAGS))
+		return false;
+
+	/* For hardware tag-based KASAN, skip if requested. */
+	return (flags & __GFP_SKIP_ZERO);
+}
+
 inline void post_alloc_hook(struct page *page, unsigned int order,
 				gfp_t gfp_flags)
 {
-	bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags);
+	bool init = !want_init_on_free() && want_init_on_alloc(gfp_flags) &&
+			!should_skip_init(gfp_flags);
 	bool init_tags = init && (gfp_flags & __GFP_ZEROTAGS);
 
 	set_page_private(page, 0);