答复: 答复: [外部邮件] Re: [PATCH] mm/mempool: use static key for boot-time debug enablement

[Li,Rongqing(ACG CCN)]

> 
> On 28/05/2026 04:00, Li,Rongqing(ACG CCN) wrote:
> >>> From: Li RongQing <lirongqing@xxxxxxxxx>
> >>>
> >>> 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@xxxxxxxxxx>
> >>> Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
> >>> Cc: Vlastimil Babka <vbabka@xxxxxxxxxx>
> >>> Cc: Harry Yoo <harry@xxxxxxxxxx>
> >>> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> >>> Cc: Hao Li <hao.li@xxxxxxxxx>
> >>> Cc: Christoph Lameter <cl@xxxxxxxxxx>
> >>> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> >>> Cc: Roman Gushchin <roman.gushchin@xxxxxxxxx>
> >>> ---
> >>>  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
> 
> >