linux-kernel - Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <C2036549-30B7-4F4D-AA82-51E75138F1A9@nvidia.com>
Date: Mon, 24 Nov 2025 12:29:50 -0500
From: Zi Yan <ziy@...dia.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
Cc: David Hildenbrand <david@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>, Nico Pache <npache@...hat.com>,
 Ryan Roberts <ryan.roberts@....com>, Dev Jain <dev.jain@....com>,
 Barry Song <baohua@...nel.org>, Lance Yang <lance.yang@...ux.dev>,
 Miaohe Lin <linmiaohe@...wei.com>, Naoya Horiguchi <nao.horiguchi@...il.com>,
 Wei Yang <richard.weiyang@...il.com>, Balbir Singh <balbirs@...dia.com>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting

On 24 Nov 2025, at 10:21, Lorenzo Stoakes wrote:

> On Fri, Nov 21, 2025 at 09:55:29PM -0500, Zi Yan wrote:
>> The "return <error code>" statements for error checks at the beginning of
>> __folio_split() skip necessary count_vm_event() and count_mthp_stat() at
>> the end of the function. Fix these by replacing them with
>> "ret = <error code>; goto out;".
>
> I guess the xas_destroy() there will be a no-op in these cases!

Right. And there is no memory leak, since xas_split_alloc() does the
memory allocation and code after it all never returns directly, letting
xas_destroy() do its job.

>
> Good spot, as David said, maybe one for stable then... not sure if necessary for
> statistical stuff though?
>
> But at the same time, maybe users will be misled if these are incorrect?
>
> Has this bug been around since the beginning? Be curious to know if that's the
> case or if it was introduced somewhere along the line?

It started from commit 59807685a7e7 ("mm, THP, swap: support
splitting THP for THP swap out”) back in 2017 and more inconsistent code
was added later.

Unless someone relies on split stats heavily, I am not sure we need to backport
it.

>
>>
>> Signed-off-by: Zi Yan <ziy@...dia.com>
>
> LGTM, so:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>

Thanks.

>
>> ---
>>  mm/huge_memory.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index ebc3ba0907fd..a42c4f29ce4f 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -3954,16 +3954,20 @@ static int __folio_split(struct folio *folio, unsigned int new_order,
>>  	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))
>> -		return -EINVAL;
>> +	if (folio != page_folio(split_at) || folio != page_folio(lock_at)) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>>
>> -	if (new_order >= old_order)
>> -		return -EINVAL;
>> +	if (new_order >= old_order) {
>> +		ret = -EINVAL;
>> +		goto out;
>> +	}
>>
>>  	ret = folio_check_splittable(folio, new_order, split_type,
>>  				     /* warn = */ true);
>>  	if (ret)
>> -		return ret;
>> +		goto out;
>>
>>  	if (is_anon) {
>>  		/*
>> --
>> 2.51.0
>>
>
> Cheers, Lorenzo

Best Regards,
Yan, Zi