Re: [PATCH] mm/slab: fix an incorrect check in obj_exts_alloc_size()

[vbabka]

On 3/10/26 04:25, Harry Yoo wrote:
> On Mon, Mar 09, 2026 at 03:00:17PM +0100, vbabka@xxxxxxxxxx wrote:
>> On 3/9/26 08:22, Harry Yoo wrote:
>> > obj_exts_alloc_size() prevents recursive allocation of slabobj_ext
>> > array from the same cache, to avoid creating slabs that are never freed.
>> > 
>> > There is one mistake that returns the original size when memory
>> > allocation profiling is disabled. The assumption was that
>> > memcg-triggered slabobj_ext allocation is always served from
>> > KMALLOC_CGROUP type. But this is wrong [1]: when the caller specifies
>> > both __GFP_RECLAIMABLE and __GFP_ACCOUNT with SLUB_TINY enabled, the
>> > allocation is served from normal kmalloc. This is because kmalloc_type()
>> > prioritizes __GFP_RECLAIMABLE over __GFP_ACCOUNT, and SLUB_TINY aliases
>> > KMALLOC_RECLAIM with KMALLOC_NORMAL.
>> 
>> Hm that's suboptimal (leads to sparsely used obj_exts in normal kmalloc
>> slabs) and maybe separately from this hotfix we could make sure that with
>> SLUB_TINY, __GFP_ACCOUNT is preferred going forward?
> 
> To be honest, I don't a have strong opinion on that.
> 
> Is grouping by mobility (for anti-fragmentation less) important on
> SLUB_TINY systems?

Yeah, that's why "KMALLOC_RECLAIM = KMALLOC_NORMAL" there. So prioritizing
__GFP_RECLAIMABLE does nothing there, it goes to the same kmalloc_normal
cache. It only results in ignoring KMALLOC_CGROUP.
(I think in practice SLUB_TINY systems wouldn't enabled CONFIG_MEMCG either,
so it's a low priority, but still logical imho).

>> > As a result, the recursion guard is bypassed and the problematic slabs
>> > can be created. Fix this by removing the mem_alloc_profiling_enabled()
>> > check entirely. The remaining is_kmalloc_normal() check is still
>> > sufficient to detect whether the cache is of KMALLOC_NORMAL type and
>> > avoid bumping the size if it's not.
>> > 
>> > Without SLUB_TINY, no functional change intended.
>> > With SLUB_TINY, allocations with __GFP_ACCOUNT|__GFP_RECLAIMABLE
>> > now allocate a larger array if the sizes equal.
>> > 
>> > Reported-by: Zw Tang <shicenci@xxxxxxxxx>
>> > Fixes: 280ea9c3154b ("mm/slab: avoid allocating slabobj_ext array from its own slab")
>> > Closes: https://lore.kernel.org/linux-mm/CAPHJ_VKuMKSke8b11AZQw1PTSFN4n2C0gFxC6xGOG0ZLHgPmnA@xxxxxxxxxxxxxx
>> > Cc: stable@xxxxxxxxxxxxxxx
>> > Signed-off-by: Harry Yoo <harry.yoo@xxxxxxxxxx>
>> 
>> Added to slab/for-next-fixes, thanks!
> 
> Thanks!
>