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

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

[hu.shengming]

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.

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.

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>
---
 mm/slub.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 9f754cf1c187..4d7e3067a5d6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6193,7 +6193,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	void *remote_objects[PCS_BATCH_MAX];
 	unsigned int remote_nr = 0;

-next_remote_batch:
 	while (i < size) {
 		struct slab *slab = virt_to_slab(p[i]);

@@ -6208,8 +6207,11 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 		if (unlikely(!can_free_to_pcs(slab))) {
 			remote_objects[remote_nr] = p[i];
 			p[i] = p[--size];
-			if (++remote_nr >= PCS_BATCH_MAX)
-				goto flush_remote;
+			if (++remote_nr >= PCS_BATCH_MAX) {
+				__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
+				stat_add(s, FREE_SLOWPATH, remote_nr);
+				remote_nr = 0;
+			}
 			continue;
 		}

@@ -6293,10 +6295,6 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 	if (remote_nr) {
 		__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
 		stat_add(s, FREE_SLOWPATH, remote_nr);
-		if (i < size) {
-			remote_nr = 0;
-			goto next_remote_batch;
-		}
 	}
 }

-- 
2.25.1

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

[Harry Yoo]

[-- 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 --]

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

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@zte.com.cn>

--
With Best Regards,
Shengming

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

[Harry Yoo]

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



On 7/5/26 10:37 PM, hu.shengming@zte.com.cn wrote:
> Harry wrote:
>> Hi Shengming,
> 
> Hi Harry,
> 
>> 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.
> 
> 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.

That's fair!

>>> 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  :)

Could we have these explanations/analysis above
in the commit message please? :)

But yeah, we should fix this.

When fixing a bug usually I prefer to go through the process...

 1) Confirm the bug exists by reproducing it, and
 2) Confirm that the patch indeed fixes the bug

But this one is impossible to reproduce, uh.

In this case I think it's better not to Cc stable (which is already the
case). In case there is an error in the analysis, it's still
straightforward to backport later.

I will double check if the patch is indeed correct.

Thanks!

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

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

[hu.shengming]

Harry wrote:
> On 7/5/26 10:37 PM, hu.shengming@zte.com.cn wrote:
> > Harry wrote:
> >> Hi Shengming,
> > 
> > Hi Harry,
> > 
> >> 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.
> > 
> > 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.
> 
> That's fair!
> 
> >>> 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  :)
> 
> Could we have these explanations/analysis above
> in the commit message please? :)
> 

Sure, I'll fold this full analysis into the commit message in v2.  :)

> But yeah, we should fix this.
> 
> When fixing a bug usually I prefer to go through the process...
> 
>  1) Confirm the bug exists by reproducing it, and
>  2) Confirm that the patch indeed fixes the bug
> 
> But this one is impossible to reproduce, uh.
> 
> In this case I think it's better not to Cc stable (which is already the
> case). In case there is an error in the analysis, it's still
> straightforward to backport later.

Agreed. No stable Cc for now given no known real-world trigger path; it can
be backported easily later if needed.

> I will double check if the patch is indeed correct.
> 
> Thanks!

Thanks a lot for reviewing and double-checking!

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

--
With Best Regards,
Shengming