Re: [PATCH] mm/mglru: Use folio_mark_accessed to replace folio_set_active in PF

[Barry Song]

On Mon, Apr 27, 2026 at 10:46 PM Pedro Falcato <pfalcato@xxxxxxx> wrote:
>
> On Sun, Apr 26, 2026 at 12:35:46PM +0800, Barry Song wrote:
> > On Fri, Apr 24, 2026 at 11:19 PM Pedro Falcato <pfalcato@xxxxxxx> wrote:
> > >
> > > On Sat, Apr 18, 2026 at 08:02:33PM +0800, Barry Song (Xiaomi) wrote:
> > > > MGLRU gives high priority to folios mapped in page tables.
> > > > As a result, folio_set_active() is invoked for all folios
> > > > read during page faults. In practice, however, readahead
> > > > can bring in many folios that are never accessed via page
> > > > tables.
> > > >
> > > > A previous attempt by Lei Liu proposed introducing a separate
> > > > LRU for readahead[1] to make readahead pages easier to reclaim,
> > > > but that approach is likely over-engineered.
> > >
> > > Why does this even need to be kept? I'm not sure it makes sense
> > > to even mark readahead folios as referenced.
> > >
> > > I'd suggest folios should only be marked referenced (or even active, whatever)
> > > when they're mapped. Anything else is a bit random and is hoping you are
> > > eventually going to map them in the future (which is not true for, for example,
> > > anything in an ELF file that may be readahead but not mapped, like debug info,
> > > symbol tables, section headers, relocation tables, etc etc)
> >
> > The patch targets the mmap readahead path rather than the syscall
> > readahead path.
>
> Yes.
>
> >
> > With lru_gen_in_fault() in place, it’s roughly equivalent to
> > the mapped case, since readahead is typically 128 KB while
> > fault_around is 64 KB in PF.
>
> I'm not sure I understand. How is 128KB roughly equivalent to 64KB?
> That's almost a 2x difference!

For two reasons. First, we have moved those folios out of
active generations, so they no longer significantly occupy
active space in MGLRU. In any case, they require a real PTE
reference to be promoted in folio_check_references():

        if (lru_gen_enabled() && !lru_gen_switching()) {
                if (!referenced_ptes)
                        return FOLIOREF_RECLAIM;

                return lru_gen_set_refs(folio) ? FOLIOREF_ACTIVATE :
FOLIOREF_KEEP;
        }


Second, 64KB is effectively similar to 128KB when two
subpages within each 64KB are used by fault_around /
map_pages.

Of course, we could move the setting into multiple map
functions, but given such a small gap, I do not think it is
worth the added complexity.

>
> And readahead, as of now, will read beyond the VMA's limits (which will not
> be mapped).

Folios beyond the VMA limits will not be promoted in MGLRU.
But yes, I like the idea we don't readahead beyond vma limits:

https://lore.kernel.org/linux-mm/20260422005608.342028-1-fmayle@xxxxxxxxxx/

>
> Really, it's extremely unclear why this adhoc heuristic is here. A folio isn't
> active just because you started readahead for it inside a page fault. It's not
> even necessarily active just because it's mapped (although that is more
> understandable).

Right. And with fault_around enabled, I don’t think it is
correct to set the folio active when it is mapped.

>
> And of course, because the whole ordeal is already extremely simple, it turns
> out that in_lru_fault doesn't actually mean "we're inside a page fault" but
> "we're inside a page fault and this VMA/file don't have any sequential/random
> annotations".
>
> I would really like to understand why this is here (and this is true for the
> various odd heuristics mglru uses) if we ever want to have a chance to merge
> the two LRUs together.

I’m probably not the right person to answer this question, as I’m
sending this patch to change it :-)

It might make sense to promote PF folios directly if there is no
readahead or fault_around.

Thanks
Barry