Re: [PATCH bpf-next 1/4] slab: Introduce kmem_cache_alloc_nolock()

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Fri May 29, 2026 at 2:20 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] Deadlock in NMI context due to synchronous freeing on memcg charging failure
> --
>
> commit 831b8a379d6dfbde2570286ba8453174bc236695
> Author: Alexei Starovoitov <ast@kernel.org>
>
> slab: Introduce kmem_cache_alloc_nolock()
>
> This commit introduces kmem_cache_alloc_nolock_noprof() and its wrapper 
> kmem_cache_alloc_nolock(). It is designed to allocate an object from a
> specific cache and aims to be safe to call from any context, including NMI
> and IRQ-off environments.
>
>> diff --git a/mm/slub.c b/mm/slub.c
>> index a2bf3756ca7d0..601986aaebdfc 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
> [ ... ]
>> @@ -5402,6 +5402,45 @@ void *kmalloc_nolock_noprof(size_t size, gfp_t gfp_flags, int node)
>>  }
>>  EXPORT_SYMBOL_GPL(kmalloc_nolock_noprof);
>>  
>> +/**
>> + * kmem_cache_alloc_nolock - Allocate one object from a specific cache,
>> + * safe from any context (including NMI/IRQ-off), like kmalloc_nolock().
>> + *
>> + * Returns NULL on failure (including the trylock paths that may transiently
>> + * fail under contention).
>> + */
>> +void *kmem_cache_alloc_nolock_noprof(struct kmem_cache *s, gfp_t gfp_flags,
>> +				     int node)
>> +{
>> +	gfp_t alloc_gfp = __GFP_NOWARN | __GFP_NOMEMALLOC | gfp_flags;
>> +	void *ret;
>> +
>> +	VM_WARN_ON_ONCE(gfp_flags & ~(__GFP_ACCOUNT | __GFP_ZERO |
>> +				      __GFP_NO_OBJ_EXT));
> [ ... ]
>> +	maybe_wipe_obj_freeptr(s, ret);
>> +	slab_post_alloc_hook(s, NULL, alloc_gfp, 1, &ret,
>> +			     slab_want_init_on_alloc(alloc_gfp, s),
>> +			     s->object_size);
>
> [Severity: Critical]
> Does this sequence risk a hard deadlock if called from NMI context?
>
> The function permits __GFP_ACCOUNT, but if memory cgroup charging fails
> inside memcg_slab_post_alloc_hook(), it synchronously aborts the allocation:
>
> mm/slub.c:memcg_slab_post_alloc_hook() {
>     ...
>     if (memcg_alloc_charge_slab(s, alloc_gfp, lru, p, size)) {
>         for (i = 0; i < size; i++)
>             memcg_alloc_abort_single(s, p[i]);
>     ...
> }
>
> This calls memcg_alloc_abort_single(), which immediately attempts to free
> the object:
>
> mm/slub.c:memcg_alloc_abort_single() {
>     ...
>     if (likely(slab_free_hook(s, object, slab_want_init_on_free(s), false)))
>         __slab_free(s, slab, object, object, 1, _RET_IP_);
>     ...
> }
>
> Both slab_free_hook() (via kmemleak and kfence hooks) and __slab_free()
> will attempt to acquire non-NMI-safe spinlocks like n->list_lock. If the
> interrupted context was holding these locks, wouldn't this synchronous free
> cause a deadlock, breaking the "safe from any context" guarantee?

sashiko should have said that it's a pre-existing issue,
since kmalloc_nolock() follows exact same pattern.
Also patch 3 switches __slab_free() to use trylock.
So not really an issue.