Re: [PATCH RFC 00/15] mm/slab: introduce alloc_flags and slab_alloc_context

[Vlastimil Babka (SUSE)]

On 6/10/26 15:46, Harry Yoo wrote:
> 
> 
> On 6/9/26 6:17 PM, Vlastimil Babka (SUSE) wrote:
>> This series is based on slab/for-next. If all goes well, it would
>> hopefully go to slab/for-next soon after the 7.2 merge window, so any
>> other work can be based on it to avoid conflicts, as it touches a lot
>> parts of slab.
>> 
>> Git: https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git/log/?h=b4/slab_alloc_flags
>> 
>> The slab implementation currently relies on gfp flags to convey
>> some context information internally:
>> 
>> - The absence of both __GFP_RECLAIM flags is interpreted as "cannot spin
>>   on locks", and intended to be used by kmalloc_nolock(). But false
>>   positives are possible e.g. during early boot where gfp_allowed_mask
>>   clears __GFP_RECLAIM from all allocations. This leads to unnecessary
>>   allocation failures and workarounds such as fd3634312a04 ("debugobject:
>>   Make it work with deferred page initialization - again").
>> 
>> - __GFP_NO_OBJ_EXT exists and takes up valuable bit in the gfp flags
>>   space, only to prevent recursive kmalloc() allocations for obj_ext
>>   arrays and sheaves.
> 
> [ Cc'ing Vishal and Matthew as it's somewhat relevant to memdescs... ]
> 
> When the page allocator starts allocateing slab objects,
> we still need a way to avoid recursion for obj_ext arrays and sheaves
> (by passing SLAB_ALLOC_NO_RECURSE).
> 
> Looking at kmalloc_flags(), probably we'll end up introducing a separate
> gfp type for slab-specific flags?

What do you mean by separate gfp type?

> Hmm but SLAB_ALLOC_* flags are defined in mm/slab.h and kmalloc_flags()
> is defined in include/linux/slab.h. Do yo intend to restrict the slab
> alloc flags to MM only?

Yeah I don't expect users outside MM. If a valid one appears, we can move
it. I should try moving kmalloc_flags() to mm/slab.h as well, unless there's
some header dependency issue that will prevent it.

>> The page allocator uses its internal alloc_flags to convey various
>> context information, including ALLOC_TRYLOCK (meaning "cannot spin").
>> This series copies that concept for the slab allocator, with its own
>> slab-specific internal flags:
>> 
>> - SLAB_ALLOC_DEFAULT - no extra flags (the value is 0), but explicit
>> - SLAB_ALLOC_TRYLOCK - do not spin on locks (used by kmalloc_nolock())
>> - SLAB_ALLOC_NEW_SLAB - replacing existing 'bool new_slab' parameter
>> 			for allocating obj_ext arrays
>> - SLAB_ALLOC_NO_RECURSE - replacing usage of __GFP_NO_OBJ_EXT
>> 
>> To reduce the amount of parameters in various internal functions, we
>> additionally introduce slab_alloc_context (also inspired by page
>> allocator's alloc_context) for passing a number of existing arguments
>> and the new alloc_flags:
>> 
>> /* Structure holding extra parameters for slab allocations */
>> struct slab_alloc_context {
>> 	unsigned long caller_addr;
>> 	unsigned long orig_size;
>> 	unsigned int alloc_flags;
>> 	struct list_lru *lru;
>> };
> 
> Perhaps beyond the scope of the patchset, but I wonder if we could have
> something like struct slab_alloc_context but for kmalloc callers to
> simplify {PASS,DECL}_KMALLOC_PARAMS().
> 
> Something like:
> 
> struct kmalloc_params {
> #ifdef CONFIG_SLAB_BUCKETS
> 	kmem_buckets *b;
> #endif
> #ifdef CONFIG_KMALLOC_PARTITION_CACHES
> 	kmalloc_token_t token;
> #endif
> };
> 
> The idea is to move optional kmalloc parameters (depending on config)
> into a single struct, instead of using the macros.
> 
> void *__kmalloc_node(size_t size, gfp_t flags, int node,
> 		     unsigned long caller,
> 		     struct kmalloc_params params);
> 
> void *kmalloc_node() {
>     /* ... snip ...*/
>     struct kmalloc_params params = KMALLOC_PARAMS(params.b, params.token);
>     return __kmalloc_node(size, flags, node, _RET_IP_, params);
> }
> 
> The compiler should optimize away unused fields based on the config.
> 
> Per System V AMD64 ABI, the compiler will use registers to pass the
> struct, as long as the struct size does not exceed 16 bytes.
> (Otherwise it will be passed on stack).

Hm but does this work on all architectures, and are we doing this somewhere
(for structures larger than a native word) already?
Also Marco noted earlier that gcc won't optimize away the struct if it
becomes zero-sized:

https://lore.kernel.org/all/CANpmjNO1aNm3mKphDGWasK_NUfVY8q4K9GCjyREZFqrOu9WLcw@xxxxxxxxxxxxxx/

>> This also replaces the existing struct partial_context.
>> 
>> The last necessary piece is kmalloc_flags() which can take the
>> alloc_flags in addition to gfp flags and is intended for the recursive
>> allocations of sheaves and obj_ext arrays, so that both
>> SLAB_ALLOC_TRYLOCK and SLAB_ALLOC_NO_RECURSE can be communicated.
>> Internally it decides between kmalloc_nolock() and normal kmalloc()
>> depending SLAB_ALLOC_TRYLOCK.
>> 
>> The rest of the series is gradually expanding the usage of both
>> alloc_flags and slab_alloc_context as necessary, with bits of
>> refactoring. Then, __GFP_NO_OBJ_EXT is removed completely.
>> 
>> Note that some usage of gfpflags_allow_spinning() relying on absence of
>> __GFP_RECLAIM remains outside of slab (and page allocator) in memcg,
>> page_owner and stackdepot code. These can thus yield false-positive
>> decisions that spinning is not allowed, but should not result in
>> important allocations failing anymore.
>> 
>> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>
>> ---
>> Vlastimil Babka (SUSE) (15):
>>       mm/slab: always zero only requested size on alloc
>>       mm/slab: stop inlining __slab_alloc_node()
>>       mm/slab: introduce slab_alloc_context
>>       mm/slab: introduce alloc_flags and SLAB_ALLOC_TRYLOCK
>>       mm/slab: add alloc_flags to slab_alloc_context
>>       mm/slab: replace struct partial_context with slab_alloc_context
>>       mm/slab: pass alloc_flags to new slab allocation
>>       mm/slab: pass alloc_flags through slab_post_alloc_hook() chain
>>       mm/slab: replace slab_alloc_node() parameters with slab_alloc_context
>>       mm/slab: allow kmem_cache_alloc_bulk() with any gfp flags
>>       mm/slab: pass slab_alloc_context to __do_kmalloc_node()
>>       mm/slab: introduce kmalloc_flags()
>>       mm/slab: remove __GFP_NO_OBJ_EXT usage from alloc_slab_obj_exts()
>>       mm/slab: replace __GFP_NO_OBJ_EXT with SLAB_ALLOC_NO_RECURSE for sheaves
>>       mm: remove the __GFP_NO_OBJ_EXT flag
>> 
>>  include/linux/gfp_types.h       |   7 -
>>  include/linux/slab.h            |  14 +-
>>  include/trace/events/mmflags.h  |  10 +-
>>  lib/alloc_tag.c                 |   2 +-
>>  mm/kfence/core.c                |   6 +-
>>  mm/memcontrol.c                 |   5 +-
>>  mm/slab.h                       |  16 +-
>>  mm/slub.c                       | 423 ++++++++++++++++++++++++----------------
>>  tools/include/linux/gfp_types.h |   7 -
>>  9 files changed, 288 insertions(+), 202 deletions(-)
>> ---
>> base-commit: 500b2c9755301742bdbb61249511ac11a4665dae
>> change-id: 20260601-slab_alloc_flags-25c782b0c57c
>> 
>> Best regards,
>> --  
>> Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>
>