Re: [PATCH v2 1/2] mm/huge_memory: use folio's memcg inside __folio_split()

On Mon, Aug 03, 2026 at 11:02:30AM -0400, Zi Yan wrote:
> On Sat Aug 1, 2026 at 5:36 AM EDT, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Jul 30, 2026 at 10:18:00PM -0400, Zi Yan wrote:
> >> During a pagecache folio split, an xarray node allocation can happen and
> >> needs to charge at folio's memcg instead of folio split invoker's memcg,
> >> because for example folio split can happen during reclaim and reclaim's
> >> active memcg might not be folio's memcg. Switch to folio's memcg at the
> >> beginning and switch back afterwards.
> >
> > I assume this is the only allocation? I guess in general it makes sense to have
> > the folio's memcg be active here regardless.
> >
> >>
> >> Suggested-by: Johannes Weiner <[email protected]>
> >> Fixes: 6b24ca4a1a8d4 ("mm: Use multi-index entries in the page cache")
> >
> > Cc: stable?
>
> Like you said above, only xas_split_alloc() is affected. And we have not
> seen related workingset regression report (like what Johannes reported
> in commit 7b785645e8f13 ("mm: fix page cache convergence regression")).
> It might be OK to not backport.
>
> Johannes, what is your take on this?
>
> >
> >> Signed-off-by: Zi Yan <[email protected]>
> >
> > Change seems reasonable overall.
> >
> > Acked-by: Lorenzo Stoakes (ARM) <[email protected]>
> >
> >> ---
> >>  mm/huge_memory.c | 20 ++++++++++++++++----
> >>  1 file changed, 16 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index 04e8a6b553435..b9c2d8908e564 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -4063,34 +4063,42 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
> >>  	XA_STATE(xas, &folio->mapping->i_pages, folio->index);
> >>  	struct folio *end_folio = folio_next(folio);
> >>  	bool is_anon = folio_test_anon(folio);
> >> +	struct mem_cgroup *memcg, *old_memcg;
> >>  	struct address_space *mapping = NULL;
> >>  	struct anon_vma *anon_vma = NULL;
> >>  	int old_order = folio_order(folio);
> >>  	struct folio *new_folio, *next;
> >>  	int nr_shmem_dropped = 0;
> >>  	enum ttu_flags ttu_flags = 0;
> >> -	int ret;
> >>  	pgoff_t end = 0;
> >> +	int ret;
> >>
> >>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> >>  	VM_WARN_ON_ONCE_FOLIO(!folio_test_large(folio), folio);
> >>
> >>  	if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
> >>  		ret = -EINVAL;
> >> -		goto out;
> >> +		goto out_no_memcg;
> >>  	}
> >>
> >>  	if (new_order >= old_order) {
> >>  		ret = -EINVAL;
> >> -		goto out;
> >> +		goto out_no_memcg;
> >>  	}
> >>
> >>  	ret = folio_check_splittable(folio, new_order, split_type);
> >>  	if (ret) {
> >>  		VM_WARN_ONCE(ret == -EINVAL, "Tried to split an unsplittable folio");
> >> -		goto out;
> >> +		goto out_no_memcg;
> >
> > This function really badly needs splitting up and probably some cleanup.h work :)
>
> You mean folio_check_splittable()? You want to move -EINVAL checks a
> separate one?

No __folio_split().

Comment about cleanup.h really was the whole pattern of goto xxx for various
levels of unwinding things.

But really I mean the folio splitting code in general, there's a lot of
massive-complicated-functions with a million things going on at once,
__folio_freeze_and_split_unmapped() is another.

Feels like we should really have this stuff in something like mm/folio.c anyway
too now that's renamed :)

>
> --
> Best Regards,
> Yan, Zi
>

--
Cheers, Lorenzo