Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mm/memory-failure: fix refcount leak on soft-offline LBS folio
@ 2026-08-04  3:53 dayou5941
  2026-08-05 23:33 ` jane.chu
  0 siblings, 1 reply; 3+ messages in thread
From: dayou5941 @ 2026-08-04  3:53 UTC (permalink / raw)
  To: linmiaohe, akpm; +Cc: nao.horiguchi, linux-mm, liyouhong

From: liyouhong <liyouhong@kylinos.cn>

soft_offline_in_use_page() runs with a folio reference taken by
get_hwpoison_page().

Soft-offline skips splitting when min_order_for_split()
is non-zero, so large folios stay intact. That early
return -EBUSY path never drops the reference.

The other failure path is fine: try_to_split_thp_page(..., release=true)
puts the page when the split itself fails. memory_failure() also puts
explicitly on its analogous unsplit path.

Handle the new_order != 0 case separately and call folio_put()
before returning.

Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
Signed-off-by: liyouhong <liyouhong@kylinos.cn>
---
v2:
- Add documentation for the folio reference counting convention of soft_offline_in_use_page().
- Remove MF_COUNT_INCREASED related description in commit message to avoid misleading.
v1: https://lore.kernel.org/all/20260803060001.800638-1-dayou5941@163.com/

---
 mm/memory-failure.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 51508a55c405..3b1e6946821b 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2776,6 +2776,9 @@ EXPORT_SYMBOL(unpoison_memory);
  * soft_offline_in_use_page handles hugetlb-pages and non-hugetlb pages.
  * If the page is a non-dirty unmapped page-cache page, it simply invalidates.
  * If the page is mapped, it migrates the contents over.
+ *
+ * The folio refcount has been incremented before entering this function.
+ * This folio reference must be released before the function returns on all paths.
  */
 static int soft_offline_in_use_page(struct page *page)
 {
@@ -2801,9 +2804,19 @@ static int soft_offline_in_use_page(struct page *page)
 		 * 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().
+		 *
+		 * Drop the reference obtained upon entry;
+		 * try_to_split_thp_page(..., release=true) handles refcounting itself
+		 * when the split fails.
 		 */
-		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
-						       /* release= */ true)) {
+		if (new_order) {
+			pr_info("%#lx: order-%d folio cannot soft offline\n",
+				pfn, new_order);
+			folio_put(folio);
+			return -EBUSY;
+		}
+		if (try_to_split_thp_page(page, /* new_order= */ 0,
+					 /* release= */ true)) {
 			pr_info("%#lx: thp split failed\n", pfn);
 			return -EBUSY;
 		}
-- 
2.25.1



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

* Re: [PATCH v2] mm/memory-failure: fix refcount leak on soft-offline LBS folio
  2026-08-04  3:53 [PATCH v2] mm/memory-failure: fix refcount leak on soft-offline LBS folio dayou5941
@ 2026-08-05 23:33 ` jane.chu
  2026-08-06  3:24   ` 李佑鸿 
  0 siblings, 1 reply; 3+ messages in thread
From: jane.chu @ 2026-08-05 23:33 UTC (permalink / raw)
  To: dayou5941, linmiaohe, akpm; +Cc: nao.horiguchi, linux-mm, liyouhong



On 8/3/2026 8:53 PM, dayou5941@163.com wrote:
> From: liyouhong <liyouhong@ kylinos. cn> soft_offline_in_use_page() runs 
> with a folio reference taken by get_hwpoison_page(). Soft-offline skips 
> splitting when min_order_for_split() is non-zero, so large folios stay 
> intact. That early return
> 
> 
> From: liyouhong <liyouhong@kylinos.cn>
> 
> soft_offline_in_use_page() runs with a folio reference taken by
> get_hwpoison_page().
> 
> Soft-offline skips splitting when min_order_for_split()
> is non-zero, so large folios stay intact. That early
> return -EBUSY path never drops the reference.
> 
> The other failure path is fine: try_to_split_thp_page(..., release=true)
> puts the page when the split itself fails. memory_failure() also puts
> explicitly on its analogous unsplit path.
> 
> Handle the new_order != 0 case separately and call folio_put()
> before returning.
> 
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
> ---
> v2:
> - Add documentation for the folio reference counting convention of soft_offline_in_use_page().
> - Remove MF_COUNT_INCREASED related description in commit message to avoid misleading.
> v1: https://lore.kernel.org/all/20260803060001.800638-1-dayou5941@163.com/
> 
> ---
>   mm/memory-failure.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 51508a55c405..3b1e6946821b 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2776,6 +2776,9 @@ EXPORT_SYMBOL(unpoison_memory);
>    * soft_offline_in_use_page handles hugetlb-pages and non-hugetlb pages.
>    * If the page is a non-dirty unmapped page-cache page, it simply invalidates.
>    * If the page is mapped, it migrates the contents over.
> + *
> + * The folio refcount has been incremented before entering this function.
> + * This folio reference must be released before the function returns on all paths.
>    */
>   static int soft_offline_in_use_page(struct page *page)
>   {
> @@ -2801,9 +2804,19 @@ static int soft_offline_in_use_page(struct page *page)
>   		 * 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().
> +		 *
> +		 * Drop the reference obtained upon entry;
> +		 * try_to_split_thp_page(..., release=true) handles refcounting itself
> +		 * when the split fails.
>   		 */
> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
> -						       /* release= */ true)) {
> +		if (new_order) {
> +			pr_info("%#lx: order-%d folio cannot soft offline\n",
> +				pfn, new_order);
> +			folio_put(folio);
> +			return -EBUSY;
> +		}
> +		if (try_to_split_thp_page(page, /* new_order= */ 0,
> +					 /* release= */ true)) {
>   			pr_info("%#lx: thp split failed\n", pfn);
>   			return -EBUSY;
>   		}
> -- 
> 2.25.1
> 

It seems me this will do -

--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2802,8 +2802,8 @@ 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 (new_order || try_to_split_thp_page(page, false)) {
+                       put_page(page);
                         pr_info("%#lx: thp split failed\n", pfn);
                         return -EBUSY;
                 }


thanks,
-jane

> 



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

* Re:Re: [PATCH v2] mm/memory-failure: fix refcount leak on soft-offline LBS folio
  2026-08-05 23:33 ` jane.chu
