Re: [PATCH] mm/slub: fix lost local objects when bulk remote free batch fills

[Harry Yoo <harry@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2469 bytes --]

Hi Shengming,

On 7/4/26 5:04 PM, hu.shengming@zte.com.cn wrote:
> From: Shengming Hu <hu.shengming@zte.com.cn>
> 
> In free_to_pcs_bulk(), when remote_objects[] fills to PCS_BATCH_MAX,
> the code jumps to flush_remote to free the batch. If all remote entries
> have already been compacted out of p[] via tail swaps while local objects
> remain, the flush_remote path returns early since `i < size` no longer
> holds. The leftover local objects are then neither cached in the sheaf 
> nor returned to the slab freelist, causing a memory leak.

Ouch.

Interestingly, we fixed a similar memory leak in free_to_pcs_bulk()
during the v6.18 cycle; commit cbcff934fa7d ("mm/slub: fix memory leak
in free_to_pcs_bulk()").

Hmm, I feel is a signal that the control flow is quite complicated and
worth revisiting if we could simplify the logic there. I'm not saying we
should do that to fix the bug, just mentioning.

> For illustration:
>  size = 64, local objects at p[0..31], remote objects at p[32..63]
>  After scanning all remotes: i = 32, size = 32
>  p[0..31] local objects are dropped.

I can see that could indeed happen.
Interesting. How has it been undiscovered until today?

Well, for this to trigger, at least PCS_BATCH_MAX (hardcoded to 32)
objects in the sheaf should be from remote nodes.

Looking at kmem_cache_free_bulk() users:
- maple_node has sheaf_capacity = 32
- skbuff_head_cache has sheaf_capacity = 28
- panthor and msm drivers have sheaf_capacity = 4

The capacity is (at least for now) purely based on the object size
(with user-requested capacity as a minimum)

Only maple_node cache (out of four users) has
sheaf_capacity >= PCS_BATCH_MAX.

However, for this to trigger in maple_node cache, ALL objects in the
sheaf should have been from remote nodes. And that means there is no
local object to leak :)

So, although the logic has a leak, you cannot trigger this in reality yet.

> Replace the goto-based flush with an in-place free of full remote batches
> during the scan, then resume processing the compacted array. All local
> objects now go through the normal fast path, and the tail path only
> handles any leftover partial remote batch. The redundant next_remote_batch
> jump label is removed as well.
> 
> Fixes: <989b09b73978>("slab: skip percpu sheaves for remote object freeing")
> Signed-off-by: Shengming Hu <hu.shengming@zte.com.cn>
> ---
-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]