Re: [PATCH] mm/memory-failure: fix refcount leak on soft-offline LBS folio

Andrew Morton <[email protected]> Mon, 3 Aug 2026 15:06:46 -0700
Newsgroups org.kvack.linux-mm
Message-ID <[email protected]>
On Mon,  3 Aug 2026 14:00:01 +0800 [email protected] wrote:

> From: liyouhong <[email protected]>
> 
> soft_offline_in_use_page() runs with a folio reference taken by
> get_hwpoison_page() (or by the caller when MF_COUNT_INCREASED is set,
> e.g. madvise(MADV_SOFT_OFFLINE)).
> 
> made soft-offline skip splitting when min_order_for_split()

Something went wrong with this paragraph?

> is non-zero, so large-block-size folios stay intact.  That early
> return -EBUSY path never drops the reference.

So the effect is to leak a folio ref if min_order_for_split() returns
0.  Has this been known to happen in the real world?

It would be good to update soft_offline_in_use_page()'s introductory
comment to describe its folio refcount handling.

> 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.
> 
> Split the new_order != 0 case out and folio_put() before returning.
> 
> ...
>
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2801,9 +2801,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 ref from get_hwpoison_page()/MF_COUNT_INCREASED;
> +		 * try_to_split_thp_page(..., release=true) does that 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;
>  		}

Thanks, lgtm.

AI review might have found a couple of pre-existing issues in the
surrounding code.  From a quick read, it doesn't seem appropriate to
address either within the context of this change.

	https://sashiko.dev/#/patchset/[email protected]