@ 2026-08-06  3:24   ` 李佑鸿 
  0 siblings, 0 replies; 3+ messages in thread
From: 李佑鸿  @ 2026-08-06  3:24 UTC (permalink / raw)
  To: jane.chu; +Cc: linmiaohe, akpm, nao.horiguchi, linux-mm, liyouhong


















At 2026-08-06 07:33:08, jane.chu@oracle.com wrote:
>
>
>On 8/3/2026 8:53 PM, dayou5941@163.com wrote:
>> From: liyouhong <liyouhong@ kylinos. cn> soft_offline_in_use_page() runs 
>> with a folio reference taken by get_hwpoison_page(). Soft-offline skips 
>> splitting when min_order_for_split() is non-zero, so large folios stay 
>> intact. That early return
>> 
>> 
>> From: liyouhong <liyouhong@kylinos.cn>
>> 
>> soft_offline_in_use_page() runs with a folio reference taken by
>> get_hwpoison_page().
>> 
>> Soft-offline skips splitting when min_order_for_split()
>> is non-zero, so large folios stay intact. That early
>> return -EBUSY path never drops the reference.
>> 
>> The other failure path is fine: try_to_split_thp_page(..., release=true)
>> puts the page when the split itself fails. memory_failure() also puts
>> explicitly on its analogous unsplit path.
>> 
>> Handle the new_order != 0 case separately and call folio_put()
>> before returning.
>> 
>> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
>> Signed-off-by: liyouhong <liyouhong@kylinos.cn>
>> ---
>> v2:
>> - Add documentation for the folio reference counting convention of soft_offline_in_use_page().
>> - Remove MF_COUNT_INCREASED related description in commit message to avoid misleading.
>> v1: https://lore.kernel.org/all/20260803060001.800638-1-dayou5941@163.com/
>> 
>> ---
>>   mm/memory-failure.c | 17 +++++++++++++++--
>>   1 file changed, 15 insertions(+), 2 deletions(-)
>> 
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index 51508a55c405..3b1e6946821b 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -2776,6 +2776,9 @@ EXPORT_SYMBOL(unpoison_memory);
>>    * soft_offline_in_use_page handles hugetlb-pages and non-hugetlb pages.
>>    * If the page is a non-dirty unmapped page-cache page, it simply invalidates.
>>    * If the page is mapped, it migrates the contents over.
>> + *
>> + * The folio refcount has been incremented before entering this function.
>> + * This folio reference must be released before the function returns on all paths.
>>    */
>>   static int soft_offline_in_use_page(struct page *page)
>>   {
>> @@ -2801,9 +2804,19 @@ static int soft_offline_in_use_page(struct page *page)
>>   		 * 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().
>> +		 *
>> +		 * Drop the reference obtained upon entry;
>> +		 * try_to_split_thp_page(..., release=true) handles refcounting itself
>> +		 * when the split fails.
>>   		 */
>> -		if (new_order || try_to_split_thp_page(page, /* new_order= */ 0,
>> -						       /* release= */ true)) {
>> +		if (new_order) {
>> +			pr_info("%#lx: order-%d folio cannot soft offline\n",
>> +				pfn, new_order);
>> +			folio_put(folio);
>> +			return -EBUSY;
>> +		}
>> +		if (try_to_split_thp_page(page, /* new_order= */ 0,
>> +					 /* release= */ true)) {
>>   			pr_info("%#lx: thp split failed\n", pfn);
>>   			return -EBUSY;
>>   		}
>> -- 
>> 2.25.1
>> 
>
>It seems me this will do -
>
>--- a/mm/memory-failure.c
>+++ b/mm/memory-failure.c
>@@ -2802,8 +2802,8 @@ 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 (new_order || try_to_split_thp_page(page, false)) {
>+                       put_page(page);
>                         pr_info("%#lx: thp split failed\n", pfn);
>                         return -EBUSY;
>                 }
>

>


Hi Jane,

Thanks for the suggestion! I've ended up going with David's more
comprehensive refactoring patch, which also fixes the refcount leak.
The v4 series has been sent out:
link:https://lore.kernel.org/all/20260806031958.677935-1-dayou5941@163.com/

Thanks,
Youhong


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

end of thread, other threads:[~2026-08-06  3:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-04  3:53 [PATCH v2] mm/memory-failure: fix refcount leak on soft-offline LBS folio dayou5941
2026-08-05 23:33 ` jane.chu
2026-08-06  3:24   ` 李佑鸿 

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