1. Patchew
  2. linux
  3. [PATCH v2 0/4] Improve folio split related functions
  4. View patch

[PATCH v2 4/4] mm/huge_memory: fix folio split stats counting

Zi Yan posted 4 patches 1 week, 2 days ago
There is a newer version of this series
[PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by Zi Yan 1 week, 2 days ago 
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;".

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 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
Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by Lorenzo Stoakes 1 week ago 
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!

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?

>
> Signed-off-by: Zi Yan <ziy@nvidia.com>

LGTM, so:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>


> ---
>  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
Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by Zi Yan 1 week ago 
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@nvidia.com>
>
> LGTM, so:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.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
Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by David Hildenbrand (Red Hat) 1 week ago 
On 11/22/25 03:55, 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;".
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>   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) {
>   		/*

I guess this is not Fixes:/stable material. Wonder if such early (mostly 
-EINVAL etc) checks were at some point not intended to be counted.

In any case

Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

-- 
Cheers

David
Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by Zi Yan 1 week ago 
On 24 Nov 2025, at 5:45, David Hildenbrand (Red Hat) wrote:

> On 11/22/25 03:55, 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;".
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>   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) {
>>   		/*
>
> I guess this is not Fixes:/stable material. Wonder if such early (mostly -EINVAL etc) checks were at some point not intended to be counted.

I do not think it is worth Fixes/stable, since most checks should be caught
during development and not be triggered, except folio_test_writeback(folio)
one. And no one complained so far.

The inconsistency starts from commit 59807685a7e7 ("mm, THP, swap: support
splitting THP for THP swap out”), where if (PageWriteback(page)) return -EBUSY;
was added. Then commit 478d134e9506 ("mm/huge_memory: do not overkill when
splitting huge_zero_page") followed and so on.

This patch is intended to make code consistent.

>
> In any case
>
> Acked-by: David Hildenbrand (Red Hat) <david@kernel.org>

Thanks.

Best Regards,
Yan, Zi
Re: [PATCH v2 4/4] mm/huge_memory: fix folio split stats counting
Posted by Wei Yang 1 week, 1 day ago 
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;".
>
>Signed-off-by: Zi Yan <ziy@nvidia.com>

Sounds reasonable, Thanks.

Reviewed-by: Wei Yang <richard.weiyang@gmail.com>

-- 
Wei Yang
Help you, Help me

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.