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

[hu.shengming]

Harry wrote:
> Hi Shengming,

Hi Harry,

> On 7/4/26 5:04 PM, hu.shengming@xxxxxxxxxx wrote:
> > From: Shengming Hu <hu.shengming@xxxxxxxxxx>
> > 
> > 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.
> 

Yes, this is also a memory leak. I agree that the control flow in
free_to_pcs_bulk() is fairly subtle, especially considering the previous
leak fix.

My intention with this patch is only to remove this problematic path with
a localized change.

> > 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.
> 

Thanks for the detailed analysis!

Yes, this was found by code review rather than from a runtime report, and
I can not verified it with an actual triggering workload.

I agree that it does not seem triggerable with the current users. This fix
is mainly intended to make the code robust against possible future users or
changes to PCS_BATCH_MAX  :)

> > 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@xxxxxxxxxx>

--
With Best Regards,
Shengming