[PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[David Hildenbrand (Arm)]

hwpoison code can end up calling min_order_for_split() without holding
the folio lock. There isn't really something that would prevent
concurrent folio split. Consequently folio->mapping can get set to
NULL after checking for "!folio->mapping", and if the compiler
reloads folio->mapping, mapping_min_folio_order() would try to
dereference NULL.

While very unlikely to happen in practice, let's just enforce that
min_order_for_split() is called with the folio lock held. We can
significantly cleanup the calling hwpoison code, and just get rid
of try_to_split_thp_page() to hold the folio lock for a bit longer.

Just work on folios now, which further cleans up the code. We just
have to be careful about doing the page_folio() after splitting, which
we have to do already either way. Do not change the way we split for
now, this needs more thought and should be done separately.

Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
would currently have leaked a folio reference.

In folio_split(), document and assert that we need the folio lock.
Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.

The folio->mapping problem was identified by Sashiko, and Li Youhong
reported it by sending a proposal fix.

This likely does not really warrant CCing stable, but I expect little
conflicts when doing the backport, so let's just CC stable because of
the refcount leak.

Reported-by: Li Youhong <liyouhong@kylinos.cn>
Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
Cc: stable@vger.kernel.org
Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
---
v4 of the last fixing attempts:

https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
---
 mm/huge_memory.c    |  4 ++++
 mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
 2 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 9b1f3b24f7e0d..a00df56a68b57 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -4362,10 +4362,14 @@ int folio_split(struct folio *folio, unsigned int new_order,
  * If a file-backed folio is truncated, 0 will be returned. Any subsequent
  * split attempt should get -EBUSY from split checking code.
  *
+ * Context: @folio must be locked.
+ *
  * Return: @folio's minimum order for split
  */
 unsigned int min_order_for_split(struct folio *folio)
 {
+	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
+
 	if (folio_test_anon(folio))
 		return 0;
 
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index aaf14608b30e2..f4f3f1fc9eaff 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1705,26 +1705,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
 	return page_action(ps, p, pfn);
 }
 
-/*
- * When 'release' is 'false', it means that if thp split has failed,
- * there is still more to do, hence the page refcount we took earlier
- * is still needed.
- */
-static int try_to_split_thp_page(struct page *page, unsigned int new_order,
-		bool release)
-{
-	int ret;
-
-	lock_page(page);
-	ret = split_huge_page_to_order(page, new_order);
-	unlock_page(page);
-
-	if (ret && release)
-		put_page(page);
-
-	return ret;
-}
-
 static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
 		struct address_space *mapping, pgoff_t index, int flags)
 {
@@ -2509,9 +2489,6 @@ int memory_failure(unsigned long pfn, int flags)
 	folio_unlock(folio);
 
 	if (folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
-		int err;
-
 		/*
 		 * The flag must be set after the refcount is bumped
 		 * otherwise it may race with THP split.
@@ -2526,24 +2503,25 @@ int memory_failure(unsigned long pfn, int flags)
 		 * page is a valid handlable page.
 		 */
 		folio_set_has_hwpoisoned(folio);
-		err = try_to_split_thp_page(p, new_order, /* release= */ false);
+
+		folio_lock(folio);
+		split_huge_page_to_order(p, min_order_for_split(folio));
+		folio = page_folio(p);
+		folio_unlock(folio);
+
 		/*
 		 * If splitting a folio to order-0 fails, kill the process.
 		 * Split the folio regardless to minimize unusable pages.
 		 * Because the memory failure code cannot handle large
 		 * folios, this split is always treated as if it failed.
 		 */
-		if (err || new_order) {
-			/* get folio again in case the original one is split */
-			folio = page_folio(p);
+		if (folio_test_large(folio)) {
 			res = -EHWPOISON;
 			kill_procs_now(p, pfn, flags, folio);
-			put_page(p);
+			folio_put(folio);
 			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
 			goto unlock_mutex;
 		}
-		VM_BUG_ON_PAGE(!page_count(p), p);
-		folio = page_folio(p);
 	}
 
 	/*
@@ -2861,9 +2839,8 @@ static int soft_offline_in_use_page(struct page *page)
 		.reason = MR_MEMORY_FAILURE,
 	};
 
+	folio_lock(folio);
 	if (!huge && folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
-
 		/*
 		 * If new_order (target split order) is not 0, do not split the
 		 * folio at all to retain the still accessible large folio.
@@ -2871,15 +2848,20 @@ static int soft_offline_in_use_page(struct page *page)
 		 * preferred, split it to non-zero new_order like it is done in
 		 * memory_failure().
 		 */
-		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
-						       /* release= */ true)) {
+		if (!min_order_for_split(folio))
+			ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
+		else
+			ret = -EBUSY;
+		folio = page_folio(page);
+
+		if (ret) {
+			folio_unlock(folio);
+			folio_put(folio);
 			pr_info("%#lx: thp split failed\n", pfn);
-			return -EBUSY;
+			return ret;
 		}
-		folio = page_folio(page);
 	}
 
-	folio_lock(folio);
 	if (!huge)
 		folio_wait_writeback(folio);
 	if (PageHWPoison(page)) {

---

base-commit: e5492213654050379e78ec6f9acfd6c9fe00f334

change-id: 20260806-try_to_split_thp_page-a359f4d8c90f

--

Cheers,

David

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Usama Arif]

On 06/08/2026 12:14, David Hildenbrand (Arm) wrote:
> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
> 
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
> 
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
> 
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.
> 
> In folio_split(), document and assert that we need the folio lock.
> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
> 
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
> 
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
> 
> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
> v4 of the last fixing attempts:
> 
> https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
> ---
>  mm/huge_memory.c    |  4 ++++
>  mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
>  2 files changed, 23 insertions(+), 37 deletions(-)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 9b1f3b24f7e0d..a00df56a68b57 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4362,10 +4362,14 @@ int folio_split(struct folio *folio, unsigned int new_order,
>   * If a file-backed folio is truncated, 0 will be returned. Any subsequent
>   * split attempt should get -EBUSY from split checking code.
>   *
> + * Context: @folio must be locked.
> + *
>   * Return: @folio's minimum order for split
>   */
>  unsigned int min_order_for_split(struct folio *folio)
>  {
> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> +

Makes sense here, folio->mapping can become NULL after the `if (!folio->mapping)`
check if folio_lock() is not held.


>  	if (folio_test_anon(folio))
>  		return 0;
>  
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index aaf14608b30e2..f4f3f1fc9eaff 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1705,26 +1705,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>  	return page_action(ps, p, pfn);
>  }
>  
> -/*
> - * When 'release' is 'false', it means that if thp split has failed,
> - * there is still more to do, hence the page refcount we took earlier
> - * is still needed.
> - */
> -static int try_to_split_thp_page(struct page *page, unsigned int new_order,
> -		bool release)
> -{
> -	int ret;
> -
> -	lock_page(page);
> -	ret = split_huge_page_to_order(page, new_order);
> -	unlock_page(page);
> -
> -	if (ret && release)
> -		put_page(page);
> -
> -	return ret;
> -}
> -
>  static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
>  		struct address_space *mapping, pgoff_t index, int flags)
>  {
> @@ -2509,9 +2489,6 @@ int memory_failure(unsigned long pfn, int flags)
>  	folio_unlock(folio);
>  
>  	if (folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> -		int err;
> -
>  		/*
>  		 * The flag must be set after the refcount is bumped
>  		 * otherwise it may race with THP split.
> @@ -2526,24 +2503,25 @@ int memory_failure(unsigned long pfn, int flags)
>  		 * page is a valid handlable page.
>  		 */
>  		folio_set_has_hwpoisoned(folio);
> -		err = try_to_split_thp_page(p, new_order, /* release= */ false);
> +
> +		folio_lock(folio);
> +		split_huge_page_to_order(p, min_order_for_split(folio));
> +		folio = page_folio(p);
> +		folio_unlock(folio);
> +
>  		/*
>  		 * If splitting a folio to order-0 fails, kill the process.
>  		 * Split the folio regardless to minimize unusable pages.
>  		 * Because the memory failure code cannot handle large
>  		 * folios, this split is always treated as if it failed.
>  		 */
> -		if (err || new_order) {
> -			/* get folio again in case the original one is split */
> -			folio = page_folio(p);
> +		if (folio_test_large(folio)) {
>  			res = -EHWPOISON;
>  			kill_procs_now(p, pfn, flags, folio);
> -			put_page(p);
> +			folio_put(folio);
>  			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
>  			goto unlock_mutex;
>  		}
> -		VM_BUG_ON_PAGE(!page_count(p), p);
> -		folio = page_folio(p);
>  	}
>

Looks cleaner and preserves existing policy where with successful split to order 0,
you continue handling the individual page, but onl split failure, folio remains large and
you report the failure.

  
>  	/*
> @@ -2861,9 +2839,8 @@ static int soft_offline_in_use_page(struct page *page)
>  		.reason = MR_MEMORY_FAILURE,
>  	};
>  
> +	folio_lock(folio);
>  	if (!huge && folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> -
>  		/*
>  		 * If new_order (target split order) is not 0, do not split the
>  		 * folio at all to retain the still accessible large folio.
> @@ -2871,15 +2848,20 @@ static int soft_offline_in_use_page(struct page *page)
>  		 * preferred, split it to non-zero new_order like it is done in
>  		 * memory_failure().
>  		 */
> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
> -						       /* release= */ true)) {
> +		if (!min_order_for_split(folio))
> +			ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
> +		else
> +			ret = -EBUSY;
> +		folio = page_folio(page);
> +
> +		if (ret) {
> +			folio_unlock(folio);
> +			folio_put(folio);
>  			pr_info("%#lx: thp split failed\n", pfn);
> -			return -EBUSY;
> +			return ret;
>  		}
> -		folio = page_folio(page);
>  	}
>  
> -	folio_lock(folio);
>  	if (!huge)
>  		folio_wait_writeback(folio);
>  	if (PageHWPoison(page)) {


LGTM

Acked-by: Usama Arif <usama.arif@linux.dev>

> 
> ---
> 
> base-commit: e5492213654050379e78ec6f9acfd6c9fe00f334
> 
> change-id: 20260806-try_to_split_thp_page-a359f4d8c90f
> 
> --
> 
> Cheers,
> 
> David
> 

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 01:14:38PM +0200, David Hildenbrand (Arm) wrote:
> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
>
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
>
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
>
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.

Yikes...

>
> In folio_split(), document and assert that we need the folio lock.

You mean min_order_for_split()?

> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
>
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
>
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
>
> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

A typo above and a nit below about a comment but otherwise LGTM so:

Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

> ---
> v4 of the last fixing attempts:
>
> https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
> ---
>  mm/huge_memory.c    |  4 ++++
>  mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
>  2 files changed, 23 insertions(+), 37 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 9b1f3b24f7e0d..a00df56a68b57 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4362,10 +4362,14 @@ int folio_split(struct folio *folio, unsigned int new_order,
>   * If a file-backed folio is truncated, 0 will be returned. Any subsequent
>   * split attempt should get -EBUSY from split checking code.
>   *
> + * Context: @folio must be locked.
> + *
>   * Return: @folio's minimum order for split
>   */
>  unsigned int min_order_for_split(struct folio *folio)
>  {
> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> +
>  	if (folio_test_anon(folio))
>  		return 0;
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index aaf14608b30e2..f4f3f1fc9eaff 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1705,26 +1705,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>  	return page_action(ps, p, pfn);
>  }
>
> -/*
> - * When 'release' is 'false', it means that if thp split has failed,
> - * there is still more to do, hence the page refcount we took earlier
> - * is still needed.
> - */
> -static int try_to_split_thp_page(struct page *page, unsigned int new_order,
> -		bool release)
> -{
> -	int ret;
> -
> -	lock_page(page);
> -	ret = split_huge_page_to_order(page, new_order);
> -	unlock_page(page);
> -
> -	if (ret && release)
> -		put_page(page);
> -
> -	return ret;
> -}
> -
>  static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
>  		struct address_space *mapping, pgoff_t index, int flags)
>  {
> @@ -2509,9 +2489,6 @@ int memory_failure(unsigned long pfn, int flags)
>  	folio_unlock(folio);
>
>  	if (folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> -		int err;
> -
>  		/*
>  		 * The flag must be set after the refcount is bumped
>  		 * otherwise it may race with THP split.
> @@ -2526,24 +2503,25 @@ int memory_failure(unsigned long pfn, int flags)
>  		 * page is a valid handlable page.
>  		 */
>  		folio_set_has_hwpoisoned(folio);
> -		err = try_to_split_thp_page(p, new_order, /* release= */ false);
> +
> +		folio_lock(folio);
> +		split_huge_page_to_order(p, min_order_for_split(folio));
> +		folio = page_folio(p);
> +		folio_unlock(folio);
> +
>  		/*
>  		 * If splitting a folio to order-0 fails, kill the process.
>  		 * Split the folio regardless to minimize unusable pages.
>  		 * Because the memory failure code cannot handle large
>  		 * folios, this split is always treated as if it failed.
>  		 */
> -		if (err || new_order) {
> -			/* get folio again in case the original one is split */
> -			folio = page_folio(p);
> +		if (folio_test_large(folio)) {
>  			res = -EHWPOISON;
>  			kill_procs_now(p, pfn, flags, folio);
> -			put_page(p);
> +			folio_put(folio);
>  			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
>  			goto unlock_mutex;
>  		}
> -		VM_BUG_ON_PAGE(!page_count(p), p);
> -		folio = page_folio(p);
>  	}
>
>  	/*
> @@ -2861,9 +2839,8 @@ static int soft_offline_in_use_page(struct page *page)
>  		.reason = MR_MEMORY_FAILURE,
>  	};
>
> +	folio_lock(folio);
>  	if (!huge && folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> -
>  		/*
>  		 * If new_order (target split order) is not 0, do not split the
>  		 * folio at all to retain the still accessible large folio.
> @@ -2871,15 +2848,20 @@ static int soft_offline_in_use_page(struct page *page)
>  		 * preferred, split it to non-zero new_order like it is done in
>  		 * memory_failure().
>  		 */

This comment is:

		/*
		 * If new_order (target split order) is not 0, do not split the
		 * folio at all to retain the still accessible large folio.
		 * NOTE: if minimizing the number of soft offline pages is
		 * preferred, split it to non-zero new_order like it is done in
		 * memory_failure().
		 */

Which references 'new_order' twice - probably need to update this.

> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
> -						       /* release= */ true)) {
> +		if (!min_order_for_split(folio))
> +			ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
> +		else
> +			ret = -EBUSY;
> +		folio = page_folio(page);
> +
> +		if (ret) {
> +			folio_unlock(folio);
> +			folio_put(folio);

Wow, incredible that we had a refcount leak here for ages and didn't know. Ugh
what a mess this hw posion crap is!

>  			pr_info("%#lx: thp split failed\n", pfn);
> -			return -EBUSY;
> +			return ret;
>  		}
> -		folio = page_folio(page);
>  	}
>
> -	folio_lock(folio);
>  	if (!huge)
>  		folio_wait_writeback(folio);
>  	if (PageHWPoison(page)) {
>
> ---
>
> base-commit: e5492213654050379e78ec6f9acfd6c9fe00f334
>
> change-id: 20260806-try_to_split_thp_page-a359f4d8c90f
>
> --
>
> Cheers,
>
> David
>

--
Cheers, Lorenzo

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[David Hildenbrand (Arm)]

On 8/6/26 16:03, Lorenzo Stoakes (ARM) wrote:
> On Thu, Aug 06, 2026 at 01:14:38PM +0200, David Hildenbrand (Arm) wrote:
>> hwpoison code can end up calling min_order_for_split() without holding
>> the folio lock. There isn't really something that would prevent
>> concurrent folio split. Consequently folio->mapping can get set to
>> NULL after checking for "!folio->mapping", and if the compiler
>> reloads folio->mapping, mapping_min_folio_order() would try to
>> dereference NULL.
>>
>> While very unlikely to happen in practice, let's just enforce that
>> min_order_for_split() is called with the folio lock held. We can
>> significantly cleanup the calling hwpoison code, and just get rid
>> of try_to_split_thp_page() to hold the folio lock for a bit longer.
>>
>> Just work on folios now, which further cleans up the code. We just
>> have to be careful about doing the page_folio() after splitting, which
>> we have to do already either way. Do not change the way we split for
>> now, this needs more thought and should be done separately.
>>
>> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
>> would currently have leaked a folio reference.
> 
> Yikes...
> 
>>
>> In folio_split(), document and assert that we need the folio lock.
> 
> You mean min_order_for_split()?

Very right :)

> 
>> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
>>
>> The folio->mapping problem was identified by Sashiko, and Li Youhong
>> reported it by sending a proposal fix.
>>
>> This likely does not really warrant CCing stable, but I expect little
>> conflicts when doing the backport, so let's just CC stable because of
>> the refcount leak.
>>
>> Reported-by: Li Youhong <liyouhong@kylinos.cn>
>> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
>> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> 
> A typo above and a nit below about a comment but otherwise LGTM so:
> 
> Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> 
>> ---
>> v4 of the last fixing attempts:
>>
>> https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
>> ---
>>  mm/huge_memory.c    |  4 ++++
>>  mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
>>  2 files changed, 23 insertions(+), 37 deletions(-)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 9b1f3b24f7e0d..a00df56a68b57 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -4362,10 +4362,14 @@ int folio_split(struct folio *folio, unsigned int new_order,
>>   * If a file-backed folio is truncated, 0 will be returned. Any subsequent
>>   * split attempt should get -EBUSY from split checking code.
>>   *
>> + * Context: @folio must be locked.
>> + *
>>   * Return: @folio's minimum order for split
>>   */
>>  unsigned int min_order_for_split(struct folio *folio)
>>  {
>> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
>> +
>>  	if (folio_test_anon(folio))
>>  		return 0;
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index aaf14608b30e2..f4f3f1fc9eaff 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -1705,26 +1705,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
>>  	return page_action(ps, p, pfn);
>>  }
>>
>> -/*
>> - * When 'release' is 'false', it means that if thp split has failed,
>> - * there is still more to do, hence the page refcount we took earlier
>> - * is still needed.
>> - */
>> -static int try_to_split_thp_page(struct page *page, unsigned int new_order,
>> -		bool release)
>> -{
>> -	int ret;
>> -
>> -	lock_page(page);
>> -	ret = split_huge_page_to_order(page, new_order);
>> -	unlock_page(page);
>> -
>> -	if (ret && release)
>> -		put_page(page);
>> -
>> -	return ret;
>> -}
>> -
>>  static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
>>  		struct address_space *mapping, pgoff_t index, int flags)
>>  {
>> @@ -2509,9 +2489,6 @@ int memory_failure(unsigned long pfn, int flags)
>>  	folio_unlock(folio);
>>
>>  	if (folio_test_large(folio)) {
>> -		const int new_order = min_order_for_split(folio);
>> -		int err;
>> -
>>  		/*
>>  		 * The flag must be set after the refcount is bumped
>>  		 * otherwise it may race with THP split.
>> @@ -2526,24 +2503,25 @@ int memory_failure(unsigned long pfn, int flags)
>>  		 * page is a valid handlable page.
>>  		 */
>>  		folio_set_has_hwpoisoned(folio);
>> -		err = try_to_split_thp_page(p, new_order, /* release= */ false);
>> +
>> +		folio_lock(folio);
>> +		split_huge_page_to_order(p, min_order_for_split(folio));
>> +		folio = page_folio(p);
>> +		folio_unlock(folio);
>> +
>>  		/*
>>  		 * If splitting a folio to order-0 fails, kill the process.
>>  		 * Split the folio regardless to minimize unusable pages.
>>  		 * Because the memory failure code cannot handle large
>>  		 * folios, this split is always treated as if it failed.
>>  		 */
>> -		if (err || new_order) {
>> -			/* get folio again in case the original one is split */
>> -			folio = page_folio(p);
>> +		if (folio_test_large(folio)) {
>>  			res = -EHWPOISON;
>>  			kill_procs_now(p, pfn, flags, folio);
>> -			put_page(p);
>> +			folio_put(folio);
>>  			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
>>  			goto unlock_mutex;
>>  		}
>> -		VM_BUG_ON_PAGE(!page_count(p), p);
>> -		folio = page_folio(p);
>>  	}
>>
>>  	/*
>> @@ -2861,9 +2839,8 @@ static int soft_offline_in_use_page(struct page *page)
>>  		.reason = MR_MEMORY_FAILURE,
>>  	};
>>
>> +	folio_lock(folio);
>>  	if (!huge && folio_test_large(folio)) {
>> -		const int new_order = min_order_for_split(folio);
>> -
>>  		/*
>>  		 * If new_order (target split order) is not 0, do not split the
>>  		 * folio at all to retain the still accessible large folio.
>> @@ -2871,15 +2848,20 @@ static int soft_offline_in_use_page(struct page *page)
>>  		 * preferred, split it to non-zero new_order like it is done in
>>  		 * memory_failure().
>>  		 */
> 
> This comment is:
> 
> 		/*
> 		 * If new_order (target split order) is not 0, do not split the
> 		 * folio at all to retain the still accessible large folio.
> 		 * NOTE: if minimizing the number of soft offline pages is
> 		 * preferred, split it to non-zero new_order like it is done in
> 		 * memory_failure().
> 		 */
> 
> Which references 'new_order' twice - probably need to update this.

Agreed, thanks!

While at it, I'll just drop the NOTE that is not of any value, really?


diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index f4f3f1fc9eaff..9f1e547e7b472 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2842,11 +2842,8 @@ static int soft_offline_in_use_page(struct page *page)
        folio_lock(folio);
        if (!huge && folio_test_large(folio)) {
                /*
-                * If new_order (target split order) is not 0, do not split the
-                * folio at all to retain the still accessible large folio.
-                * NOTE: if minimizing the number of soft offline pages is
-                * preferred, split it to non-zero new_order like it is done in
-                * memory_failure().
+                * If we cannot split to order-0, do not split the folio at all
+                * to retain the still accessible large folio.
                 */
                if (!min_order_for_split(folio))
                        ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;

> 
>> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
>> -						       /* release= */ true)) {
>> +		if (!min_order_for_split(folio))
>> +			ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
>> +		else
>> +			ret = -EBUSY;
>> +		folio = page_folio(page);
>> +
>> +		if (ret) {
>> +			folio_unlock(folio);
>> +			folio_put(folio);
> 
> Wow, incredible that we had a refcount leak here for ages and didn't know. Ugh
> what a mess this hw posion crap is!

I mean, soft-offline handling is not what runs on ... everyone's box. And the
memory will effectively be dead soon either way (hardware tells us that this
memory is likely going to get hwpoison soon) in most cases ... :)

-- 
Cheers,

David

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Lorenzo Stoakes (ARM)]

On Thu, Aug 06, 2026 at 04:17:21PM +0200, David Hildenbrand (Arm) wrote:
> On 8/6/26 16:03, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Aug 06, 2026 at 01:14:38PM +0200, David Hildenbrand (Arm) wrote:
> >> hwpoison code can end up calling min_order_for_split() without holding
> >> the folio lock. There isn't really something that would prevent
> >> concurrent folio split. Consequently folio->mapping can get set to
> >> NULL after checking for "!folio->mapping", and if the compiler
> >> reloads folio->mapping, mapping_min_folio_order() would try to
> >> dereference NULL.
> >>
> >> While very unlikely to happen in practice, let's just enforce that
> >> min_order_for_split() is called with the folio lock held. We can
> >> significantly cleanup the calling hwpoison code, and just get rid
> >> of try_to_split_thp_page() to hold the folio lock for a bit longer.
> >>
> >> Just work on folios now, which further cleans up the code. We just
> >> have to be careful about doing the page_folio() after splitting, which
> >> we have to do already either way. Do not change the way we split for
> >> now, this needs more thought and should be done separately.
> >>
> >> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> >> would currently have leaked a folio reference.
> >
> > Yikes...
> >
> >>
> >> In folio_split(), document and assert that we need the folio lock.
> >
> > You mean min_order_for_split()?
>
> Very right :)
>
> >
> >> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
> >>
> >> The folio->mapping problem was identified by Sashiko, and Li Youhong
> >> reported it by sending a proposal fix.
> >>
> >> This likely does not really warrant CCing stable, but I expect little
> >> conflicts when doing the backport, so let's just CC stable because of
> >> the refcount leak.
> >>
> >> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> >> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> >> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> >> Cc: stable@vger.kernel.org
> >> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> >
> > A typo above and a nit below about a comment but otherwise LGTM so:
> >
> > Reviewed-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> >
> >> ---
> >> v4 of the last fixing attempts:
> >>
> >> https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
> >> ---
> >>  mm/huge_memory.c    |  4 ++++
> >>  mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
> >>  2 files changed, 23 insertions(+), 37 deletions(-)
> >>
> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> >> index 9b1f3b24f7e0d..a00df56a68b57 100644
> >> --- a/mm/huge_memory.c
> >> +++ b/mm/huge_memory.c
> >> @@ -4362,10 +4362,14 @@ int folio_split(struct folio *folio, unsigned int new_order,
> >>   * If a file-backed folio is truncated, 0 will be returned. Any subsequent
> >>   * split attempt should get -EBUSY from split checking code.
> >>   *
> >> + * Context: @folio must be locked.
> >> + *
> >>   * Return: @folio's minimum order for split
> >>   */
> >>  unsigned int min_order_for_split(struct folio *folio)
> >>  {
> >> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> >> +
> >>  	if (folio_test_anon(folio))
> >>  		return 0;
> >>
> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> >> index aaf14608b30e2..f4f3f1fc9eaff 100644
> >> --- a/mm/memory-failure.c
> >> +++ b/mm/memory-failure.c
> >> @@ -1705,26 +1705,6 @@ static int identify_page_state(unsigned long pfn, struct page *p,
> >>  	return page_action(ps, p, pfn);
> >>  }
> >>
> >> -/*
> >> - * When 'release' is 'false', it means that if thp split has failed,
> >> - * there is still more to do, hence the page refcount we took earlier
> >> - * is still needed.
> >> - */
> >> -static int try_to_split_thp_page(struct page *page, unsigned int new_order,
> >> -		bool release)
> >> -{
> >> -	int ret;
> >> -
> >> -	lock_page(page);
> >> -	ret = split_huge_page_to_order(page, new_order);
> >> -	unlock_page(page);
> >> -
> >> -	if (ret && release)
> >> -		put_page(page);
> >> -
> >> -	return ret;
> >> -}
> >> -
> >>  static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
> >>  		struct address_space *mapping, pgoff_t index, int flags)
> >>  {
> >> @@ -2509,9 +2489,6 @@ int memory_failure(unsigned long pfn, int flags)
> >>  	folio_unlock(folio);
> >>
> >>  	if (folio_test_large(folio)) {
> >> -		const int new_order = min_order_for_split(folio);
> >> -		int err;
> >> -
> >>  		/*
> >>  		 * The flag must be set after the refcount is bumped
> >>  		 * otherwise it may race with THP split.
> >> @@ -2526,24 +2503,25 @@ int memory_failure(unsigned long pfn, int flags)
> >>  		 * page is a valid handlable page.
> >>  		 */
> >>  		folio_set_has_hwpoisoned(folio);
> >> -		err = try_to_split_thp_page(p, new_order, /* release= */ false);
> >> +
> >> +		folio_lock(folio);
> >> +		split_huge_page_to_order(p, min_order_for_split(folio));
> >> +		folio = page_folio(p);
> >> +		folio_unlock(folio);
> >> +
> >>  		/*
> >>  		 * If splitting a folio to order-0 fails, kill the process.
> >>  		 * Split the folio regardless to minimize unusable pages.
> >>  		 * Because the memory failure code cannot handle large
> >>  		 * folios, this split is always treated as if it failed.
> >>  		 */
> >> -		if (err || new_order) {
> >> -			/* get folio again in case the original one is split */
> >> -			folio = page_folio(p);
> >> +		if (folio_test_large(folio)) {
> >>  			res = -EHWPOISON;
> >>  			kill_procs_now(p, pfn, flags, folio);
> >> -			put_page(p);
> >> +			folio_put(folio);
> >>  			action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
> >>  			goto unlock_mutex;
> >>  		}
> >> -		VM_BUG_ON_PAGE(!page_count(p), p);
> >> -		folio = page_folio(p);
> >>  	}
> >>
> >>  	/*
> >> @@ -2861,9 +2839,8 @@ static int soft_offline_in_use_page(struct page *page)
> >>  		.reason = MR_MEMORY_FAILURE,
> >>  	};
> >>
> >> +	folio_lock(folio);
> >>  	if (!huge && folio_test_large(folio)) {
> >> -		const int new_order = min_order_for_split(folio);
> >> -
> >>  		/*
> >>  		 * If new_order (target split order) is not 0, do not split the
> >>  		 * folio at all to retain the still accessible large folio.
> >> @@ -2871,15 +2848,20 @@ static int soft_offline_in_use_page(struct page *page)
> >>  		 * preferred, split it to non-zero new_order like it is done in
> >>  		 * memory_failure().
> >>  		 */
> >
> > This comment is:
> >
> > 		/*
> > 		 * If new_order (target split order) is not 0, do not split the
> > 		 * folio at all to retain the still accessible large folio.
> > 		 * NOTE: if minimizing the number of soft offline pages is
> > 		 * preferred, split it to non-zero new_order like it is done in
> > 		 * memory_failure().
> > 		 */
> >
> > Which references 'new_order' twice - probably need to update this.
>
> Agreed, thanks!
>
> While at it, I'll just drop the NOTE that is not of any value, really?

Yeah rip that out.

>
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f4f3f1fc9eaff..9f1e547e7b472 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2842,11 +2842,8 @@ static int soft_offline_in_use_page(struct page *page)
>         folio_lock(folio);
>         if (!huge && folio_test_large(folio)) {
>                 /*
> -                * If new_order (target split order) is not 0, do not split the
> -                * folio at all to retain the still accessible large folio.
> -                * NOTE: if minimizing the number of soft offline pages is
> -                * preferred, split it to non-zero new_order like it is done in
> -                * memory_failure().
> +                * If we cannot split to order-0, do not split the folio at all
> +                * to retain the still accessible large folio.
>                  */
>                 if (!min_order_for_split(folio))
>                         ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
>
> >
> >> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
> >> -						       /* release= */ true)) {
> >> +		if (!min_order_for_split(folio))
> >> +			ret = split_huge_page_to_order(page, 0) ? -EBUSY : 0;
> >> +		else
> >> +			ret = -EBUSY;
> >> +		folio = page_folio(page);
> >> +
> >> +		if (ret) {
> >> +			folio_unlock(folio);
> >> +			folio_put(folio);
> >
> > Wow, incredible that we had a refcount leak here for ages and didn't know. Ugh
> > what a mess this hw posion crap is!
>
> I mean, soft-offline handling is not what runs on ... everyone's box. And the
> memory will effectively be dead soon either way (hardware tells us that this
> memory is likely going to get hwpoison soon) in most cases ... :)

Good. I think we should burn the whole thing down tbh.

>
> --
> Cheers,
>
> David

--
Cheers, Lorenzo

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Zi Yan]

On 6 Aug 2026, at 7:14, David Hildenbrand (Arm) wrote:

> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
>
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
>
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
>
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.
>
> In folio_split(), document and assert that we need the folio lock.
> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
>
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
>
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
>
> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---
> v4 of the last fixing attempts:
>
> https://lore.kernel.org/r/20260806031958.677935-1-dayou5941@163.com
> ---
>  mm/huge_memory.c    |  4 ++++
>  mm/memory-failure.c | 56 ++++++++++++++++++-----------------------------------
>  2 files changed, 23 insertions(+), 37 deletions(-)
>

Changes look good to me. Removing NOTE part from soft offline looks good
to me too. Thanks for cleaning it up.

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

Best Regards,
Yan, Zi

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Baolin Wang]

On 8/6/26 7:14 PM, David Hildenbrand (Arm) wrote:
> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
> 
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
> 
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
> 
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.
> 
> In folio_split(), document and assert that we need the folio lock.
> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
> 
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
> 
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
> 
> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>
> ---

LGTM. Thanks.
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>

Re: [PATCH] mm/memory-failure: fix folio refcount leak and min_order_for_split() locking

[Miaohe Lin]

On 2026/8/6 19:14, David Hildenbrand (Arm) wrote:
> hwpoison code can end up calling min_order_for_split() without holding
> the folio lock. There isn't really something that would prevent
> concurrent folio split. Consequently folio->mapping can get set to
> NULL after checking for "!folio->mapping", and if the compiler
> reloads folio->mapping, mapping_min_folio_order() would try to
> dereference NULL.
> 
> While very unlikely to happen in practice, let's just enforce that
> min_order_for_split() is called with the folio lock held. We can
> significantly cleanup the calling hwpoison code, and just get rid
> of try_to_split_thp_page() to hold the folio lock for a bit longer.
> 
> Just work on folios now, which further cleans up the code. We just
> have to be careful about doing the page_folio() after splitting, which
> we have to do already either way. Do not change the way we split for
> now, this needs more thought and should be done separately.
> 
> Cleaning this up we fix another issue: in soft_offline_in_use_page(), we
> would currently have leaked a folio reference.
> 
> In folio_split(), document and assert that we need the folio lock.
> Drop the questionable VM_BUG_ON_PAGE(!page_count(p), p) check entirely.
> 
> The folio->mapping problem was identified by Sashiko, and Li Youhong
> reported it by sending a proposal fix.
> 
> This likely does not really warrant CCing stable, but I expect little
> conflicts when doing the backport, so let's just CC stable because of
> the refcount leak.
> 
> Reported-by: Li Youhong <liyouhong@kylinos.cn>
> Closes: https://lore.kernel.org/r/20260804035828.2684059-1-dayou5941@163.com
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Hildenbrand (Arm) <david@kernel.org>

LGTM. Thanks David.

Acked-by: Miaohe Lin <linmiaohe@huawei.com>

Thanks.
.