linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
@ 2025-09-04  9:51 Pankaj Raghav (Samsung)
  2025-09-04 10:11 ` David Hildenbrand
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Pankaj Raghav (Samsung) @ 2025-09-04  9:51 UTC (permalink / raw)
  To: Zi Yan, Ryan Roberts, David Hildenbrand, Lorenzo Stoakes,
	Baolin Wang, Barry Song, Andrew Morton, Nico Pache, Dev Jain,
	Liam R . Howlett
  Cc: linux-kernel, kernel, willy, linux-mm, mcgrof, gost.dev, Pankaj Raghav

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-04  9:51 [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled Pankaj Raghav (Samsung)
@ 2025-09-04 10:11 ` David Hildenbrand
  2025-09-04 13:21 ` Zi Yan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2025-09-04 10:11 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung),
	Zi Yan, Ryan Roberts, Lorenzo Stoakes, Baolin Wang, Barry Song,
	Andrew Morton, Nico Pache, Dev Jain, Liam R . Howlett
  Cc: linux-kernel, willy, linux-mm, mcgrof, gost.dev, Pankaj Raghav

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-04  9:51 [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled Pankaj Raghav (Samsung)
  2025-09-04 10:11 ` David Hildenbrand
@ 2025-09-04 13:21 ` Zi Yan
  2025-09-04 15:26 ` Kiryl Shutsemau
  2025-09-05 11:57 ` Lorenzo Stoakes
  3 siblings, 0 replies; 8+ messages in thread
From: Zi Yan @ 2025-09-04 13:21 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: Ryan Roberts, David Hildenbrand, Lorenzo Stoakes, Baolin Wang,
	Barry Song, Andrew Morton, Nico Pache, Dev Jain,
	Liam R . Howlett, linux-kernel, willy, linux-mm, mcgrof,
	gost.dev, Pankaj Raghav

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-04  9:51 [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled Pankaj Raghav (Samsung)
  2025-09-04 10:11 ` David Hildenbrand
  2025-09-04 13:21 ` Zi Yan
@ 2025-09-04 15:26 ` Kiryl Shutsemau
  2025-09-05  8:13   ` Pankaj Raghav
  2025-09-05 11:57 ` Lorenzo Stoakes
  3 siblings, 1 reply; 8+ messages in thread
From: Kiryl Shutsemau @ 2025-09-04 15:26 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: Zi Yan, Ryan Roberts, David Hildenbrand, Lorenzo Stoakes,
	Baolin Wang, Barry Song, Andrew Morton, Nico Pache, Dev Jain,
	Liam R . Howlett, linux-kernel, willy, linux-mm, mcgrof,
	gost.dev, Pankaj Raghav

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-04 15:26 ` Kiryl Shutsemau
@ 2025-09-05  8:13   ` Pankaj Raghav
  2025-09-05  9:12     ` David Hildenbrand
  2025-09-05  9:13     ` Kiryl Shutsemau
  0 siblings, 2 replies; 8+ messages in thread
From: Pankaj Raghav @ 2025-09-05  8:13 UTC (permalink / raw)
  To: Kiryl Shutsemau
  Cc: Zi Yan, Ryan Roberts, David Hildenbrand, Lorenzo Stoakes,
	Baolin Wang, Barry Song, Andrew Morton, Nico Pache, Dev Jain,
	Liam R . Howlett, linux-kernel, willy, linux-mm, mcgrof,
	gost.dev, Pankaj Raghav

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-05  8:13   ` Pankaj Raghav
@ 2025-09-05  9:12     ` David Hildenbrand
  2025-09-05  9:13     ` Kiryl Shutsemau
  1 sibling, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2025-09-05  9:12 UTC (permalink / raw)
  To: Pankaj Raghav, Kiryl Shutsemau
  Cc: Zi Yan, Ryan Roberts, Lorenzo Stoakes, Baolin Wang, Barry Song,
	Andrew Morton, Nico Pache, Dev Jain, Liam R . Howlett,
	linux-kernel, willy, linux-mm, mcgrof, gost.dev, Pankaj Raghav

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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-05  8:13   ` Pankaj Raghav
  2025-09-05  9:12     ` David Hildenbrand
@ 2025-09-05  9:13     ` Kiryl Shutsemau
  1 sibling, 0 replies; 8+ messages in thread
From: Kiryl Shutsemau @ 2025-09-05  9:13 UTC (permalink / raw)
  To: Pankaj Raghav
  Cc: Zi Yan, Ryan Roberts, David Hildenbrand, Lorenzo Stoakes,
	Baolin Wang, Barry Song, Andrew Morton, Nico Pache, Dev Jain,
	Liam R . Howlett, linux-kernel, willy, linux-mm, mcgrof,
	gost.dev, Pankaj Raghav

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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled
  2025-09-04  9:51 [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled Pankaj Raghav (Samsung)
                   ` (2 preceding siblings ...)
  2025-09-04 15:26 ` Kiryl Shutsemau
@ 2025-09-05 11:57 ` Lorenzo Stoakes
  3 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Stoakes @ 2025-09-05 11:57 UTC (permalink / raw)
  To: Pankaj Raghav (Samsung)
  Cc: Zi Yan, Ryan Roberts, David Hildenbrand, Baolin Wang, Barry Song,
	Andrew Morton, Nico Pache, Dev Jain, Liam R . Howlett,
	linux-kernel, willy, linux-mm, mcgrof, gost.dev, Pankaj Raghav

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
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-09-05 11:57 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-04  9:51 [PATCH v2] huge_memory: return -EINVAL in folio split functions when THP is disabled Pankaj Raghav (Samsung)
2025-09-04 10:11 ` David Hildenbrand
2025-09-04 13:21 ` Zi Yan
2025-09-04 15:26 ` Kiryl Shutsemau
2025-09-05  8:13   ` Pankaj Raghav
2025-09-05  9:12     ` David Hildenbrand
2025-09-05  9:13     ` Kiryl Shutsemau
2025-09-05 11:57 ` Lorenzo Stoakes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox