Linux Documentation
 help / color / mirror / Atom feed
* [PATCH] mm/mempool: use static key for boot-time debug enablement
@ 2026-05-27 10:46 lirongqing
  2026-05-27 13:03 ` Usama Arif
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: lirongqing @ 2026-05-27 10:46 UTC (permalink / raw)
  To: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, Christoph Lameter, David Rientjes,
	Roman Gushchin, linux-doc, linux-kernel, linux-mm
  Cc: Li RongQing

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);
+
 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;
+
 	/* 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 10:46 [PATCH] mm/mempool: use static key for boot-time debug enablement lirongqing
@ 2026-05-27 13:03 ` Usama Arif
  2026-05-27 16:43   ` Vlastimil Babka (SUSE)
  2026-05-28  3:00   ` 答复: [外部邮件] " Li,Rongqing(ACG CCN)
  2026-05-27 20:06 ` Andrew Morton
  2026-05-27 21:29 ` Christoph Lameter (Ampere)
  2 siblings, 2 replies; 14+ messages in thread
From: Usama Arif @ 2026-05-27 13:03 UTC (permalink / raw)
  To: lirongqing
  Cc: Usama Arif, Jonathan Corbet, Shuah Khan, Vlastimil Babka,
	Harry Yoo, Andrew Morton, Hao Li, Christoph Lameter,
	David Rientjes, Roman Gushchin, linux-doc, linux-kernel, linux-mm

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 13:03 ` Usama Arif
@ 2026-05-27 16:43   ` Vlastimil Babka (SUSE)
  2026-05-28  3:00   ` 答复: [外部邮件] " Li,Rongqing(ACG CCN)
  1 sibling, 0 replies; 14+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-05-27 16:43 UTC (permalink / raw)
  To: Usama Arif, lirongqing
  Cc: Jonathan Corbet, Shuah Khan, Harry Yoo, Andrew Morton, Hao Li,
	Christoph Lameter, David Rientjes, Roman Gushchin, linux-doc,
	linux-kernel, linux-mm

On 5/27/26 15:03, Usama Arif wrote:
> On Wed, 27 May 2026 06:46:34 -0400 lirongqing <lirongqing@baidu.com> wrote:
>>  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.

The same would apply to the dmapool_debug [1]

https://lore.kernel.org/all/20260524034015.1830-1-lirongqing@baidu.com/

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

There's DEFINE_STATIC_KEY_MAYBE for this.

But I think nobody will care really. My objection was that this and
dmapool_debug was tied to CONFIG_SLUB_DEBUG_ON in the first place, despite
not being part of slab. I'd rather disconnect it completely.

If testing bots relied on this, we could give them heads up to start using
those boot options. I assume they already use some for stuff that has no
CONFIG_ enablement.

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 10:46 [PATCH] mm/mempool: use static key for boot-time debug enablement lirongqing
  2026-05-27 13:03 ` Usama Arif
