Linux Documentation
 help / color / mirror / Atom feed
From: Usama Arif <usama.arif@linux.dev>
To: lirongqing <lirongqing@baidu.com>
Cc: Usama Arif <usama.arif@linux.dev>,
	Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Vlastimil Babka <vbabka@kernel.org>, Harry Yoo <harry@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Hao Li <hao.li@linux.dev>, Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
Date: Wed, 27 May 2026 06:03:36 -0700	[thread overview]
Message-ID: <20260527130337.983366-1-usama.arif@linux.dev> (raw)
In-Reply-To: <20260527104634.2434-1-lirongqing@baidu.com>

On Wed, 27 May 2026 06:46:34 -0400 lirongqing <lirongqing@baidu.com> wrote:

> From: Li RongQing <lirongqing@baidu.com>
> 
> Replace the #ifdef CONFIG_SLUB_DEBUG_ON conditional compilation with a
> static key (mempool_debug_enabled). This allows enabling mempool debugging
> at boot time via:
> 
>     mempool_debug
> 
> Instead of requiring CONFIG_SLUB_DEBUG_ON at compile time. Benefits:
> 
> - Debugging can be enabled without rebuilding the kernel
> - Uses standard kernel static_key mechanism with minimal overhead
> 
> Suggested-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
> Cc: Vlastimil Babka <vbabka@kernel.org>
> Cc: Harry Yoo <harry@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Hao Li <hao.li@linux.dev>
> Cc: Christoph Lameter <cl@gentwo.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Gushchin <roman.gushchin@linux.dev>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  5 ++++
>  mm/mempool.c                                    | 32 ++++++++++++++++++-------
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 35ed9dc..5a070e6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3998,6 +3998,11 @@ Kernel parameters
>  			Note that even when enabled, there are a few cases where
>  			the feature is not effective.
>  
> +	mempool_debug	[MM]
> +			Enable mempool debugging. This enables element
> +			poison checking when freeing elements back to the
> +			pool. Useful for debugging mempool corruption.
> +
>  	memtest=	[KNL,X86,ARM,M68K,PPC,RISCV,EARLY] Enable memtest
>  			Format: <integer>
>  			default : 0 <disable>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index db23e0e..4f429a1 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -16,11 +16,28 @@
>  #include <linux/export.h>
>  #include <linux/mempool.h>
>  #include <linux/writeback.h>
> +#include <linux/static_key.h>
> +#include <linux/init.h>
>  #include "slab.h"
>  
>  static DECLARE_FAULT_ATTR(fail_mempool_alloc);
>  static DECLARE_FAULT_ATTR(fail_mempool_alloc_bulk);
>  
> +/*
> + * Debugging support for mempool using static key.
> + *
> + * This allows enabling mempool debug at boot time via:
> + *   mempool_debug
> + */
> +static DEFINE_STATIC_KEY_FALSE(mempool_debug_enabled);
> +
> +static int __init mempool_debug_setup(char *str)
> +{
> +	static_branch_enable(&mempool_debug_enabled);
> +	return 0;
> +}
> +early_param("mempool_debug", mempool_debug_setup);
> +

Can static_branch_enable() in mempool_debug_setup() run before
jump_label_init() has set static_key_initialized?

Looking at start_kernel() in init/main.c:

	setup_arch(&command_line);
	mm_core_init_early();
	/* Static keys and static calls are needed by LSMs */
	jump_label_init();
	...
	/* parameters may set static keys */
	parse_early_param();

This will trigger the warning in include/linux/jump_label.h has:

	#define STATIC_KEY_CHECK_USE(key) WARN(!static_key_initialized, \
	    "%s(): static key '%pS' used before call to jump_label_init()", \
	    __func__, (key))


mm/dmapool.c registers an equivalent debug toggle via __setup()
rather than early_param():

	static int __init dmapool_debug_setup(char *str)
	{
		static_branch_enable(&dmapool_debug_enabled);
		return 1;
	}
	__setup("dmapool_debug", dmapool_debug_setup);

I think you can reuse that.

>  static int __init mempool_faul_inject_init(void)
>  {
>  	int error;
> @@ -37,7 +54,6 @@ static int __init mempool_faul_inject_init(void)
>  }
>  late_initcall(mempool_faul_inject_init);
>  
> -#ifdef CONFIG_SLUB_DEBUG_ON
>  static void poison_error(struct mempool *pool, void *element, size_t size,
>  			 size_t byte)
>  {
> @@ -73,6 +89,9 @@ static void __check_element(struct mempool *pool, void *element, size_t size)
>  
>  static void check_element(struct mempool *pool, void *element)
>  {
> +	if (!static_branch_unlikely(&mempool_debug_enabled))
> +		return;
> +
>  	/* Skip checking: KASAN might save its metadata in the element. */
>  	if (kasan_enabled())
>  		return;
> @@ -112,6 +131,9 @@ static void __poison_element(void *element, size_t size)
>  
>  static void poison_element(struct mempool *pool, void *element)
>  {
> +	if (!static_branch_unlikely(&mempool_debug_enabled))
> +		return;
> +

Before this change, building with CONFIG_SLUB_DEBUG_ON=y compiled in
check_element() and poison_element() unconditionally, so the
poisoning and corruption checks ran on every mempool free/alloc.
After this change those checks are gated on the mempool_debug boot
parameter even when CONFIG_SLUB_DEBUG_ON=y.

Existing users who relied on CONFIG_SLUB_DEBUG_ON=y giving them
mempool poison checking will silently lose it on upgrade unless they
also add "mempool_debug" to the command line.

Would it be worth defaulting the static key to true under
CONFIG_SLUB_DEBUG_ON=y, for example:

	#ifdef CONFIG_SLUB_DEBUG_ON
	static DEFINE_STATIC_KEY_TRUE(mempool_debug_enabled);
	#else
	static DEFINE_STATIC_KEY_FALSE(mempool_debug_enabled);
	#endif

so the previous default behaviour is preserved.


>  	/* Skip poisoning: KASAN might save its metadata in the element. */
>  	if (kasan_enabled())
>  		return;
> @@ -140,14 +162,6 @@ static void poison_element(struct mempool *pool, void *element)
>  #endif
>  	}
>  }
> -#else /* CONFIG_SLUB_DEBUG_ON */
> -static inline void check_element(struct mempool *pool, void *element)
> -{
> -}
> -static inline void poison_element(struct mempool *pool, void *element)
> -{
> -}
> -#endif /* CONFIG_SLUB_DEBUG_ON */
>  
>  static __always_inline bool kasan_poison_element(struct mempool *pool,
>  		void *element)
> -- 
> 2.9.4
> 
> 

  reply	other threads:[~2026-05-27 13:04 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27 10:46 [PATCH] mm/mempool: use static key for boot-time debug enablement lirongqing
2026-05-27 13:03 ` Usama Arif [this message]
2026-05-27 16:43   ` Vlastimil Babka (SUSE)
2026-05-27 20:06 ` Andrew Morton
2026-05-27 21:29 ` Christoph Lameter (Ampere)
2026-05-27 22:13   ` Matthew Wilcox
2026-05-27 23:06     ` Christoph Lameter (Ampere)
2026-05-27 23:09       ` Matthew Wilcox

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=20260527130337.983366-1-usama.arif@linux.dev \
    --to=usama.arif@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=corbet@lwn.net \
    --cc=hao.li@linux.dev \
    --cc=harry@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lirongqing@baidu.com \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=skhan@linuxfoundation.org \
    --cc=vbabka@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox