[PATCH] mm/slab: extract __free_to_pcs_batch() from free_to_pcs_bulk()

[Vlastimil Babka (SUSE)]

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