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

On Thu, Aug 20, 2026 at 4:53 PM Baolin Wang
<[email protected]> wrote:
> On 8/20/26 11:45 AM, Kairui Song wrote:
> > On Thu, Aug 20, 2026 at 9:52 AM Baolin Wang
> > <[email protected]> wrote:
> >> On 8/18/26 1:38 PM, Kairui Song via B4 Relay wrote:
> >>> From: Kairui Song <[email protected]>
> >>>
> >>> 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.
>
> Thanks for the expalnation. Now I see the problem and I think your are
> right.
>
> (This involves the combination of the gen counter and the folio's type,
> so it would be better to have a diagram describing the race, otherwise
> it's indeed hard to follow.)

Thanks for the confirmation, I'll try my best but that's hard to
describe :(, will also try send a proper cleanup later maybe also
clean up and merge PG_lru if doable to make the convention cleaner.