Re: [PATCH] mm/swapfile: drop a large unmapped swap-in folio in unuse_pte()

[Andrew Morton]

On Fri, 30 Oct 2026 15:11:56 -0400 Mani Tofigh <mt4022@xxxxxxxxxxxx> wrote:

> Since commit f1879e8a0c60 ("mm, swap: never bypass the swap cache even
> for SWP_SYNCHRONOUS_IO"), a fault can read a swapped-out mTHP range into
> one large folio in the swap cache.  The folio only becomes anon once
> do_swap_page() maps all of it, so if the faulting PTE changes in the
> meantime the folio stays in the swap cache while not being mapped or
> anon:
> 
>   Task A (fault)                       Task B
>   do_swap_page()
>     /* swaps in large folio F */
>                                        madvise(MADV_DONTNEED)
>                                          /* zaps the faulting PTE */
>     /* PTE changed (F not mapped) */
> 
> swapoff then maps it, which marks all of its pages as mapped and
> exclusive, and the next fault on one of the other PTEs crashes in
> do_swap_page():
> 
>   [    6.424485] kernel BUG at mm/memory.c:5195!
> 
> ...
>
> do_swap_page() already drops such a folio from the swap cache when it
> can't map all of it.  Do the same in unuse_pte() so that swapoff swaps
> the entries in again.  With the fix, nothing is reported.
> 
> This bug was identified and confirmed with the help of a deterministic
> hypervisor currently under development.  The C reproducer below however
> hits it within a second.
> 
> --- a/mm/swapfile.c
> +++ b/mm/swapfile.c
> @@ -2518,6 +2518,18 @@ static int unuse_pte(struct vm_area_struct *vma, pmd_t *pmd,
>  	if (!folio_matches_swap_entry(folio, entry))
>  		return 0;
>  
> +	/*
> +	 * A large folio that is not anon yet was swapped in by a page fault
> +	 * that did not map it (yet). It can only be mapped as a whole, by
> +	 * do_swap_page(). Drop it from the swap cache and swap in small
> +	 * folios instead.
> +	 */
> +	if (unlikely(folio_test_large(folio) && !folio_test_anon(folio))) {
> +		if (!WARN_ON_ONCE(folio_test_dirty(folio)))
> +			swap_cache_del_folio(folio);
> +		return 0;
> +	}

Let's not WARN here.  If it hits and someone reports it, what do we
tell them?  "uh, that's working as designed, no action needed".

Also, won't this change cause try_to_unuse() to keep spinning away until
someone kills it?  Might be better to return -EBUSY and fail the
swapoff?