@ 2026-05-27 20:06 ` Andrew Morton
  2026-05-28  7:54   ` Vlastimil Babka (SUSE)
  2026-05-27 21:29 ` Christoph Lameter (Ampere)
  2 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2026-05-27 20:06 UTC (permalink / raw)
  To: lirongqing
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo, Hao Li,
	Christoph Lameter, David Rientjes, Roman Gushchin, linux-doc,
	linux-kernel, linux-mm

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

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

Sashiko is suggesting that we use mempool_debug=<...> here.  Which permits
mempool_debug=n if for some reason the kernel is defaulting to "on".  Which
we might choose to do in the future.   I think that's a little better - do others agree?

Same goes for the new dmapool_debug.

Sashiko asked a second question:
	https://sashiko.dev/#/patchset/20260527104634.2434-1-lirongqing@baidu.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 10:46 [PATCH] mm/mempool: use static key for boot-time debug enablement lirongqing
  2026-05-27 13:03 ` Usama Arif
  2026-05-27 20:06 ` Andrew Morton
@ 2026-05-27 21:29 ` Christoph Lameter (Ampere)
  2026-05-27 22:13   ` Matthew Wilcox
  2 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter (Ampere) @ 2026-05-27 21:29 UTC (permalink / raw)
  To: lirongqing
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, David Rientjes, Roman Gushchin, linux-doc,
	linux-kernel, linux-mm

On Wed, 27 May 2026, lirongqing wrote:

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

I am a bit confused here. CONFIG_SLUB_DEBUG_ON sets debugging to be on by
default at compile time. If you do not use CONFIG_SLUB_DEBUG_ON then
debugging code will still be compiled in but is disabled by default.

Debugging can still be enabled by specifying

slub_debug

on the kernel command line. And AFACIT it would be easy
for the mempool subsystem to switch on slub debuggin if needed also.



Why is there a requirement to compile with SLUB_DEBUG_ON?


Oh someone put an #ifdef CONFIG_SLUB_DEBUG_ON in mempool.c. Overloading
the meaning of SLUB_DEBUG_ON as it is used in slub.c with something else.

Someone was assuming that SLUB_DEBUG_ON means the same as SLUB_DEBUG?

Please clean this mess up.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 21:29 ` Christoph Lameter (Ampere)
@ 2026-05-27 22:13   ` Matthew Wilcox
  2026-05-27 23:06     ` Christoph Lameter (Ampere)
  0 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2026-05-27 22:13 UTC (permalink / raw)
  To: Christoph Lameter (Ampere)
  Cc: lirongqing, Jonathan Corbet, Shuah Khan, Vlastimil Babka,
	Harry Yoo, Andrew Morton, Hao Li, David Rientjes, Roman Gushchin,
	linux-doc, linux-kernel, linux-mm

On Wed, May 27, 2026 at 02:29:22PM -0700, Christoph Lameter (Ampere) wrote:
> Oh someone put an #ifdef CONFIG_SLUB_DEBUG_ON in mempool.c. Overloading
> the meaning of SLUB_DEBUG_ON as it is used in slub.c with something else.
> 
> Someone was assuming that SLUB_DEBUG_ON means the same as SLUB_DEBUG?
> 
> Please clean this mess up.

Isn't that what this patch does?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 22:13   ` Matthew Wilcox
@ 2026-05-27 23:06     ` Christoph Lameter (Ampere)
  2026-05-27 23:09       ` Matthew Wilcox
  0 siblings, 1 reply; 14+ messages in thread
From: Christoph Lameter (Ampere) @ 2026-05-27 23:06 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: lirongqing, Jonathan Corbet, Shuah Khan, Vlastimil Babka,
	Harry Yoo, Andrew Morton, Hao Li, David Rientjes, Roman Gushchin,
	linux-doc, linux-kernel, linux-mm

On Wed, 27 May 2026, Matthew Wilcox wrote:

> > Please clean this mess up.
>
> Isn't that what this patch does?

