| Message ID | YjLtBdH8GpTO0/eX@ip-172-31-19-208.ap-northeast-1.compute.internal (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/slub, kunit: Make slub_kunit unaffected by global slub debugging flags | expand |
On 3/17/22 09:10, Hyeonggon Yoo wrote: > slub_kunit does not expect other debugging flags to be set when running > tests. When SLAB_RED_ZONE flag is set globally, test fails because the > flag affects number of errors reported. > > To make slub_kunit unaffected by global slub debugging flags, introduce > SLAB_NO_GLOBAL_FLAGS to ignore them. It's still allowed to specify > debugging flags by specifying cache name(s) in slub_debug parameter. Given how we support globbing, I think it would be safest to just ignore everything that comes from slub_debug parameter when the SLAB_NO_GLOBAL_FLAGS flag is specified, even if it involves a (partial) cache name match. Maybe name it SLAB_NO_USER_FLAGS then? > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > --- > include/linux/slab.h | 3 +++ > lib/slub_kunit.c | 10 +++++----- > mm/slab.h | 5 +++-- > mm/slub.c | 3 +++ > 4 files changed, 14 insertions(+), 7 deletions(-) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 0381868e5118..11fe2c28422d 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -112,6 +112,9 @@ > #define SLAB_KASAN 0 > #endif > > +/* Ignore globally specified debugging flags */ I'd add that this is intended for caches created for self-tests so they always have flags as specified in the code. > +#define SLAB_NO_GLOBAL_FLAGS ((slab_flags_t __force)0x10000000U) > + > /* The following flags affect the page allocator grouping pages by mobility */ > /* Objects are reclaimable */ > #define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000U) > diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c > index 8662dc6cb509..acf061dc558d 100644 > --- a/lib/slub_kunit.c > +++ b/lib/slub_kunit.c > @@ -12,7 +12,7 @@ static int slab_errors; > static void test_clobber_zone(struct kunit *test) > { > struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_alloc", 64, 0, > - SLAB_RED_ZONE, NULL); > + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > kasan_disable_current(); > @@ -30,7 +30,7 @@ static void test_clobber_zone(struct kunit *test) > static void test_next_pointer(struct kunit *test) > { > struct kmem_cache *s = kmem_cache_create("TestSlub_next_ptr_free", 64, 0, > - SLAB_POISON, NULL); > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > unsigned long tmp; > unsigned long *ptr_addr; > @@ -75,7 +75,7 @@ static void test_next_pointer(struct kunit *test) > static void test_first_word(struct kunit *test) > { > struct kmem_cache *s = kmem_cache_create("TestSlub_1th_word_free", 64, 0, > - SLAB_POISON, NULL); > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > kmem_cache_free(s, p); > @@ -90,7 +90,7 @@ static void test_first_word(struct kunit *test) > static void test_clobber_50th_byte(struct kunit *test) > { > struct kmem_cache *s = kmem_cache_create("TestSlub_50th_word_free", 64, 0, > - SLAB_POISON, NULL); > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > kmem_cache_free(s, p); > @@ -106,7 +106,7 @@ static void test_clobber_50th_byte(struct kunit *test) > static void test_clobber_redzone_free(struct kunit *test) > { > struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_free", 64, 0, > - SLAB_RED_ZONE, NULL); > + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > kasan_disable_current(); > diff --git a/mm/slab.h b/mm/slab.h > index c7f2abc2b154..69946131208a 100644 > --- a/mm/slab.h > +++ b/mm/slab.h > @@ -330,7 +330,7 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > SLAB_ACCOUNT) > #elif defined(CONFIG_SLUB) > #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE | SLAB_RECLAIM_ACCOUNT | \ > - SLAB_TEMPORARY | SLAB_ACCOUNT) > + SLAB_TEMPORARY | SLAB_ACCOUNT | SLAB_NO_GLOBAL_FLAGS) > #else > #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE) > #endif > @@ -349,7 +349,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > SLAB_NOLEAKTRACE | \ > SLAB_RECLAIM_ACCOUNT | \ > SLAB_TEMPORARY | \ > - SLAB_ACCOUNT) > + SLAB_ACCOUNT | \ > + SLAB_NO_GLOBAL_FLAGS) > > bool __kmem_cache_empty(struct kmem_cache *); > int __kmem_cache_shutdown(struct kmem_cache *); > diff --git a/mm/slub.c b/mm/slub.c > index 71e8663f6037..2a3cffd7b27f 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -1592,6 +1592,9 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, > if (flags & SLAB_NOLEAKTRACE) > slub_debug_local &= ~SLAB_STORE_USER; > > + if (flags & SLAB_NO_GLOBAL_FLAGS) > + slub_debug_local = 0; > + > len = strlen(name); > next_block = slub_debug_string; > /* Go through all blocks of debug options, see if any matches our slab's name */
On Tue, Apr 05, 2022 at 12:58:53PM +0200, Vlastimil Babka wrote: > On 3/17/22 09:10, Hyeonggon Yoo wrote: > > slub_kunit does not expect other debugging flags to be set when running > > tests. When SLAB_RED_ZONE flag is set globally, test fails because the > > flag affects number of errors reported. > > > > To make slub_kunit unaffected by global slub debugging flags, introduce > > SLAB_NO_GLOBAL_FLAGS to ignore them. It's still allowed to specify > > debugging flags by specifying cache name(s) in slub_debug parameter. > > Given how we support globbing, I think it would be safest to just ignore > everything that comes from slub_debug parameter when the > SLAB_NO_GLOBAL_FLAGS flag is specified, even if it involves a (partial) > cache name match. > Maybe name it SLAB_NO_USER_FLAGS then? > Seems reasonable. letting your specify debugging flags for self-test cache is not useful for any case. Let's make self-test caches completely isolated from user-specified flags. I sent v2 with your comments in mind. > > Suggested-by: Vlastimil Babka <vbabka@suse.cz> > > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> > > --- > > include/linux/slab.h | 3 +++ > > lib/slub_kunit.c | 10 +++++----- > > mm/slab.h | 5 +++-- > > mm/slub.c | 3 +++ > > 4 files changed, 14 insertions(+), 7 deletions(-) > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > index 0381868e5118..11fe2c28422d 100644 > > --- a/include/linux/slab.h > > +++ b/include/linux/slab.h > > @@ -112,6 +112,9 @@ > > #define SLAB_KASAN 0 > > #endif > > > > +/* Ignore globally specified debugging flags */ > > I'd add that this is intended for caches created for self-tests so they > always have flags as specified in the code. > Did it in v2. Thanks! Hyeonggon > > +#define SLAB_NO_GLOBAL_FLAGS ((slab_flags_t __force)0x10000000U) > > + > > /* The following flags affect the page allocator grouping pages by mobility */ > > /* Objects are reclaimable */ > > #define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000U) > > diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c > > index 8662dc6cb509..acf061dc558d 100644 > > --- a/lib/slub_kunit.c > > +++ b/lib/slub_kunit.c > > @@ -12,7 +12,7 @@ static int slab_errors; > > static void test_clobber_zone(struct kunit *test) > > { > > struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_alloc", 64, 0, > > - SLAB_RED_ZONE, NULL); > > + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); > > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > > > kasan_disable_current(); > > @@ -30,7 +30,7 @@ static void test_clobber_zone(struct kunit *test) > > static void test_next_pointer(struct kunit *test) > > { > > struct kmem_cache *s = kmem_cache_create("TestSlub_next_ptr_free", 64, 0, > > - SLAB_POISON, NULL); > > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > unsigned long tmp; > > unsigned long *ptr_addr; > > @@ -75,7 +75,7 @@ static void test_next_pointer(struct kunit *test) > > static void test_first_word(struct kunit *test) > > { > > struct kmem_cache *s = kmem_cache_create("TestSlub_1th_word_free", 64, 0, > > - SLAB_POISON, NULL); > > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > > > kmem_cache_free(s, p); > > @@ -90,7 +90,7 @@ static void test_first_word(struct kunit *test) > > static void test_clobber_50th_byte(struct kunit *test) > > { > > struct kmem_cache *s = kmem_cache_create("TestSlub_50th_word_free", 64, 0, > > - SLAB_POISON, NULL); > > + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); > > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > > > kmem_cache_free(s, p); > > @@ -106,7 +106,7 @@ static void test_clobber_50th_byte(struct kunit *test) > > static void test_clobber_redzone_free(struct kunit *test) > > { > > struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_free", 64, 0, > > - SLAB_RED_ZONE, NULL); > > + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); > > u8 *p = kmem_cache_alloc(s, GFP_KERNEL); > > > > kasan_disable_current(); > > diff --git a/mm/slab.h b/mm/slab.h > > index c7f2abc2b154..69946131208a 100644 > > --- a/mm/slab.h > > +++ b/mm/slab.h > > @@ -330,7 +330,7 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > > SLAB_ACCOUNT) > > #elif defined(CONFIG_SLUB) > > #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE | SLAB_RECLAIM_ACCOUNT | \ > > - SLAB_TEMPORARY | SLAB_ACCOUNT) > > + SLAB_TEMPORARY | SLAB_ACCOUNT | SLAB_NO_GLOBAL_FLAGS) > > #else > > #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE) > > #endif > > @@ -349,7 +349,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, > > SLAB_NOLEAKTRACE | \ > > SLAB_RECLAIM_ACCOUNT | \ > > SLAB_TEMPORARY | \ > > - SLAB_ACCOUNT) > > + SLAB_ACCOUNT | \ > > + SLAB_NO_GLOBAL_FLAGS) > > > > bool __kmem_cache_empty(struct kmem_cache *); > > int __kmem_cache_shutdown(struct kmem_cache *); > > diff --git a/mm/slub.c b/mm/slub.c > > index 71e8663f6037..2a3cffd7b27f 100644 > > --- a/mm/slub.c > > +++ b/mm/slub.c > > @@ -1592,6 +1592,9 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, > > if (flags & SLAB_NOLEAKTRACE) > > slub_debug_local &= ~SLAB_STORE_USER; > > > > + if (flags & SLAB_NO_GLOBAL_FLAGS) > > + slub_debug_local = 0; > > + > > len = strlen(name); > > next_block = slub_debug_string; > > /* Go through all blocks of debug options, see if any matches our slab's name */ >
diff --git a/include/linux/slab.h b/include/linux/slab.h index 0381868e5118..11fe2c28422d 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -112,6 +112,9 @@ #define SLAB_KASAN 0 #endif +/* Ignore globally specified debugging flags */ +#define SLAB_NO_GLOBAL_FLAGS ((slab_flags_t __force)0x10000000U) + /* The following flags affect the page allocator grouping pages by mobility */ /* Objects are reclaimable */ #define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000U) diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c index 8662dc6cb509..acf061dc558d 100644 --- a/lib/slub_kunit.c +++ b/lib/slub_kunit.c @@ -12,7 +12,7 @@ static int slab_errors; static void test_clobber_zone(struct kunit *test) { struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_alloc", 64, 0, - SLAB_RED_ZONE, NULL); + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); u8 *p = kmem_cache_alloc(s, GFP_KERNEL); kasan_disable_current(); @@ -30,7 +30,7 @@ static void test_clobber_zone(struct kunit *test) static void test_next_pointer(struct kunit *test) { struct kmem_cache *s = kmem_cache_create("TestSlub_next_ptr_free", 64, 0, - SLAB_POISON, NULL); + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); u8 *p = kmem_cache_alloc(s, GFP_KERNEL); unsigned long tmp; unsigned long *ptr_addr; @@ -75,7 +75,7 @@ static void test_next_pointer(struct kunit *test) static void test_first_word(struct kunit *test) { struct kmem_cache *s = kmem_cache_create("TestSlub_1th_word_free", 64, 0, - SLAB_POISON, NULL); + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); u8 *p = kmem_cache_alloc(s, GFP_KERNEL); kmem_cache_free(s, p); @@ -90,7 +90,7 @@ static void test_first_word(struct kunit *test) static void test_clobber_50th_byte(struct kunit *test) { struct kmem_cache *s = kmem_cache_create("TestSlub_50th_word_free", 64, 0, - SLAB_POISON, NULL); + SLAB_POISON|SLAB_NO_GLOBAL_FLAGS, NULL); u8 *p = kmem_cache_alloc(s, GFP_KERNEL); kmem_cache_free(s, p); @@ -106,7 +106,7 @@ static void test_clobber_50th_byte(struct kunit *test) static void test_clobber_redzone_free(struct kunit *test) { struct kmem_cache *s = kmem_cache_create("TestSlub_RZ_free", 64, 0, - SLAB_RED_ZONE, NULL); + SLAB_RED_ZONE|SLAB_NO_GLOBAL_FLAGS, NULL); u8 *p = kmem_cache_alloc(s, GFP_KERNEL); kasan_disable_current(); diff --git a/mm/slab.h b/mm/slab.h index c7f2abc2b154..69946131208a 100644 --- a/mm/slab.h +++ b/mm/slab.h @@ -330,7 +330,7 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, SLAB_ACCOUNT) #elif defined(CONFIG_SLUB) #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE | SLAB_RECLAIM_ACCOUNT | \ - SLAB_TEMPORARY | SLAB_ACCOUNT) + SLAB_TEMPORARY | SLAB_ACCOUNT | SLAB_NO_GLOBAL_FLAGS) #else #define SLAB_CACHE_FLAGS (SLAB_NOLEAKTRACE) #endif @@ -349,7 +349,8 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size, SLAB_NOLEAKTRACE | \ SLAB_RECLAIM_ACCOUNT | \ SLAB_TEMPORARY | \ - SLAB_ACCOUNT) + SLAB_ACCOUNT | \ + SLAB_NO_GLOBAL_FLAGS) bool __kmem_cache_empty(struct kmem_cache *); int __kmem_cache_shutdown(struct kmem_cache *); diff --git a/mm/slub.c b/mm/slub.c index 71e8663f6037..2a3cffd7b27f 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1592,6 +1592,9 @@ slab_flags_t kmem_cache_flags(unsigned int object_size, if (flags & SLAB_NOLEAKTRACE) slub_debug_local &= ~SLAB_STORE_USER; + if (flags & SLAB_NO_GLOBAL_FLAGS) + slub_debug_local = 0; + len = strlen(name); next_block = slub_debug_string; /* Go through all blocks of debug options, see if any matches our slab's name */
slub_kunit does not expect other debugging flags to be set when running tests. When SLAB_RED_ZONE flag is set globally, test fails because the flag affects number of errors reported. To make slub_kunit unaffected by global slub debugging flags, introduce SLAB_NO_GLOBAL_FLAGS to ignore them. It's still allowed to specify debugging flags by specifying cache name(s) in slub_debug parameter. Suggested-by: Vlastimil Babka <vbabka@suse.cz> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com> --- include/linux/slab.h | 3 +++ lib/slub_kunit.c | 10 +++++----- mm/slab.h | 5 +++-- mm/slub.c | 3 +++ 4 files changed, 14 insertions(+), 7 deletions(-)