Re: [PATCH v2 08/16] mm/slab: pass alloc_flags to new slab allocation

[Vlastimil Babka (SUSE)]

On 6/11/26 09:52, Harry Yoo wrote:
> 
> 
> On 6/11/26 12:40 AM, Vlastimil Babka (SUSE) wrote:
>> Add the alloc_flags parameter to allocate_slab() and new_slab()
>> so it can be used to determine if spinning is allowed, independently
>> from gfp flags.
>> 
>> refill_objects() passes SLAB_ALLOC_DEFAULT because it can only be
>> reached from contexts that allow spinning.
>> 
>> Also change how trynode_flags are constructed in ___slab_alloc() to
>> achieve the same "do not upgrade to GFP_NOWAIT" by using masking instead
>> of a branch. It will now also not upgrade in cases where gfp is weaker
>> than GFP_NOWAIT (i.e. lacks __GFP_KSWAPD_RECLAIM) but doesn't come from
>> kmalloc_nolock() - which is more correct anyway.
> 
> Wait, debugobjects intentionally avoids __GFP_KSWAPD_RECLAIM,
> but we have been upgrading it to GFP_NOWAIT?

Actually, we have not been upgrading it until patch 6/16, which made the
upgrade trigger by starting to rely on alloc_flags? Because previously it
would be !allow_spin due to lack of __GFP_KSWAPD_RECLAIM.

So I will move that flags adjustment to 6/16 (now 7/16).

>> During the masking keep also existing __GFP_NOMEMALLOC (pointed out by
>> Sashiko) and __GFP_ACCOUNT. Previously the hardcoded GFP_NOWAIT would
>> eliminate them, but it's not a big problem that would need a separate
>> fix.
> 
> Ack.
> 
>> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>
>> ---
>>  mm/slub.c | 28 ++++++++++++++--------------
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>> 
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 98b79e5e7679..8f6ca3d5fdfa 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -4467,25 +4470,22 @@ static void *___slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>>  	 * 1) try to get a partial slab from target node only by having
>>  	 *    __GFP_THISNODE in pc.flags for get_from_partial()
>>  	 * 2) if 1) failed, try to allocate a new slab from target node with
>> -	 *    GPF_NOWAIT | __GFP_THISNODE opportunistically
>> +	 *    (at most) GFP_NOWAIT | __GFP_THISNODE opportunistically
>>  	 * 3) if 2) failed, retry with original gfpflags which will allow
>>  	 *    get_from_partial() try partial lists of other nodes before
>>  	 *    potentially allocating new page from other nodes
>>  	 */
>>  	if (unlikely(node != NUMA_NO_NODE && !(gfpflags & __GFP_THISNODE)
>>  		     && try_thisnode)) {
>> -		if (unlikely(!allow_spin))
>> -			/* Do not upgrade gfp to NOWAIT from more restrictive mode */
>> -			trynode_flags = gfpflags | __GFP_THISNODE;
>> -		else
>> -			trynode_flags = GFP_NOWAIT | __GFP_THISNODE;
>> +		trynode_flags &= GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_ACCOUNT;
>> +		trynode_flags |= __GFP_NOWARN | __GFP_THISNODE;
>>  	}
>