Re: [PATCH 6/7] mm/mglru: fix potential generation folio number leak

[Kairui Song]

On Thu, Aug 20, 2026 at 9:52 AM Baolin Wang
<baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> On 8/18/26 1:38 PM, Kairui Song via B4 Relay wrote:
> > From: Kairui Song <kasong@xxxxxxxxxxx>
> >
> > Each generation of MGLRU accounts anon and file folio numbers
> > separately. The page table walker's update_batch_size() derives the
> > anon / file type of a folio from its current flags, but the page table
> > walk holds neither the lruvec lock nor the folio lock, so the type can
> > change during that period.
>
> Right.
>
> > MADV_FREE's lazyfree path clears PG_swapbacked under the lruvec lock,
> > so the folio is no longer considered on the anon LRU list. Lazyfreed
> > folios can also be changed back to the anon list again. If the flip
> > lands between folio_update_gen()'s cmpxchg and the type read in
> > update_batch_size(), the batched delta pair is applied to the wrong
> > type. The anon and file generation counters then carry phantom deltas
> > that nothing reconciles, permanently skewing lrugen->nr_pages and the
> > reclaim budgets derived from it.
>
> But I think the problem occurs between update_batch_size() and
> sort_folio(). update_batch_size() only updates the anon or file folio
> statistics, while sort_folio() moves promoted folios to the
> corresponding type's list:
>
> /* promoted */
> if (gen != lru_gen_from_seq(lrugen->min_seq[type])) {
>         list_move(&folio->lru, &lrugen->folios[gen][type][zone]);
>         return true;
> }
>
> If the folio's anon/file type changes between these two steps (e.g., a
> lazyfree folio), it would lead to what you described: "The anon and file
> generation counters then carry phantom deltas that nothing reconciles,
> permanently skewing lrugen->nr_pages and the reclaim budgets derived
> from it."

Actually no, the counters follow eventual consistency (note the word
"permanently"), we are fine with a drift as long as it will eventually
be corrected. Lazy promotions creates counter drift from the physical
location, but that is actually fixed by the sort_folio.

Now, for the type issue, use the lazyfree case as example (I think
that's actually the only place,), lru_lazyfree will remove the folio
form lruvec before marking it !PG_swapbacked, so during that removal
period, the gen bits are zero (folio's gen == -1), so any lazy
promotion CAS won't touch the counter, and only folio_update_gen will
do it since folio_inc_gen only handles on list folios. The
PG_swapbacked clearing in lazyfree only happens on folio wich has gen
== -1 (off-list). And it makes sense since update PG_swapbacked need
to update the counter and move the folio.

And if the CAS happends before the list removal, the list removal, the
folio is on the anon list, so the CAS is moving a folio in the anon
list, folio_update_gen will call update_batch_size asking it to update
the anon counters, we are fine after this commit. (Before this commit,
the CAS is moving a folio in the anon list but update_batch_size may
occur on file coutners). The list removal will update the anon
counter, and the subsequent list addition will account for the right
file counter.

And if the CAS happens after the list add, we are still fine since the
file counter is charged and sees a file type here.

>
> If you agree that this is where the problem lies, I don't see a good way
> to fix it, since the state of a lazyfree folio can change between
> update_batch_size() and sort_folio().

That's the tricky part and I struggled a lot with it since MGLRU-FG
will rely on this kind of lazy promotion very aggressively :)

I plan to do a proper cleanup of MGLRU's gen bits and PG_lru flag, and
introduce a cleaner convention later for all of this, and I think
maybe PG_lru can be merged with the GEN bits. But for now we are still
fine. The CAS change always explicitly checks for gen > 0: so as long
as we don't touch PG_swapbacked for gen >= 0 folios, we are fine. So
far every user is following this. We can fix that if an offending user
appears, and it's not hard to add a sanity check to catch
folio_clear/set_swapbacked that happens for gen >= 0 folios (maybe
check PG_lru too).

>
> A simple approach would be to skip checking the access flag for lazyfree
> folios during the page table walk, and let shrink_folio_list()
> reactivate accessed lazyfree folios instead. What do you think?

The thing is it can go from !PG_swapbacked to PG_swapbacked, or
PG_swapbacked to !PG_swapbacked. Or maybe lazy free isn't set the
moment you look at it but it got set the next moment?