Re: [PATCH 1/5] mm: Separate folio_split_memcg() from split_page_memcg()

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Mar 13, 2025 at 02:58:50PM +0000, Matthew Wilcox (Oracle) wrote:
> Folios always use memcg_data to refer to the mem_cgroup while pages
> allocated with GFP_ACCOUNT have a pointer to the obj_cgroup.  Since the
> caller already knows what it has, split the function into two and then
> we don't need to check.
> 
> Move the assignment of split folio memcg_data to the point where we set
> up the other parts of the new folio.  That leaves folio_split_memcg()
> just handling the memcg accounting.
> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>

Makes sense to me.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

One small nit:

> @@ -3101,10 +3101,19 @@ void split_page_memcg(struct page *head, int old_order, int new_order)
>  	for (i = new_nr; i < old_nr; i += new_nr)
>  		folio_page(folio, i)->memcg_data = folio->memcg_data;
>  
> -	if (folio_memcg_kmem(folio))
> -		obj_cgroup_get_many(__folio_objcg(folio), old_nr / new_nr - 1);
> -	else
> -		css_get_many(&folio_memcg(folio)->css, old_nr / new_nr - 1);
> +	obj_cgroup_get_many(__folio_objcg(folio), old_nr / new_nr - 1);
> +}
> +
> +void folio_split_memcg(struct folio *folio, unsigned old_order,
> +		unsigned new_order)
> +{
> +	unsigned new_refs;
> +
> +	if (mem_cgroup_disabled() || !folio_memcg_charged(folio))
> +		return;
> +
> +	new_refs = (1 << (old_order - new_order)) - 1;
> +	css_get_many(&__folio_memcg(folio)->css, new_refs);
>  }

Since this doesn't do the full memcg split anymore, and there is still
a split_page_memcg() which does, it would be good to reflect this in
the name. E.g. folio_split_memcg_refs().