Re: [RFC PATCH] mm/mglru: preserve inactive placement when enabling MGLRU
Ridong Chen <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.mm |
|---|---|
| Message-ID | <[email protected]> |
On 8/13/2026 7:37 PM, Barry Song wrote: > On Thu, Aug 13, 2026 at 7:24 PM Barry Song <[email protected]> wrote: >> >> On Thu, Aug 13, 2026 at 7:02 PM Ridong Chen <[email protected]> wrote: >>> >>> From: Ridong Chen <[email protected]> >>> >>> When the LRU is switched to MGLRU (echo y > /sys/kernel/mm/lru_gen/ >>> enabled), fill_evictable() re-inserts every folio via >>> lru_gen_add_folio(..., false). With reclaiming hardcoded to false, an >>> inactive anonymous folio (no PG_active, not in the swapcache) takes the >>> "gen = MIN_NR_GENS" branch in lru_gen_folio_seq() and is seeded at >>> seq = max_seq - 1, which lru_gen_is_active() treats as active. Its >>> inactive placement is lost and NR_INACTIVE_ANON is folded into >>> NR_ACTIVE_ANON. >>> >>> Pass reclaiming=!active so a folio from an inactive list is seeded into >>> an older generation. Folios from the active list carry PG_active and >>> hit the first branch either way, so they are unchanged. >>> >>> Tested on x86_64, next-20260812, 2G VM + 1G swap, ~1.5G anon pushed onto >>> the inactive list before enabling MGLRU: >>> >>> Active(anon) Inactive(anon) >>> before switch (legacy) 2952 1548792 kB >>> after `echo y`, unpatched 1552052 0 kB >>> after `echo y`, patched 15144 1536636 kB >>> >>> Inactive file folios stay inactive either way (NR_INACTIVE_FILE is >>> preserved). >>> >>> Fixes: 354ed5974429 ("mm: multi-gen LRU: kill switch") >>> Assisted-by: Claude:claude-opus-4-8 >>> Signed-off-by: Ridong Chen <[email protected]> >>> --- >>> mm/vmscan.c | 7 ++++++- >>> 1 file changed, 6 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>> index 94fc4f25e99f..2befc8d7dd3f 100644 >>> --- a/mm/vmscan.c >>> +++ b/mm/vmscan.c >>> @@ -5319,7 +5319,12 @@ static bool fill_evictable(struct lruvec *lruvec) >>> VM_WARN_ON_ONCE_FOLIO(folio_lru_gen(folio) != -1, folio); >>> >>> lruvec_del_folio(lruvec, folio); >>> - success = lru_gen_add_folio(lruvec, folio, false); >>> + /* >>> + * Keep a folio from the inactive list inactive: >>> + * pass reclaiming=!active so it is not seeded as >>> + * active. See lru_gen_folio_seq(). >>> + */ >>> + success = lru_gen_add_folio(lruvec, folio, !active); >> Hi Barry, Thank you for your reply. >> This is a very interesting use of the reclaim argument, as it is not >> intended to serve this MGLRU switch purpose. It is really only meant >> for `folio_rotate_reclaimable()`. >> Yeah, I realize tying the reclaim argument to the MGLRU switch is a bit of a hack, since it was really only designed for folio_rotate_reclaimable(). That said, I'm not blind to it, I just posted an RFC patch to get the discussion rolling and see what people think. >> However, the change itself seems to be *partially* correct and >> *partially* wrong. >> >> One real issue is that inactive is always placed in the oldest >> generation, while we have two old generations. Maybe we can ignore >> this for now. >> >> but somehow, are we also inverting the cold/hot ordering in the >> inactive list? >> >> `lru_to_folio(head)` always takes the tail, but now we are putting the >> tail before the head folios. >> >> Because reclaim == true will use list_add_tail(). >> >> if (reclaiming) >> list_add_tail(&folio->lru, &lrugen->folios[gen][type][zone]); > > I guess we can fix this by iterating in `fill_evictable()` from head > to tail order. > > struct list_head *pos = head->next; > > while (pos != head) { > struct folio *folio = list_entry(pos, struct folio, lru); > ... > } > > Then the oldest generation will maintain the same folio order as the > inactive list. > Thanks for the suggestion. Traversing head to tail does fix the inversion, but we need to distinguish active from inactive during iteration, because they are inserted differently: list_add_tail() when reclaiming, and list_add() otherwise. That said, the reclaim argument usage still feels off to me—I'd like to hear if others have a better idea before we proceed with further changes. -- Best regards Ridong