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

[Zi Yan]

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 <hannes@xxxxxxxxxxx>
>> 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 <ziy@xxxxxxxxxx>
>
> Change seems reasonable overall.
>
> Acked-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>
>> ---
>>  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?

-- 
Best Regards,
Yan, Zi