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

[Harry Yoo]

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@xxxxxxxxxx
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>
> ---

Looks good to me,
Reviewed-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

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

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.

Those are quite subtle and perhaps worth some attention.
But technically, should not be a blocker for the patch.

-- 
Cheers,
Harry / Hyeonggon

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature