* Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()
2026-07-10 12:04 [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk() Vlastimil Babka (SUSE)
@ 2026-07-10 12:17 ` Pedro Falcato
2026-07-13 2:38 ` hu.shengming
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2026-07-10 12:17 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Harry Yoo, Andrew Morton, Hao Li, Shengming Hu, Christoph Lameter,
David Rientjes, Roman Gushchin, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()
2026-07-10 12:04 [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk() Vlastimil Babka (SUSE)
2026-07-10 12:17 ` Pedro Falcato
@ 2026-07-13 2:38 ` hu.shengming
2026-07-13 5:56 ` Harry Yoo
2026-07-13 6:33 ` Hao Li
3 siblings, 0 replies; 5+ messages in thread
From: hu.shengming @ 2026-07-13 2:38 UTC (permalink / raw)
To: vbabka, pfalcato
Cc: harry, akpm, hao.li, cl, rientjes, roman.gushchin, linux-mm,
linux-kernel, vbabka
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>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()
2026-07-10 12:04 [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk() Vlastimil Babka (SUSE)
2026-07-10 12:17 ` Pedro Falcato
2026-07-13 2:38 ` hu.shengming
@ 2026-07-13 5:56 ` Harry Yoo
2026-07-13 6:33 ` Hao Li
3 siblings, 0 replies; 5+ messages in thread
From: Harry Yoo @ 2026-07-13 5:56 UTC (permalink / raw)
To: Vlastimil Babka (SUSE), Andrew Morton
Cc: Hao Li, Shengming Hu, Christoph Lameter, David Rientjes,
Roman Gushchin, linux-mm, linux-kernel
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk()
2026-07-10 12:04 [PATCH slab/for-next] mm/slab: simplify freeing remote objects in free_to_pcs_bulk() Vlastimil Babka (SUSE)
` (2 preceding siblings ...)
2026-07-13 5:56 ` Harry Yoo
@ 2026-07-13 6:33 ` Hao Li
3 siblings, 0 replies; 5+ messages in thread
From: Hao Li @ 2026-07-13 6:33 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Harry Yoo, Andrew Morton, Shengming Hu, Christoph Lameter,
David Rientjes, Roman Gushchin, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread