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

[Usama Arif <usama.arif@linux.dev>]

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.