Re: [PATCH v3 14/15] mm/slab: remove __GFP_NO_OBJ_EXT usage from alloc_slab_obj_exts()

[Harry Yoo <harry@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3475 bytes --]



On 6/17/26 11:36 PM, Vlastimil Babka (SUSE) wrote:
> On 6/17/26 15:56, Harry Yoo wrote:
>> On 6/15/26 8:54 PM, Vlastimil Babka (SUSE) wrote:
>>> __GFP_NO_OBJ_EXT has limited scope within the slab allocator itself and
>>> gfp flags are a scarce resource, unlike slab's alloc_flags.
>>>
>>> Introduce SLAB_ALLOC_NO_RECURSE alloc flag that has the same intent as
>>> __GFP_NO_OBJ_EXT but a more generic name, meaning that a kmalloc()
>>> family function should not recurse into another kmalloc*() for the
>>> purposes of allocating auxiliary structures (obj_ext arrays or sheaves).
>>>
>>> First, replace the __GFP_NO_OBJ_EXT for allocating obj_ext arrays in
>>> alloc_slab_obj_exts(). Make use of the newly added kmalloc_flags()
>>> function, where we can pass alloc_flags with SLAB_ALLOC_NO_RECURSE
>>> added. This will also pass through SLAB_ALLOC_NOLOCK so we don't need
>>> to special case kmalloc_nolock() anymore.
>>>
>>> Note that until now the kmalloc_nolock() ignored the incoming gfp flags
>>> and hardcoded __GFP_ZERO | __GFP_NO_OBJ_EXT. But it's correct to pass on
>>> the incoming gfp flags (only augmented with __GFP_ZERO), because if
>>> alloc_flags contain SLAB_ALLOC_NOLOCK, the incoming gfp flags have to
>>> be also compatible with it. However, we might have added __GFP_THISNODE
>>> for opportunistic slab allocation, as pointed out by Hao Li, and
>>> __GFP_COMP by allocate_slab() as pointed out by Shengming Hu. Solve this
>>> by adding both flags to OBJCGS_CLEAR_MASK as it makes sense to strip
>>> them anyway for non-kmalloc_nolock() allocations of sheaves or obj_ext
>>> arrays as well.
>>>
>>> To avoid recursion of sheaf -> obj_ext -> sheaf -> ... allocations at
>>> this patch, until the next patch converts sheaves to
>>> SLAB_ALLOC_NO_RECURSE, use both gfp and alloc_flags for obj_ext. The
>>> next patch will remove the gfp part.
>>>
>>> Link: https://patch.msgid.link/20260610-slab_alloc_flags-v2-15-7190909db118@kernel.org
>>> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
>>> ---
>>
>> Looks good to me,
>> Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>
> 
> Thanks!
>  
>> With some comments below.
>>
>> I was worried that perhaps replacing SLAB_ALLOC_NO_RECURSE with
>> __GFP_NO_OBJ_EXT will create a cycle of
>>
>> alloc_slab_obj_exts(SLAB_ALLOC_DEFAULT)
>> -> kmalloc_flags(SLAB_ALLOC_NO_RECURSE)
>> -> alloc_from_pcs(SLAB_ALLOC_NO_RECURSE)
>> -> refill_objects(SLAB_ALLOC_DEFAULT)
>> -> new_slab(SLAB_ALLOC_DEFAULT)
>> -> account_slab(SLAB_ALLOC_DEFAULT)
>> -> alloc_slab_obj_exts(SLAB_ALLOC_DEFAULT)
>>
>> with __GFP_NO_OBJ_EXT, it would have been passed to refill_objects(),
>> but SLAB_ALLOC_NO_RECURSE is not. However this cycle does not exist
>> because alloc_slab_obj_exts() clears __GFP_ACCOUNT (as part of
>> OBJCG_CLEAR_MASK) and memory profiling itself does not invoke
>> alloc_slab_obj_exts() when allocating new slabs if SLAB_ACCOUNT is not
>> set (which is interesting, by the way).
> 
> Hm yeah I think we should propagate alloc_flags to refill_objects() etc, to 
> avoid later surprise. But can be done as a later cleanup.

Ack.

>> Also alloc_slab_obj_exts() propagating SLAB_ALLOC_NEW_SLAB to
>> kmalloc_flags() is little bit confusing because it does not have any
>> effect due to SLAB_ALLOC_NO_RECURSE.
> 
> OK let's address this one by this fixup:

The fixup looks good to me, thanks!

-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]