[PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[Vlastimil Babka (SUSE)]

We have been moving remote objects to an on-stack array and flushing it
when full. Instead, we can swap them towards the beginning of the
supplied array and bulk-free it just once.

Also add a comment to explain the rationale of freeing remote objects
last, because now it would appear to be simpler to free them first.

Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
---
One more cleanup I realized it's possible when processing feedback to
the previous one [1]

[1] https://lore.kernel.org/all/20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org/
---
 mm/slub.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 22045dc919ef..966d2fb114e8 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -6255,7 +6255,7 @@ static unsigned int __free_to_pcs_batch(struct kmem_cache *s, size_t size, void
 static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 {
 	bool init = slab_want_init_on_free(s);
-	void *remote_objects[PCS_BATCH_MAX];
+	void **remote_objects = p;
 	unsigned int remote_nr = 0;
 
 	for (unsigned int i = 0; i < size;) {
@@ -6270,19 +6270,17 @@ 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) {
-				__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
-				stat_add(s, FREE_SLOWPATH, remote_nr);
-				remote_nr = 0;
-			}
-			continue;
+			if (i != remote_nr)
+				swap(remote_objects[remote_nr], p[i]);
+			remote_nr++;
 		}
 
 		i++;
 	}
 
+	p += remote_nr;
+	size -= remote_nr;
+
 	while (size) {
 		unsigned int batch_freed = __free_to_pcs_batch(s, size, p);
 
@@ -6296,8 +6294,12 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
 		size -= batch_freed;
 	}
 
+	/*
+	 * Processing remote objects last decreases the chances of cpu migration
+	 * while freeing to sheaves and compromising object locality
+	 */
 	if (remote_nr) {
-		__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
+		__kmem_cache_free_bulk(s, remote_nr, remote_objects);
 		stat_add(s, FREE_SLOWPATH, remote_nr);
 	}
 }

---
base-commit: e1fa26489025d6deac76d1dbfe2e0720a3ad84b1
change-id: 20260710-bulk_free_remote-7fbe6f3abf5d

Best regards to everyone but Pedro (btw this is not hard-coded,
there's a config option for this),
--
Vlastimil Babka (SUSE) <vbabka@kernel.org>

Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[Pedro Falcato]

On Fri, Jul 10, 2026 at 02:04:58PM +0200, Vlastimil Babka (SUSE) wrote:
> We have been moving remote objects to an on-stack array and flushing it
> when full. Instead, we can swap them towards the beginning of the
> supplied array and bulk-free it just once.
> 
> Also add a comment to explain the rationale of freeing remote objects
> last, because now it would appear to be simpler to free them first.
> 
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> One more cleanup I realized it's possible when processing feedback to
> the previous one [1]
> 
> [1] https://lore.kernel.org/all/20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org/
> ---
>  mm/slub.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 22045dc919ef..966d2fb114e8 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6255,7 +6255,7 @@ static unsigned int __free_to_pcs_batch(struct kmem_cache *s, size_t size, void
>  static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  {
>  	bool init = slab_want_init_on_free(s);
> -	void *remote_objects[PCS_BATCH_MAX];
> +	void **remote_objects = p;
>  	unsigned int remote_nr = 0;
>  

Maybe explaining what magic you're pulling off would be useful in the code
as well.

>  	for (unsigned int i = 0; i < size;) {
> @@ -6270,19 +6270,17 @@ 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) {
> -				__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
> -				stat_add(s, FREE_SLOWPATH, remote_nr);
> -				remote_nr = 0;
> -			}
> -			continue;
> +			if (i != remote_nr)
> +				swap(remote_objects[remote_nr], p[i]);
> +			remote_nr++;
>  		}
>  
>  		i++;
>  	}
>  
> +	p += remote_nr;
> +	size -= remote_nr;
> +
>  	while (size) {
>  		unsigned int batch_freed = __free_to_pcs_batch(s, size, p);
>  
> @@ -6296,8 +6294,12 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  		size -= batch_freed;
>  	}
>  
> +	/*
> +	 * Processing remote objects last decreases the chances of cpu migration
> +	 * while freeing to sheaves and compromising object locality
> +	 */
>  	if (remote_nr) {
> -		__kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
> +		__kmem_cache_free_bulk(s, remote_nr, remote_objects);
>  		stat_add(s, FREE_SLOWPATH, remote_nr);
>  	}
>  }

All-in-all, LGTM

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

