[PATCH] memcg: sink mem_cgroup_uncharge_folios() into free_unref_folios()

[PATCH] memcg: sink mem_cgroup_uncharge_folios() into free_unref_folios()

[Ridong Chen]

From: Ridong Chen <chenridong@xiaomi.com>

Every caller of free_unref_folios() invokes mem_cgroup_uncharge_folios()
on the same batch immediately beforehand.  This pattern is duplicated
across shrink_folio_list(), move_folios_to_lru(), folio_batch_move_lru()
and folios_put_refs().

Move the uncharge into free_unref_folios() itself so the batch is
uncharged in one place before the folios are freed.  This removes the
repeated boilerplate at every call site and makes it impossible to free
a batch without uncharging it first.  No functional change intended.

Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
---
 mm/folio.c      | 5 +----
 mm/page_alloc.c | 1 +
 mm/vmscan.c     | 7 +------
 3 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/mm/folio.c b/mm/folio.c
index c02dcea9c03c..2edbba47be1e 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -199,10 +199,8 @@ static void folio_batch_move_lru(struct folio_batch *fbatch, move_fn_t move_fn)
 		lruvec_unlock_irqrestore(lruvec, flags);
 
 	/* Cleanup filtered dead folios. */
-	if (is_lru_add) {
-		mem_cgroup_uncharge_folios(&free_fbatch);
+	if (is_lru_add)
 		free_unref_folios(&free_fbatch);
-	}
 
 	folios_put(fbatch);
 }
@@ -1030,7 +1028,6 @@ void folios_put_refs(struct folio_batch *folios, unsigned int *refs)
 	}
 
 	folios->nr = j;
-	mem_cgroup_uncharge_folios(folios);
 	free_unref_folios(folios);
 }
 EXPORT_SYMBOL(folios_put_refs);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 12fac9084c48..fb1ecab0ee78 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3013,6 +3013,7 @@ void free_unref_folios(struct folio_batch *folios)
 	struct zone *locked_zone = NULL;
 	int i, j;
 
+	mem_cgroup_uncharge_folios(folios);
 	/* Prepare folios for freeing */
 	for (i = 0, j = 0; i < folios->nr; i++) {
 		struct folio *folio = folios->folios[i];
diff --git a/mm/vmscan.c b/mm/vmscan.c
index e572d2742c8c..113e2e49447b 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1506,7 +1506,6 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 
 		folio_unqueue_deferred_split(folio);
 		if (folio_batch_add(&free_folios, folio) == 0) {
-			mem_cgroup_uncharge_folios(&free_folios);
 			try_to_unmap_flush();
 			free_unref_folios(&free_folios);
 		}
@@ -1575,7 +1574,6 @@ static unsigned int shrink_folio_list(struct list_head *folio_list,
 
 	pgactivate = stat->nr_activate[0] + stat->nr_activate[1];
 
-	mem_cgroup_uncharge_folios(&free_folios);
 	try_to_unmap_flush();
 	free_unref_folios(&free_folios);
 
@@ -1902,7 +1900,6 @@ static unsigned int move_folios_to_lru(struct list_head *list)
 			folio_unqueue_deferred_split(folio);
 			if (folio_batch_add(&free_folios, folio) == 0) {
 				lruvec_unlock_irq(lruvec);
-				mem_cgroup_uncharge_folios(&free_folios);
 				free_unref_folios(&free_folios);
 				lruvec = NULL;
 			}
@@ -1920,10 +1917,8 @@ static unsigned int move_folios_to_lru(struct list_head *list)
 	if (lruvec)
 		lruvec_unlock_irq(lruvec);
 
-	if (free_folios.nr) {
-		mem_cgroup_uncharge_folios(&free_folios);
+	if (free_folios.nr)
 		free_unref_folios(&free_folios);
-	}
 
 	return nr_moved;
 }
-- 
2.34.1

Re: [PATCH] memcg: sink mem_cgroup_uncharge_folios() into free_unref_folios()

[Johannes Weiner]

On Thu, Aug 27, 2026 at 11:05:16AM +0800, Ridong Chen wrote:
> From: Ridong Chen <chenridong@xiaomi.com>
> 
> Every caller of free_unref_folios() invokes mem_cgroup_uncharge_folios()
> on the same batch immediately beforehand.  This pattern is duplicated
> across shrink_folio_list(), move_folios_to_lru(), folio_batch_move_lru()
> and folios_put_refs().
> 
> Move the uncharge into free_unref_folios() itself so the batch is
> uncharged in one place before the folios are freed.  This removes the
> repeated boilerplate at every call site and makes it impossible to free
> a batch without uncharging it first.  No functional change intended.

Hm, IMO this is confusing. You're making uncharging an included
service in a subset of the page allocator freeing API. Batch freeing
uncharges, but single page freeing does not.

I don't think saving 7 lines is worth that API caveat.

Re: [PATCH] memcg: sink mem_cgroup_uncharge_folios() into free_unref_folios()

[Ridong Chen]

On 8/27/2026 10:52 PM, Johannes Weiner wrote:
> On Thu, Aug 27, 2026 at 11:05:16AM +0800, Ridong Chen wrote:
>> From: Ridong Chen <chenridong@xiaomi.com>
>>
>> Every caller of free_unref_folios() invokes mem_cgroup_uncharge_folios()
>> on the same batch immediately beforehand.  This pattern is duplicated
>> across shrink_folio_list(), move_folios_to_lru(), folio_batch_move_lru()
>> and folios_put_refs().
>>
>> Move the uncharge into free_unref_folios() itself so the batch is
>> uncharged in one place before the folios are freed.  This removes the
>> repeated boilerplate at every call site and makes it impossible to free
>> a batch without uncharging it first.  No functional change intended.
> 
> Hm, IMO this is confusing. You're making uncharging an included
> service in a subset of the page allocator freeing API. Batch freeing
> uncharges, but single page freeing does not.
> 
> I don't think saving 7 lines is worth that API caveat.

Thanks Johannes.

I thought this could improve cohesion, since we have to uncharge the batch when 
freeing unreferenced folios. However, I didn't consider the symmetry with single 
folio freeing.

I'll just drop the patch if it's causing confusion.

-- 
Best regards
Ridong