Its not marked as fixing something nor as addressing the weirdness of
using CONFIG_SLUB_DEBUG_ON here, A kernel build with CONFIG_SLUB_DEBUG_ON
can still boot without debugging if a certain kernel command line option
is given.




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 23:06     ` Christoph Lameter (Ampere)
@ 2026-05-27 23:09       ` Matthew Wilcox
  2026-05-28  7:57         ` 答复: [????] " Li,Rongqing(ACG CCN)
  0 siblings, 1 reply; 14+ messages in thread
From: Matthew Wilcox @ 2026-05-27 23:09 UTC (permalink / raw)
  To: Christoph Lameter (Ampere)
  Cc: lirongqing, Jonathan Corbet, Shuah Khan, Vlastimil Babka,
	Harry Yoo, Andrew Morton, Hao Li, David Rientjes, Roman Gushchin,
	linux-doc, linux-kernel, linux-mm

On Wed, May 27, 2026 at 04:06:18PM -0700, Christoph Lameter (Ampere) wrote:
> On Wed, 27 May 2026, Matthew Wilcox wrote:
> 
> > > Please clean this mess up.
> >
> > Isn't that what this patch does?
> 
> Its not marked as fixing something nor as addressing the weirdness of
> using CONFIG_SLUB_DEBUG_ON here, A kernel build with CONFIG_SLUB_DEBUG_ON
> can still boot without debugging if a certain kernel command line option
> is given.

Right, but ... if you look at what the patch _does_, doesn't it do what
you're asking for it to do?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* 答复: [外部邮件] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 13:03 ` Usama Arif
  2026-05-27 16:43   ` Vlastimil Babka (SUSE)
@ 2026-05-28  3:00   ` Li,Rongqing(ACG CCN)
  2026-05-28 10:33     ` Usama Arif
  1 sibling, 1 reply; 14+ messages in thread
From: Li,Rongqing(ACG CCN) @ 2026-05-28  3:00 UTC (permalink / raw)
  To: Usama Arif
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, Christoph Lameter, David Rientjes,
	Roman Gushchin, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org

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

Thanks for your review!

While this boot-time ordering used to be a generic issue, it seems many
architectures have already aligned or fixed this internally. For instance,

commit ca829e05d3d4 ("powerpc/64: Init jump labels before parse_early_param()")
and commit 6070970db9fe ("m68k: Initialize jump labels early during setup_arch()")
explicitly relocated jump_label_init() before the early parameter parsing.

Furthermore, leveraging early_param() to directly manage static keys is still
actively used and accepted in the current core kernel. Some examples include:

  - early_param("randomize_kstack_offset", early_randomize_kstack_offset);
  - early_param("threadirqs", setup_forced_irqthreads);

The primary reason for using early_param() here instead of __setup() is that
mempool allocations can happen extremely early during the boot phase. Moving
this to a later stage like __setup() would mean missing the tracking for the
most critical early-stage memory pools, which defeats the purpose of boot-time
debugging.

Therefore, I think using early_param() here is the most robust option to
ensure full coverage of mempool allocations.

What do you think?

Thanks

-Li


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 20:06 ` Andrew Morton
@ 2026-05-28  7:54   ` Vlastimil Babka (SUSE)
  0 siblings, 0 replies; 14+ messages in thread
From: Vlastimil Babka (SUSE) @ 2026-05-28  7:54 UTC (permalink / raw)
  To: Andrew Morton, lirongqing, Usama Arif
  Cc: Jonathan Corbet, Shuah Khan, Harry Yoo, Hao Li, Christoph Lameter,
	David Rientjes, Roman Gushchin, linux-doc, linux-kernel, linux-mm

On 5/27/26 22:06, Andrew Morton wrote:
> On Wed, 27 May 2026 06:46:34 -0400 lirongqing <lirongqing@baidu.com> wrote:
> 
>> 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:
> 
> Sashiko is suggesting that we use mempool_debug=<...> here.  Which permits
> mempool_debug=n if for some reason the kernel is defaulting to "on".  Which
> we might choose to do in the future.   I think that's a little better - do others agree?

Yeah we could do that. But I still think "CONFIG_SLUB_DEBUG_ON" isn't what
should cause mempool_debug to default to "on". It can be revisited once
there's a solid argument why default on would be needed.

> Same goes for the new dmapool_debug.

Ack.

> Sashiko asked a second question:
> 	https://sashiko.dev/#/patchset/20260527104634.2434-1-lirongqing@baidu.com

Seems the same thing as Usama raised about static key init ordering.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* 答复: [????] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-27 23:09       ` Matthew Wilcox
@ 2026-05-28  7:57         ` Li,Rongqing(ACG CCN)
  0 siblings, 0 replies; 14+ messages in thread
From: Li,Rongqing(ACG CCN) @ 2026-05-28  7:57 UTC (permalink / raw)
  To: Matthew Wilcox, Christoph Lameter (Ampere)
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, David Rientjes, Roman Gushchin,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org

> > > > Please clean this mess up.
> > >
> > > Isn't that what this patch does?
> >
> > Its not marked as fixing something nor as addressing the weirdness of
> > using CONFIG_SLUB_DEBUG_ON here, A kernel build with
> > CONFIG_SLUB_DEBUG_ON can still boot without debugging if a certain
> > kernel command line option is given.
> 
> Right, but ... if you look at what the patch _does_, doesn't it do what you're
> asking for it to do?

Hi Matthew, Christoph,

Matthew, thanks a lot for standing up for the code logic! I really appreciate your support 
on this.

Christoph, thank you for pointing out the semantic gaps. To address your concerns, 
I have completely rewritten the commit message to focus strictly on untangling 
the CONFIG_SLUB_DEBUG_ON abuse and switching to mempool's own runtime parameter.

Here is the revised commit message:

---
mm/mempool: Untangle CONFIG_SLUB_DEBUG_ON abuse and switch to static key

The mempool subsystem historically wrapped its debugging logic inside an
#ifdef CONFIG_SLUB_DEBUG_ON block. This abused the config's intent (which
merely defines compile-time defaults for SLUB) and caused two flaws:

1. On production kernels where CONFIG_SLUB_DEBUG=y but CONFIG_SLUB_DEBUG_ON=n,
   mempool debugging was completely truncated at compile time.
2. On kernels with CONFIG_SLUB_DEBUG_ON=y, mempool debugging stayed active
   even if a user explicitly disabled debugging at boot time.

Clean up this mess by removing the #ifdef and switching to a runtime static
key (mempool_debug_enabled), allowing mempool debugging to be toggled cleanly
via its own boot parameter.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---

Does this revised text look good to you? If so, I will officially send out V2.

Thanks,

[Li,Rongqing] 




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 答复: [外部邮件] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  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)
  0 siblings, 1 reply; 14+ messages in thread
From: Usama Arif @ 2026-05-28 10:33 UTC (permalink / raw)
  To: Li,Rongqing(ACG CCN)
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, Christoph Lameter, David Rientjes,
	Roman Gushchin, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org



On 28/05/2026 04:00, Li,Rongqing(ACG CCN) 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.
> 
> Thanks for your review!
> 
> While this boot-time ordering used to be a generic issue, it seems many
> architectures have already aligned or fixed this internally. For instance,
> 
> commit ca829e05d3d4 ("powerpc/64: Init jump labels before parse_early_param()")
> and commit 6070970db9fe ("m68k: Initialize jump labels early during setup_arch()")
> explicitly relocated jump_label_init() before the early parameter parsing.
> 

I think 32 bit ARM doesnt? 

> Furthermore, leveraging early_param() to directly manage static keys is still
> actively used and accepted in the current core kernel. Some examples include:
> 
>   - early_param("randomize_kstack_offset", early_randomize_kstack_offset);
>   - early_param("threadirqs", setup_forced_irqthreads);
> 
> The primary reason for using early_param() here instead of __setup() is that
> mempool allocations can happen extremely early during the boot phase. Moving
> this to a later stage like __setup() would mean missing the tracking for the
> most critical early-stage memory pools, which defeats the purpose of boot-time
> debugging.

Ack

> 
> Therefore, I think using early_param() here is the most robust option to
> ensure full coverage of mempool allocations.
> 
> What do you think?
> > Thanks
> 
> -Li
> 
> 
>>
>>>  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
>>>
>>>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* 答复: 答复: [外部邮件] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-28 10:33     ` Usama Arif
@ 2026-05-28 10:50       ` Li,Rongqing(ACG CCN)
  2026-05-28 12:59         ` Usama Arif
  0 siblings, 1 reply; 14+ messages in thread