> 
> ---
> base-commit: e1fa26489025d6deac76d1dbfe2e0720a3ad84b1
> change-id: 20260710-bulk_free_remote-7fbe6f3abf5d
> 
> Best regards to everyone but Pedro (btw this is not hard-coded,
> there's a config option for this),

I know there's a config option to change your signature but I haven't
been able to simply omit this bit :|

Anyway, I get a stray, you get a stray review. Nice trade.

-- 
Pedro

Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[hu.shengming]

Vlastimil wrote:
> We have been moving remote objects to an on-stack array and flushing it
> when full. Instead, we can swap them towards the beginning of the
> supplied array and bulk-free it just once.
> 
> Also add a comment to explain the rationale of freeing remote objects
> last, because now it would appear to be simpler to free them first.
> 
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

Good idea!

> ---
> One more cleanup I realized it's possible when processing feedback to
> the previous one [1]
> 
> [1] https://lore.kernel.org/all/20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org/
> ---
>  mm/slub.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 22045dc919ef..966d2fb114e8 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -6255,7 +6255,7 @@ static unsigned int __free_to_pcs_batch(struct kmem_cache *s, size_t size, void
>  static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>  {
>      bool init = slab_want_init_on_free(s);
> -    void *remote_objects[PCS_BATCH_MAX];
> +    void **remote_objects = p;
>      unsigned int remote_nr = 0;
>  
>      for (unsigned int i = 0; i < size;) {
> @@ -6270,19 +6270,17 @@ 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) {
> -                __kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
> -                stat_add(s, FREE_SLOWPATH, remote_nr);
> -                remote_nr = 0;
> -            }
> -            continue;
> +            if (i != remote_nr)
> +                swap(remote_objects[remote_nr], p[i]);
> +            remote_nr++;
>          }
>  

I had the same thought as Pedro: the in-place partitioning is correct, but
the partitioning logic is not immediately obvious. A short comment documenting
the layout would make the code easier to understand and maintain, for example:

/*
 * Partition the array in place:
 * [0, remote_nr) contains processed non-PCS objects,
 * [remote_nr, i) contains processed PCS candidates, and
 * [i, size) contains unprocessed objects.
 */

With that tiny nit, the patch looks good to me.
Reviewed-by: Shengming Hu <hu.shengming@zte.com.cn>

--
With Best Regards,
Shengming

>          i++;
>      }
>  
> +    p += remote_nr;
> +    size -= remote_nr;
> +
>      while (size) {
>          unsigned int batch_freed = __free_to_pcs_batch(s, size, p);
>  
> @@ -6296,8 +6294,12 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p)
>          size -= batch_freed;
>      }
>  
> +    /*
> +     * Processing remote objects last decreases the chances of cpu migration
> +     * while freeing to sheaves and compromising object locality
> +     */
>      if (remote_nr) {
> -        __kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]);
> +        __kmem_cache_free_bulk(s, remote_nr, remote_objects);
>          stat_add(s, FREE_SLOWPATH, remote_nr);
>      }
>  }
> 
> ---
> base-commit: e1fa26489025d6deac76d1dbfe2e0720a3ad84b1
> change-id: 20260710-bulk_free_remote-7fbe6f3abf5d
> 
> Best regards to everyone but Pedro (btw this is not hard-coded,
> there's a config option for this),
> --
> Vlastimil Babka (SUSE) <vbabka@kernel.org>

Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[Harry Yoo]

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



On 7/10/26 9:04 PM, Vlastimil Babka (SUSE) wrote:
> We have been moving remote objects to an on-stack array and flushing it
> when full. Instead, we can swap them towards the beginning of the
> supplied array and bulk-free it just once.

Nice!

> Also add a comment to explain the rationale of freeing remote objects
> last, because now it would appear to be simpler to free them first.
> 
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---

Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>

-- 
Cheers,
Harry / Hyeonggon

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

Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()

[Hao Li]

On Fri, Jul 10, 2026 at 02:04:58PM +0200, Vlastimil Babka (SUSE) wrote:
> We have been moving remote objects to an on-stack array and flushing it
> when full. Instead, we can swap them towards the beginning of the
> supplied array and bulk-free it just once.
> 
> Also add a comment to explain the rationale of freeing remote objects
> last, because now it would appear to be simpler to free them first.
> 
> Signed-off-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
> ---
> One more cleanup I realized it's possible when processing feedback to
> the previous one [1]
> 
> [1] https://lore.kernel.org/all/20260707-slab-simplify-bulk-pcs-v1-1-4850dbe0d904@kernel.org/
> ---
>  mm/slub.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)

Nice idea!
Reviewed-by: Hao Li <hao.li@linux.dev>

-- 
Thanks,
Hao