1. Patchew
  2. linux
  3. View series

[PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled

Pankaj Raghav (Samsung) posted 1 patch 4 weeks ago
There is a newer version of this series
include/linux/huge_mm.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
[PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Pankaj Raghav (Samsung) 4 weeks ago 
From: Pankaj Raghav <p.raghav@samsung.com>

split_huge_page_to_list_[to_order](), split_huge_page() and
try_folio_split() return 0 on success and error codes on failure.

When THP is disabled, these functions return 0 indicating success even
though an error code should be returned as it is not possible to split a
folio when THP is disabled.

Make all these functions return -EINVAL to indicate failure instead of
0. As large folios depend on CONFIG_THP, issue warning as this function
should not be called without a large folio.

Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
---
This issue was discovered while experimenting enabling large folios
without THP and found that returning 0 in these functions is resulting in
undefined behavior in truncate operations. This change fixes the issue.

 include/linux/huge_mm.h | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 29ef70022da1..23f124493c47 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -588,22 +588,30 @@ static inline int
 split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
 		unsigned int new_order)
 {
-	return 0;
+	struct folio *folio = page_folio(page);
+
+	VM_WARN_ON_ONCE_FOLIO(1, folio);
+	return -EINVAL;
 }
 static inline int split_huge_page(struct page *page)
 {
-	return 0;
+	struct folio *folio = page_folio(page);
+
+	VM_WARN_ON_ONCE_FOLIO(1, folio);
+	return -EINVAL;
 }
 
 static inline int split_folio_to_list(struct folio *folio, struct list_head *list)
 {
-	return 0;
+	VM_WARN_ON_ONCE_FOLIO(1, folio);
+	return -EINVAL;
 }
 
 static inline int try_folio_split(struct folio *folio, struct page *page,
 		struct list_head *list)
 {
-	return 0;
+	VM_WARN_ON_ONCE_FOLIO(1, folio);
+	return -EINVAL;
 }
 
 static inline void deferred_split_folio(struct folio *folio, bool partially_mapped) {}

base-commit: 291634ccfd2820c09f6e8c4982c2dee8155d09ae
-- 
2.50.1
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Lorenzo Stoakes 3 weeks, 6 days ago 
On Thu, Sep 04, 2025 at 11:51:29AM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
>
> split_huge_page_to_list_[to_order](), split_huge_page() and
> try_folio_split() return 0 on success and error codes on failure.
>
> When THP is disabled, these functions return 0 indicating success even
> though an error code should be returned as it is not possible to split a
> folio when THP is disabled.
>
> Make all these functions return -EINVAL to indicate failure instead of
> 0. As large folios depend on CONFIG_THP, issue warning as this function
> should not be called without a large folio.
>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Other than the trivially-fixable issue mentioned below this LGTM so:

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

On basis of those being fixed.

> ---
> This issue was discovered while experimenting enabling large folios
> without THP and found that returning 0 in these functions is resulting in
> undefined behavior in truncate operations. This change fixes the issue.
>
>  include/linux/huge_mm.h | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 29ef70022da1..23f124493c47 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -588,22 +588,30 @@ static inline int
>  split_huge_page_to_list_to_order(struct page *page, struct list_head *list,
>  		unsigned int new_order)
>  {
> -	return 0;
> +	struct folio *folio = page_folio(page);
> +
> +	VM_WARN_ON_ONCE_FOLIO(1, folio);

As per the bot report, you should just do page_folio(page) here or otherwise
when compiled out you'll get an unused var warning.

> +	return -EINVAL;
>  }
>  static inline int split_huge_page(struct page *page)
>  {
> -	return 0;
> +	struct folio *folio = page_folio(page);

Same as above!

> +
> +	VM_WARN_ON_ONCE_FOLIO(1, folio);
> +	return -EINVAL;
>  }
>
>  static inline int split_folio_to_list(struct folio *folio, struct list_head *list)
>  {
> -	return 0;
> +	VM_WARN_ON_ONCE_FOLIO(1, folio);
> +	return -EINVAL;
>  }
>
>  static inline int try_folio_split(struct folio *folio, struct page *page,
>  		struct list_head *list)
>  {
> -	return 0;
> +	VM_WARN_ON_ONCE_FOLIO(1, folio);
> +	return -EINVAL;
>  }
>
>  static inline void deferred_split_folio(struct folio *folio, bool partially_mapped) {}
>
> base-commit: 291634ccfd2820c09f6e8c4982c2dee8155d09ae
> --
> 2.50.1
>
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Kiryl Shutsemau 4 weeks ago 
On Thu, Sep 04, 2025 at 11:51:29AM +0200, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> split_huge_page_to_list_[to_order](), split_huge_page() and
> try_folio_split() return 0 on success and error codes on failure.
> 
> When THP is disabled, these functions return 0 indicating success even
> though an error code should be returned as it is not possible to split a
> folio when THP is disabled.
> 
> Make all these functions return -EINVAL to indicate failure instead of
> 0. As large folios depend on CONFIG_THP, issue warning as this function
> should not be called without a large folio.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>

Acked-by: Kiryl Shutsemau <kas@kernel.org>

Just curious, did you give BUILD_BUG() a try?

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Pankaj Raghav 3 weeks, 6 days ago 
On 9/4/25 17:26, Kiryl Shutsemau wrote:
> On Thu, Sep 04, 2025 at 11:51:29AM +0200, Pankaj Raghav (Samsung) wrote:
>> From: Pankaj Raghav <p.raghav@samsung.com>
>>
>> split_huge_page_to_list_[to_order](), split_huge_page() and
>> try_folio_split() return 0 on success and error codes on failure.
>>
>> When THP is disabled, these functions return 0 indicating success even
>> though an error code should be returned as it is not possible to split a
>> folio when THP is disabled.
>>
>> Make all these functions return -EINVAL to indicate failure instead of
>> 0. As large folios depend on CONFIG_THP, issue warning as this function
>> should not be called without a large folio.
>>
>> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> 
> Acked-by: Kiryl Shutsemau <kas@kernel.org>
> 
> Just curious, did you give BUILD_BUG() a try?
> 

Yes, I tried BUILD_BUG() but it actually fails the build because truncate_inode_partial_folio()
calls try_folio_split().

It won't be called in runtime because there is a check for large folios before we call this
function. So a runtime warning is better in this case.

--
Pankaj
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by David Hildenbrand 3 weeks, 6 days ago 
On 05.09.25 10:13, Pankaj Raghav wrote:
> On 9/4/25 17:26, Kiryl Shutsemau wrote:
>> On Thu, Sep 04, 2025 at 11:51:29AM +0200, Pankaj Raghav (Samsung) wrote:
>>> From: Pankaj Raghav <p.raghav@samsung.com>
>>>
>>> split_huge_page_to_list_[to_order](), split_huge_page() and
>>> try_folio_split() return 0 on success and error codes on failure.
>>>
>>> When THP is disabled, these functions return 0 indicating success even
>>> though an error code should be returned as it is not possible to split a
>>> folio when THP is disabled.
>>>
>>> Make all these functions return -EINVAL to indicate failure instead of
>>> 0. As large folios depend on CONFIG_THP, issue warning as this function
>>> should not be called without a large folio.
>>>
>>> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
>>
>> Acked-by: Kiryl Shutsemau <kas@kernel.org>
>>
>> Just curious, did you give BUILD_BUG() a try?
>>
> 
> Yes, I tried BUILD_BUG() but it actually fails the build because truncate_inode_partial_folio()
> calls try_folio_split().
> 
> It won't be called in runtime because there is a check for large folios before we call this
> function. So a runtime warning is better in this case.

Right, folio_test_large() is not compiled out.

-- 
Cheers

David / dhildenb
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Kiryl Shutsemau 3 weeks, 6 days ago 
On Fri, Sep 05, 2025 at 10:13:05AM +0200, Pankaj Raghav wrote:
> On 9/4/25 17:26, Kiryl Shutsemau wrote:
> > On Thu, Sep 04, 2025 at 11:51:29AM +0200, Pankaj Raghav (Samsung) wrote:
> >> From: Pankaj Raghav <p.raghav@samsung.com>
> >>
> >> split_huge_page_to_list_[to_order](), split_huge_page() and
> >> try_folio_split() return 0 on success and error codes on failure.
> >>
> >> When THP is disabled, these functions return 0 indicating success even
> >> though an error code should be returned as it is not possible to split a
> >> folio when THP is disabled.
> >>
> >> Make all these functions return -EINVAL to indicate failure instead of
> >> 0. As large folios depend on CONFIG_THP, issue warning as this function
> >> should not be called without a large folio.
> >>
> >> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> > 
> > Acked-by: Kiryl Shutsemau <kas@kernel.org>
> > 
> > Just curious, did you give BUILD_BUG() a try?
> > 
> 
> Yes, I tried BUILD_BUG() but it actually fails the build because truncate_inode_partial_folio()
> calls try_folio_split().
> 
> It won't be called in runtime because there is a check for large folios before we call this
> function. So a runtime warning is better in this case.

If truncate_inode_partial_folio() shouldn't ever be calling on THP=n
kernel, it also can be BUILD_BUG(). Just saying :P

-- 
  Kiryl Shutsemau / Kirill A. Shutemov
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by Zi Yan 4 weeks ago 
On 4 Sep 2025, at 5:51, Pankaj Raghav (Samsung) wrote:

> From: Pankaj Raghav <p.raghav@samsung.com>
>
> split_huge_page_to_list_[to_order](), split_huge_page() and
> try_folio_split() return 0 on success and error codes on failure.
>
> When THP is disabled, these functions return 0 indicating success even
> though an error code should be returned as it is not possible to split a
> folio when THP is disabled.
>
> Make all these functions return -EINVAL to indicate failure instead of
> 0. As large folios depend on CONFIG_THP, issue warning as this function
> should not be called without a large folio.
>
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---
> This issue was discovered while experimenting enabling large folios
> without THP and found that returning 0 in these functions is resulting in
> undefined behavior in truncate operations. This change fixes the issue.
>
>  include/linux/huge_mm.h | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
Acked-by: Zi Yan <ziy@nvidia.com>

Best Regards,
Yan, Zi
Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
Posted by David Hildenbrand 4 weeks ago 
On 04.09.25 11:51, Pankaj Raghav (Samsung) wrote:
> From: Pankaj Raghav <p.raghav@samsung.com>
> 
> split_huge_page_to_list_[to_order](), split_huge_page() and
> try_folio_split() return 0 on success and error codes on failure.
> 
> When THP is disabled, these functions return 0 indicating success even
> though an error code should be returned as it is not possible to split a
> folio when THP is disabled.
> 
> Make all these functions return -EINVAL to indicate failure instead of
> 0. As large folios depend on CONFIG_THP, issue warning as this function
> should not be called without a large folio.
> 
> Signed-off-by: Pankaj Raghav <p.raghav@samsung.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.