Re: [PATCH v4] mm/memory-failure: fix folio refcount leak and locking in soft/hard offline

On Thu,  6 Aug 2026 11:19:58 +0800 [email protected] wrote:

> From: Li Youhong <[email protected]>
> 
> The min_order_for_split() function accesses folio->mapping without proper
> synchronization. In memory_failure(), the folio lock is dropped before the
> call, and in soft_offline_in_use_page(), the lock is not held at all. This
> means that while min_order_for_split() is executing, the value of
> folio->mapping may be modified by a truncate or invalidate operation,
> leading to a torn read or use of a stale mapping value.
> 
> Additionally, the soft_offline_in_use_page() path fails to release the
> folio reference taken by get_hwpoison_page() when new_order != 0, causing a
> reference leak.
> 
> Fix these issues by:
> - Moving the split operation logic into the callers and holding the folio
>   lock around min_order_for_split() and split_huge_page_to_order().
> - Removing the try_to_split_thp_page() helper to simplify refcount
>   handling.
> - Ensuring the folio reference is always dropped before returning on the
>   soft-offline error path.
> 
> This refactors the code to be more maintainable, fixes the locking issue
> reported by Sashiko, and also addresses the folio reference leak on the
> soft-offline error path.
> 
> ...
>
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
>  static void unmap_and_kill(struct list_head *to_kill, unsigned long pfn,
>  		struct address_space *mapping, pgoff_t index, int flags)
>  {
> @@ -2440,7 +2420,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;
>  
>  		/*
> @@ -2457,24 +2436,24 @@ 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);

AI review asks (effectively) why the try_to_split_thp_page() return value
never gets used

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

> +
> +		lock_page(p);

This code is a maddening mixture of `pages' and `folios'.  I assume
that migrating it over is a work in progress.

> +		err = split_huge_page_to_order(p, min_order_for_split(folio));
> +		unlock_page(p);
>  		/*
>  		 * 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);
> +		folio = page_folio(p);

We did that 40 lines earlier?

	folio = page_folio(p);

	/* filter pages that are protected from hwpoison test by users */
	folio_lock(folio);