Re: [PATCH RFC 02/13] mm/huge_memory: fix rejection of swap cache folios with a mapping

"Zi Yan" <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
On Fri Aug 7, 2026 at 5:17 PM EDT, Kairui Song via B4 Relay wrote:
> From: Kairui Song <[email protected]>
>
> A folio in the swap cache cannot be split if it has a mapping (shmem).
> The split code only checks for this in __folio_freeze_and_split_unmapped,
> after the folio ref has been frozen and the NR_SHMEM_THPS/NR_FILE_THPS
> counters have been decremented, and returns -EINVAL without unfreezing
> the folio or restoring the counters. That error path is fragile: if it
> is ever taken, the folio is left frozen and stuck, the counters are
> skewed, and the VM_WARN_ON_ONCE_FOLIO would fire for a state that is
> actually legitimate.
>
> Check for this case up front in folio_check_splittable and return
> -EINVAL before any state is modified. Under DEBUG_VM, the existing
> "Tried to split an unsplittable folio" warning in __folio_split
> reports the rejection.

Should we return -EBUSY instead? -EINVAL means the caller should not
split a swapcache shmem with a mapping and the caller needs to avoid
that. The Fixes tag tells me a caller can split a swapcache shmem with a
mapping, so with -EINVAL, we will want to add checks at callers to avoid
it from happening.

>
> Fixes: 00527733d0dc ("mm/huge_memory: add two new (not yet used) functions for folio_split()")
> Fixes: 714b056c8321 ("mm/huge_memory: convert VM_BUG* to VM_WARN* in __folio_split")
> Signed-off-by: Kairui Song <[email protected]>
> ---
>  mm/huge_memory.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index ced400f72d43..2fa72158e063 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3878,6 +3878,9 @@ static int __split_unmapped_folio(struct folio *folio, int new_order,
>  int folio_check_splittable(struct folio *folio, unsigned int new_order,
>  			   enum split_type split_type)
>  {
> +	bool is_anon = folio_test_anon(folio);
> +	bool is_swapcache = folio_test_swapcache(folio);
> +
>  	VM_WARN_ON_FOLIO(!folio_test_locked(folio), folio);
>  	/*
>  	 * Folios that just got truncated cannot get split. Signal to the
> @@ -3886,11 +3889,11 @@ int folio_check_splittable(struct folio *folio, unsigned int new_order,
>  	 * TODO: this will also currently refuse folios without a mapping in the
>  	 * swapcache (shmem or to-be-anon folios).
>  	 */
> -	if (!folio->mapping && !folio_test_anon(folio))
> +	if (!folio->mapping && !is_anon)
>  		return -EBUSY;
>  
>  	/* order-1 is not supported for anonymous THP. */
> -	if (folio_test_anon(folio) && new_order == 1)
> +	if (is_anon && new_order == 1)
>  		return -EINVAL;
>  
>  	/*
> @@ -3901,7 +3904,7 @@ int folio_check_splittable(struct folio *folio, unsigned int new_order,
>  	 * swapcache folio split. Only uniform split to order-0 can be used
>  	 * here.
>  	 */
> -	if ((split_type == SPLIT_TYPE_NON_UNIFORM || new_order) && folio_test_swapcache(folio)) {
> +	if ((split_type == SPLIT_TYPE_NON_UNIFORM || new_order) && is_swapcache) {
>  		return -EINVAL;
>  	}
>  
> @@ -3911,6 +3914,15 @@ int folio_check_splittable(struct folio *folio, unsigned int new_order,
>  	if (folio_test_writeback(folio))
>  		return -EBUSY;
>  
> +	/*
> +	 * A non-anon swapcache folio that still has a mapping should only
> +	 * be a shmem folio under IO, there is little benefit in splitting
> +	 * them hence not supported. Reject it here up front: the split
> +	 * routine cannot back out cleanly once the folio ref is frozen.
> +	 */
> +	if (!is_anon && is_swapcache && folio->mapping)
> +		return -EINVAL;
> +
>  	return 0;
>  }
>  
> @@ -3983,14 +3995,8 @@ static int __folio_freeze_and_split_unmapped(struct folio *folio, unsigned int n
>  			}
>  		}
>  
> -		if (folio_test_swapcache(folio)) {
> -			if (mapping) {
> -				VM_WARN_ON_ONCE_FOLIO(mapping, folio);
> -				return -EINVAL;
> -			}
> -
> +		if (folio_test_swapcache(folio))
>  			ci = swap_cluster_get_and_lock(folio);
> -		}
>  
>  		/* lock lru list/PageCompound, ref frozen by page_ref_freeze */
>  		if (do_lru)




-- 
Best Regards,
Yan, Zi
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.