Re: [PATCH] mm/mempool: use static key for boot-time debug enablement

[Vlastimil Babka (SUSE)]

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

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