It has been noted that free_to_pcs_bulk() is difficult to follow, with a number of goto labels, and this has contributed to two memory leak bugs in there. Extract part of the code to __free_to_pcs_batch(), which focuses only on freeing free-hook-processed local objects to a percpu sheaf, and returning how many were freed. Zero means a trylock failure or no empty sheaf available, and thus the caller should fallback to __kmem_cache_free_bulk(). Make free_to_pcs_bulk() call this in a while loop, removing all goto labels from the function. __free_to_pcs_batch() retains two rather straightforward ones. Signed-off-by: Vlastimil Babka (SUSE) --- A followup refactoring to Shengming's fix. --- mm/slub.c | 116 +++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 61 insertions(+), 55 deletions(-) diff --git a/mm/slub.c b/mm/slub.c index 65febe957886..3f13f497aab4 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -6203,51 +6203,21 @@ static __always_inline bool can_free_to_pcs(struct slab *slab) } /* - * Bulk free objects to the percpu sheaves. - * Unlike free_to_pcs() this includes the calls to all necessary hooks - * and the fallback to freeing to slab pages. + * Try to free as many objects (already processed by free hooks) as possible to + * a single per-cpu sheaf. + * + * Returns how many objects were freed. Zero means failure and the caller should + * fall back to __kmem_cache_free_bulk(). */ -static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p) +static unsigned int __free_to_pcs_batch(struct kmem_cache *s, size_t size, void **p) { struct slub_percpu_sheaves *pcs; struct slab_sheaf *main, *empty; - bool init = slab_want_init_on_free(s); - unsigned int batch, i = 0; struct node_barn *barn; - void *remote_objects[PCS_BATCH_MAX]; - unsigned int remote_nr = 0; - - while (i < size) { - struct slab *slab = virt_to_slab(p[i]); - - memcg_slab_free_hook(s, slab, p + i, 1); - alloc_tagging_slab_free_hook(s, slab, p + i, 1); - - if (unlikely(!slab_free_hook(s, p[i], init, false))) { - p[i] = p[--size]; - continue; - } - - 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; - } - - i++; - } - - if (!size) - goto flush_remote; + unsigned int batch; -next_batch: if (!local_trylock(&s->cpu_sheaves->lock)) - goto fallback; + return 0; pcs = this_cpu_ptr(s->cpu_sheaves); @@ -6293,29 +6263,65 @@ static void free_to_pcs_bulk(struct kmem_cache *s, size_t size, void **p) stat_add(s, FREE_FASTPATH, batch); - if (batch < size) { - p += batch; - size -= batch; - goto next_batch; + return batch; + +no_empty: + local_unlock(&s->cpu_sheaves->lock); + + return 0; +} + +/* + * Bulk free objects to the percpu sheaves. + * Unlike free_to_pcs() this includes the calls to all necessary hooks + * and the fallback to freeing to slab pages. + */ +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]; + unsigned int remote_nr = 0; + + for (unsigned int i = 0; i < size;) { + struct slab *slab = virt_to_slab(p[i]); + + memcg_slab_free_hook(s, slab, p + i, 1); + alloc_tagging_slab_free_hook(s, slab, p + i, 1); + + if (unlikely(!slab_free_hook(s, p[i], init, false))) { + p[i] = p[--size]; + continue; + } + + 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; + } + + i++; } - if (remote_nr) - goto flush_remote; + while (size) { + unsigned int batch_freed = __free_to_pcs_batch(s, size, p); - return; + if (!batch_freed) + break; -no_empty: - local_unlock(&s->cpu_sheaves->lock); + p += batch_freed; + size -= batch_freed; + } - /* - * if we depleted all empty sheaves in the barn or there are too - * many full sheaves, free the rest to slab pages - */ -fallback: - __kmem_cache_free_bulk(s, size, p); - stat_add(s, FREE_SLOWPATH, size); + if (size) { + __kmem_cache_free_bulk(s, size, p); + stat_add(s, FREE_SLOWPATH, size); + } -flush_remote: if (remote_nr) { __kmem_cache_free_bulk(s, remote_nr, &remote_objects[0]); stat_add(s, FREE_SLOWPATH, remote_nr); --- base-commit: 72bb229f9161a1efcd5df32141b69fcc6ae81a13 change-id: 20260707-slab-simplify-bulk-pcs-a8d0478feef6 Best regards, -- Vlastimil Babka (SUSE)