All of lore.kernel.org
 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:03 UTC|newest]

Thread overview: 16+ 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-28  3:00   ` 答复: [外部邮件] " Li,Rongqing(ACG CCN)
2026-05-28 10:33     ` Usama Arif
2026-05-28 10:50       ` 答复: " Li,Rongqing(ACG CCN)
2026-05-28 12:59         ` Usama Arif
2026-05-29  9:39           ` Vlastimil Babka (SUSE)
2026-06-01  1:28             ` 答复: " Li,Rongqing(ACG CCN)
2026-05-27 20:06 ` Andrew Morton
2026-05-28  7:54   ` Vlastimil Babka (SUSE)
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
2026-05-28  7:57         ` 答复: [????] " Li,Rongqing(ACG CCN)

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 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.