| Message ID | cd8667450f7a0daf6b4081276e11a5f7bed60128.1639432170.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kasan, vmalloc, arm64: add vmalloc tagging support for SW/HW_TAGS | expand |
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.
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!
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 > >
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 --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);