All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Cc: linux-mm@kvack.org, Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Oliver Glitta <glittao@gmail.com>
Subject: Re: [PATCH] mm/slub, kunit: Make slub_kunit unaffected by global slub debugging flags
Date: Tue, 5 Apr 2022 12:58:53 +0200	[thread overview]
Message-ID: <f5b8e996-dc1e-c223-ff71-31e67801b772@suse.cz> (raw)
In-Reply-To: <YjLtBdH8GpTO0/eX@ip-172-31-19-208.ap-northeast-1.compute.internal>

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 */



  reply	other threads:[~2022-04-05 10:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-16 14:38 [PATCH] mm/slub, kunit: Make slub_kunit pass even when SLAB_RED_ZONE flag is set Hyeonggon Yoo
2022-03-16 22:11 ` Vlastimil Babka
2022-03-17  7:06   ` Hyeonggon Yoo
2022-03-17  8:10   ` [PATCH] mm/slub, kunit: Make slub_kunit unaffected by global slub debugging flags Hyeonggon Yoo
2022-04-05 10:58     ` Vlastimil Babka [this message]
2022-04-06  6:00       ` [PATCH v2] mm/slub, kunit: Make slub_kunit unaffected by user specified flags Hyeonggon Yoo
2022-04-06  8:17         ` Vlastimil Babka
2022-04-06  6:06       ` [PATCH] mm/slub, kunit: Make slub_kunit unaffected by global slub debugging flags 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=f5b8e996-dc1e-c223-ff71-31e67801b772@suse.cz \
    --to=vbabka@suse.cz \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@linux.com \
    --cc=glittao@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    /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.