Re: [RFC PATCH 1/2] mm: always call rmap_walk() on locked folios

[Lokesh Gidra <lokeshgidra@google.com>]

On Mon, Sep 8, 2025 at 5:40 PM Barry Song <21cnbao@gmail.com> wrote:
>
> On Tue, Sep 9, 2025 at 6:12 AM Lokesh Gidra <lokeshgidra@google.com> wrote:
> >
> > On Mon, Sep 8, 2025 at 2:47 PM Barry Song <21cnbao@gmail.com> wrote:
> > >
> > > On Mon, Sep 8, 2025 at 12:50 PM Lokesh Gidra <lokeshgidra@google.com> wrote:
> > > >
> > > > Prior discussion about this can be found at [1].
> > > >
> > > > rmap_walk() requires all folios, except non-KSM anon, to be locked. This
> > > > implies that when threads update folio->mapping to an anon_vma with
> > > > different root (currently only done by UFFDIO MOVE), they have to
> > > > serialize against rmap_walk() with write-lock on the anon_vma, hurting
> > > > scalability. Furthermore, this necessitates rechecking anon_vma when
> > > > pinning/locking an anon_vma (like in folio_lock_anon_vma_read()).
> > > >
> > > > This can be simplified quite a bit by ensuring that rmap_walk() is
> > > > always called on locked folios. Among the few callers of rmap_walk() on
> > > > unlocked anon folios, shrink_active_list()->folio_referenced() is the
> > > > only performance critical one.
> > >
> > > As I understand it, shrink_inactive_list() also invokes folio_referenced().
> > > Shouldn’t it be called just as often as shrink_active_list()?
> >
> > I'm only talking about those callers which call rmap_walk() without
> > locking anon folio. The
> > shrink_inactive_list()->folio_check_references()->folio_referenced()
> > path that you are talking about always locks the folio. So the
> > behavior in that case wouldn't change.
>
> Thanks for the clarification. Could you add a note about this if there
> is a v2?
>
Certainly, will do.
> > >
> > > >
> > > > shrink_active_list() doesn't act differently depending on what
> > > > folio_referenced() returns for an anon folio. So returning 1 when it
> > > > is contended, like in case of other folio types, wouldn't have any
> > > > negative impact.
> > >
> > > A complaint was raised that the LRU could become slightly disordered:
> > > https://lore.kernel.org/linux-mm/20240219141703.3851-1-lipeifeng@oppo.com/
> > >
> > > We can re-test to confirm if this is the case.
> > The patch in the link you provided is suggesting to control try-lock
> > for anon_vma lock. But here we are dealing with folio lock. Not sure
> > if the ordering issue will be there in this case.
>
> Right. Not sure what percentage of folios will be contended; I assume
> it is minor. Maybe you could share some data on this in a v2?

Any suggestion on how (or which test/benchmark) would be good to
gather data on this?

IIUC, shink_active_list() doesn't behave any differently whether there
is contention or not, except when it's an executable file folio. So I
doubt such data would be any useful. Please correct me if I'm wrong.
>
> Thanks
> Barry