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

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 t=
o 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, un=
signed int new_order,
>>  	XA_STATE(xas, &folio->mapping->i_pages, folio->index);
>>  	struct folio *end_folio =3D folio_next(folio);
>>  	bool is_anon =3D folio_test_anon(folio);
>> +	struct mem_cgroup *memcg, *old_memcg;
>>  	struct address_space *mapping =3D NULL;
>>  	struct anon_vma *anon_vma =3D NULL;
>>  	int old_order =3D folio_order(folio);
>>  	struct folio *new_folio, *next;
>>  	int nr_shmem_dropped =3D 0;
>>  	enum ttu_flags ttu_flags =3D 0;
>> -	int ret;
>>  	pgoff_t end =3D 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 !=3D page_folio(split_at) || folio !=3D page_folio(lock_at))=
 {
>>  		ret =3D -EINVAL;
>> -		goto out;
>> +		goto out_no_memcg;
>>  	}
>>
>>  	if (new_order >=3D old_order) {
>>  		ret =3D -EINVAL;
>> -		goto out;
>> +		goto out_no_memcg;
>>  	}
>>
>>  	ret =3D folio_check_splittable(folio, new_order, split_type);
>>  	if (ret) {
>>  		VM_WARN_ONCE(ret =3D=3D -EINVAL, "Tried to split an unsplittable foli=
o");
>> -		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?

--=20
Best Regards,
Yan, Zi