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@xxxxxxxxxxxxx>
Makes sense to me.
Acked-by: Johannes Weiner <hannes@xxxxxxxxxxx>
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().
