Re: [PATCH 3/3] mm, swap: fix large swap-in across swap file extents

[Kairui Song]

On Fri, Oct 30, 2026 at 5:21 AM Nhat Pham <nphamcs@xxxxxxxxx> wrote:
>
> On Thu, Oct 29, 2026 at 11:32 AM Kairui Song via B4 Relay
> <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> >
> > From: Chris Mason <mason@xxxxxxxxxx>
> >
> > A page fault can give an unprivileged process blocks of other users'
> > files instead of its own swapped-out data. This affects systems with a
> > swap file on a synchronous block device, such as brd or zram. For an
> > anonymous mapping it also takes a huge page size below the PMD size
> > (mTHP), such as 64 KiB, that is enabled for anonymous memory.
> >
> > A swap area is divided into slots of one page each. A swap file is a
> > regular file, so adjacent swap slots can lie in two extents, far apart
> > on disk. A folio is read from swap with a single bio that starts at the
> > sector of the folio's first slot. So all slots of a large folio must be
> > contiguous on disk.
> >
> > The swap allocator knows this: it only hands out a run of slots for a
> > large folio if the swap area is a block device (SWP_BLKDEV).
> >
> > The swap-in path does not. On a synchronous device (SWP_SYNCHRONOUS_IO),
> > one that always completes I/O in the submitter's context, do_swap_page()
> > can read several swapped-out pages as one large folio (a large swap-in).
> > thp_swapin_suitable_orders() makes that decision, and looks only at the
> > swap offsets (the slot numbers) in the page table.
> >
> > Pages that were swapped out one at a time can end up in consecutive,
> > aligned slots, and are then read as one large folio. If the extent of
> > the first slot ends inside that folio, the rest of the folio is filled
> > from the blocks that follow the extent on disk. These blocks can belong
> > to other files.
> >
> > Reproduced with a fragmented swap file on brd and 64 KiB folios enabled.
> > After single pages were swapped out, a fault returned blocks of a
> > root-owned file with mode 0600.
> >
> > Fix this by allowing a large swap-in only when SWP_BLKDEV is set.
> >
> > Known gaps:
> > - A swap file on zram, brd, pmem or btt is now read one page at a time,
> >   even where its slots lie in one extent. Its swpin counters in
> >   /sys/kernel/mm/transparent_hugepage/hugepages-*/stats stay at 0.
>
>
> I guess we can check for this right? Look for the swap extent of the
> first offset, then get extent's size (swap_extent->nr_pages).
>
> Not sure if these users use swap file here though.

Thanks for the review!

I think it's really not a good idea to use swap over FS on a
zram/brd/pmem device, and we already disable mTHP allocation for any
such setups, and extents is a burden there. Without high-order
allocation, there is no high-order swapout, so high-order swapin seems
much less helpful.

So keeping the fix simple is a good first step, IMO. We may revisit
this after mTHP allocation is doable on these devices, and get rid of
the extent completely.

> > - With the posted "mm, swap: enable THP swapin for xswap entries"
> >   applied as well, the large swap-in that it adds never happens. That
> >   patch gives its swap device SWP_SYNCHRONOUS_IO but not SWP_BLKDEV, so
> >   the new check would have to let that device pass.
>
> Mentioning unmerged patches in changelog? ;)

I can help simplify that part.

>
> >
> > Fixes: 242d12c98174 ("mm: support large folios swap-in for sync io devices")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Assisted-by: LLM
> > Signed-off-by: Chris Mason <mason@xxxxxxxxxx>
> > Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
> > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> > ---
> >  mm/memory.c | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index b65b56fe9714..944f57153c89 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -4863,7 +4863,8 @@ static inline unsigned long thp_swap_suitable_orders(pgoff_t swp_offset,
> >         return orders;
> >  }
> >
> > -static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> > +static unsigned long thp_swapin_suitable_orders(struct swap_info_struct *si,
> > +                                               struct vm_fault *vmf)
> >  {
> >         struct vm_area_struct *vma = vmf->vma;
> >         unsigned long orders;
> > @@ -4888,6 +4889,14 @@ static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> >         if (!zswap_never_enabled())
> >                 return 0;
> >
> > +       /*
> > +        * Neighbouring slots of a swap file can lie in two extents, and a folio
> > +        * is read with one bio. The flag is set in swapon() and stays, so the
> > +        * race is benign.
> > +        */
> > +       if (!data_race(si->flags & SWP_BLKDEV))
> > +               return 0;
> > +
> >         entry = softleaf_from_pte(vmf->orig_pte);
> >         /*
> >          * Get a list of all the (large) orders below PMD_ORDER that are enabled
> > @@ -4924,7 +4933,8 @@ static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> >         return orders;
> >  }
> >  #else /* !CONFIG_TRANSPARENT_HUGEPAGE */
> > -static unsigned long thp_swapin_suitable_orders(struct vm_fault *vmf)
> > +static unsigned long thp_swapin_suitable_orders(struct swap_info_struct *si,
> > +                                               struct vm_fault *vmf)
>
> Nice, I think Sashiko is complaining about it on one of my vswap
> patches lol, and I was wondering how on earth is correct in non-vswap
> case.
>
> Fix LGTM:
>
> Reviewed-by: Nhat Pham <nphamcs@xxxxxxxxx>

Thanks!