Re: [PATCH for-next-fixes v2 4/5] mm/slab: fix a deadlock in memcg_alloc_abort_single()

[Vlastimil Babka (SUSE)]

On 11/3/26 22:11, Alexei Starovoitov wrote:
> On Tue, Nov 03, 2026 at 12:21 PM Harry Yoo <harry@xxxxxxxxxx> wrote:
>> While this is technically true, this relies too much on the current
>> implementation details and makes the code hard to follow.
> 
> Fair enough. I don't insist.
> 
>> Patch 2 is an independent bugfix as-is already. We should really stop
>> open-coding slab_free_hook(). That ended up missing init_on_free handing.
> 
> Right. No objection to patch 2.
> With patches 2 and 3 the !allow_spin side of memcg_alloc_abort_single()
> is an open coded kfree_nolock():
> 
>   kfree_nolock()                    memcg_alloc_abort_single(!allow_spin)
>   --------------------------------  -------------------------------------
>   memcg_slab_free_hook()            -
>   alloc_tagging_slab_free_hook()    alloc_tagging_slab_free_hook()
>   slab_free_hook(!allow_spin)       slab_free_hook(!allow_spin)
>   free_to_pcs()                     -
>   defer_free()                      defer_free()
> 
> I don't insist on kfree_nolock() there,
> but can we try free_to_pcs() before defer_free() ?
> Like:
> 
> 	if (likely(allow_spin))
> 		__slab_free(s, slab, object, object, 1, _RET_IP_);
> 	else if (!can_free_to_pcs(slab) || !free_to_pcs(s, object, false))
> 		defer_free(s, object);

Why not try free_to_pcs with allow_spin as well then, and improve
memcg_alloc_abort_single() for both contexts?