From: Li,Rongqing(ACG CCN) @ 2026-05-28 10:50 UTC (permalink / raw)
  To: Usama Arif
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, Christoph Lameter, David Rientjes,
	Roman Gushchin, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org



> 
> On 28/05/2026 04:00, Li,Rongqing(ACG CCN) 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.
> >
> > Thanks for your review!
> >
> > While this boot-time ordering used to be a generic issue, it seems
> > many architectures have already aligned or fixed this internally. For
> > instance,
> >
> > commit ca829e05d3d4 ("powerpc/64: Init jump labels before
> > parse_early_param()") and commit 6070970db9fe ("m68k: Initialize jump
> > labels early during setup_arch()") explicitly relocated jump_label_init() before
> the early parameter parsing.
> >
> 
> I think 32 bit ARM doesnt?

You are right, 32-bit ARM doesn't. 

However, the correct architectural approach should be fixing the boot sequence 
inside arch/arm/ to match arm64 , powerpc and m68k, rather than compromising core MM 
code with temporary boilerplate variables.

I prefer to keep the mempool implementation clean. If ARM32 triggers the 
warning, the proper remedy is a follow-up patch to align its setup_arch() 
ordering.

What do you think?

-LiRongQing

> 
> > Furthermore, leveraging early_param() to directly manage static keys
> > is still actively used and accepted in the current core kernel. Some examples
> include:
> >
> >   - early_param("randomize_kstack_offset", early_randomize_kstack_offset);
> >   - early_param("threadirqs", setup_forced_irqthreads);
> >
> > The primary reason for using early_param() here instead of __setup()
> > is that mempool allocations can happen extremely early during the boot
> > phase. Moving this to a later stage like __setup() would mean missing
> > the tracking for the most critical early-stage memory pools, which
> > defeats the purpose of boot-time debugging.
> 
> Ack
> 
> >


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: 答复: 答复: [外部邮件] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement
  2026-05-28 10:50       ` 答复: " Li,Rongqing(ACG CCN)
@ 2026-05-28 12:59         ` Usama Arif
  0 siblings, 0 replies; 14+ messages in thread
From: Usama Arif @ 2026-05-28 12:59 UTC (permalink / raw)
  To: Li,Rongqing(ACG CCN)
  Cc: Jonathan Corbet, Shuah Khan, Vlastimil Babka, Harry Yoo,
	Andrew Morton, Hao Li, Christoph Lameter, David Rientjes,
	Roman Gushchin, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org



On 28/05/2026 11:50, Li,Rongqing(ACG CCN) wrote:
> 
> 
>>
>> On 28/05/2026 04:00, Li,Rongqing(ACG CCN) 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.
>>>
>>> Thanks for your review!
>>>
>>> While this boot-time ordering used to be a generic issue, it seems
>>> many architectures have already aligned or fixed this internally. For
>>> instance,
>>>
>>> commit ca829e05d3d4 ("powerpc/64: Init jump labels before
>>> parse_early_param()") and commit 6070970db9fe ("m68k: Initialize jump
>>> labels early during setup_arch()") explicitly relocated jump_label_init() before
>> the early parameter parsing.
>>>
>>
>> I think 32 bit ARM doesnt?
> 
> You are right, 32-bit ARM doesn't. 
> 
> However, the correct architectural approach should be fixing the boot sequence 
> inside arch/arm/ to match arm64 , powerpc and m68k, rather than compromising core MM 
> code with temporary boilerplate variables.
> 
> I prefer to keep the mempool implementation clean. If ARM32 triggers the 
> warning, the proper remedy is a follow-up patch to align its setup_arch() 
> ordering.
> 
> What do you think?
> 

I think it would be a prerequisite rather than a follow up patch inorder to not
break 32 bit arm. I will let ARM and slab maintainers decide on this. 



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-05-28 12:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-27 10:46 [PATCH] mm/mempool: use static key for boot-time debug enablement lirongqing
2026-05-27 13:03 ` Usama Arif
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-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)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox