Re: [PATCH] mm/swap: fix swap_cache_get_folio() returning a folio that left the swap cache

[Kairui Song]

On Fri, Oct 16, 2026 at 11:00 AM Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Fri, 16 Oct 2026 10:22:20 +0800 Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> > On Fri, Oct 16, 2026 at 10:16 AM Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
> > >
> > > Hi Andrew,
> > >
> > > On 10/16/26 9:40 AM, Andrew Morton wrote:
> > > > On Thu, 15 Oct 2026 14:05:27 +0800 Jiayuan Chen <jiayuan.chen@xxxxxxxxx> wrote:
> > > >
> > > [...]
> >
> > Hi all,
> >
> > > >
> > > > So CPU 0 writes to a folio it doesn't own: it may clear PG_readahead and
> > > > takes the folio lock of someone else's page, before
> > > > folio_matches_swap_entry() finds out the folio is not ours.
> > > >
> > > > Recheck the slot after folio_try_get() and retry if it no longer holds
> > > > this folio.
> > > > Thanks.
> > > >
> > > > As always, when fixing a bug please fully describe the worst-case
> > > > userspace-visible runtime effects of that bug.
> > > >
> > > > I asked $LLM for this and was told
> > > >
> > > > : When the race occurs, a swap fault can acquire a reference to a
> > > > : physical page that has been freed and reused for an unrelated purpose,
> > > > : potentially by another process.  The fault handler may then clear that
> > > > : page's PG_readahead/PG_reclaim flag, wait on or acquire its folio lock,
> > > > : and potentially attempt to remove it from an unrelated swap cache.
> > > > :
> > > > : These operations can interfere with unrelated workloads and may cause
> > > > : unexpected stalls, impaired reclaim or writeback, and potentially
> > > > : system hangs.  The subsequent swap-entry validation normally prevents
> > >
> > >
> > > I don't see a way to a system hang: the folio lock waiter is woken when
> > > the real owner unlocks, so the worst case is a stall. I suggest
> > > dropping "and potentially system hangs".
>
> Yes, I think it's assuming the other-owned folio never gets unlocked.
>
> > >
> > > Writeback itself is not affected, only the PG_reclaim hint on where to
> > > put the folio after writeback. So "impaired reclaim or writeback" ->
> > > "impaired reclaim".
> > >
> > > So I'd suggest:
> > >
> > >    : When the race occurs, a swap fault can acquire a reference to a
> > >    : physical page that has been freed and reused for an unrelated purpose,
> > >    : potentially by another process.  The fault handler may then clear that
> > >    : page's PG_readahead/PG_reclaim flag, wait on or acquire its folio lock,
> > >    : and potentially attempt to remove it from an unrelated swap cache.
> > >    :
> > >    : These operations can interfere with unrelated workloads and may cause
> > >    : unexpected stalls and impaired reclaim.  The subsequent swap-entry
> > >    : validation normally prevents the wrong folio from being mapped into
> >
> > It's documented that caller always double checks the folio so this
> > part is harmless from the aspect of data corruption or crash.
>
> OK, thanks, adjustment made.
>
> > > >> Fixes: 8578e0c00dcf ("mm, swap: use the swap table for the swap cache and switch API")
> > > > Given the above, I suggest adding cc:stable.
> > > >
> > > > Do you and maintainers agree?
> > >
> > > Agreed for me.
> > >
> >
> > This is more like a optimization than a bug fix, but it's clean and
> > easy to follow, Cc'ing stable is fine to me.
>
> Was that an ack ;)
>

Right it looks good to me :D

Reviewed-by: Kairui Song <kasong@xxxxxxxxxxx>