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

[Kairui Song]

On Sat, Oct 31, 2026 at 6:54 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> 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".

I think we better keep it, a dirty folio in the swap cache must not be
dropped directly or it can lose data: the copy on the swap device is stale,
so a later swapin of a shared entry reads that stale copy back. If it
triggers it means somewhere forgot to write back a dirty folio properly.

We may generalize this check into the swap cache helper though,
something like:

WARN_ON_ONCE(folio_swapped(folio) && folio_test_dirty(folio));

There are some special users so it's kept out side.

> 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?

It seems fine? The folio is removed from the swap cache, so the next pass
will go on. But you're right about the dirty and WARN case, it spins, so
maybe:

   if (unlikely(folio_test_large(folio) && !folio_test_anon(folio))) {
      if (WARN_ON_ONCE(folio_test_dirty(folio)))
         return -EBUSY;
      swap_cache_del_folio(folio);
      return 0;
   }

We won't run into this unless there are some other bugs I think.