Re: [RFC PATCH] mm/mglru: preserve inactive placement when enabling MGLRU
Ridong Chen <[email protected]>
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 8/14/2026 10:31 AM, Barry Song wrote: > On Fri, Aug 14, 2026 at 10:22 AM Ridong Chen <[email protected]> wrote: >> >> >> >> 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. > > I think it’s fine. Just update the comment there and mention that it > can serve another purpose as well. > > [...] >>> 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. > > We can change reclaim to a tristate integer, or simply make the > following change and add a comment explaining why? > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 9e52f49114e7..5e624d0316bc 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -5403,7 +5403,8 @@ static bool fill_evictable(struct lruvec *lruvec) > > while (!list_empty(head)) { > bool success; > - struct folio *folio = lru_to_folio(head); > + struct folio *folio = active ? lru_to_folio(head) : > + list_entry((head)->next, struct folio, lru); > I've sent a new patch following this approach. Link: https://lore.kernel.org/linux-mm/[email protected]/T/#u > Or you could even move the entire inactive list to the oldest gen > instead of moving the folios one by one. Then you can avoid dealing > with the reclaim argument entirely. > However, since MGLRU distinguishes zones, we would have to move the folios one by one. So I've dropped this approach. -- Best regards Ridong