Re: [PATCH v4 00/17] mm/mglru: frequency guided promotion (MGLRU-FG) and flag cleanup
From: Kairui Song
Date: Fri Oct 16 2026 - 08:12:15 EST
On Fri, Oct 16, 2026 at 7:43 PM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> On Fri, Oct 16, 2026 at 4:43 PM Barry Song (Xiaomi) <baohua@xxxxxxxxxx> wrote:
> >
> > On Fri, Oct 16, 2026 at 2:15 AM Kairui Song via B4 Relay <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> > [...]
> > >
> > > LevelDB Scan/Get
> > > ================
> > > I also retested the LevelDB benchmark from the cache_ext paper [5].
> > > Interestingly, mainline MGLRU already beats CLRU on this one after a
> > > recent change in lru_gen_folio_seq that bumps new folios with refs == 1
> > > to the second-oldest generation as discussed. That change accidentally
> > > gave random reads a higher hotness level while making sequential reads
> > > much colder as described before [14]. However it's unreliable and
> > > incorrect because it depends on LRU drain timing, and hurts some workloads
> > > as reported, or where the sequential part is actually hotter (any workload
> > > involving hotter large file and many small cold files will be affected)
> > > which is reproduced below.
> > >
> > > This series improves it while covering ordinary workloads too and should
> > > fix regressions:
> > >
> > > LevelDB Scan / Get, Throughput Total:
> > > CLRU: 4708.3 ops/s
> > > MGLRU: 5026.9 ops/s (faster than CLRU, but hurts other workloads)
> > > MGLRU After: 5179.2 ops/s (even better in all cases, and no regression)
> > >
> > > The hot-sequential and cold-random workload can be easily reproduced with
> > > SQLite and grep. SQLite continuously scans and looks up a small hot
> > > portion of a DB file, while grep iterates over a set of small files much
> > > larger than RAM [6]:
> > >
> > > SQLite scan & lookup time: Grep iterate time:
> > > CLRU: 14.51ms 13281.37ms
> > > MGLRU mainline: 567.05ms 13694.47ms
> > > MGLRU After this series: 10.58ms 12930.43ms
> > >
> >
> > Thanks, Kairui. I'll do a thorough review of the whole patchset over
> > the next two weeks.
> >
> > BTW, do you think we need a quick fix patch like the one below
> > for the above problem before this patchset, so that it can be
> > backported to stable? I assume your patchset is too large to
> > backport?
>
> Hi Barry,
>
> Good question, I think we can patch it first.
>
> > From f9683205d2ab6e3b185022d7d5af3496dfe5f35f Mon Sep 17 00:00:00 2001
> > From: "Barry Song (Xiaomi)" <baohua@xxxxxxxxxx>
> > Date: Fri, 16 Oct 2026 11:48:06 +0800
> > Subject: [PATCH] mm/mglru: fix using folio_mark_accessed to replace
> > folio_set_active
> >
> > Commit 6cbdd9726fb5 ("mm/mglru: use folio_mark_accessed to
> > replace folio_set_active") tried to avoid over-protecting
> > fault-around read-ahead file folios by placing them in the
> > second-oldest generation instead of the youngest generations.
> >
> > However, it abuses `PG_referenced` on read-ahead file folios
> > for this purpose, potentially coupling their handling with
> > normal syscall-based file accesses that also use
> > `PG_referenced`.
> >
> > This could regress some workloads, as reported by Kairui Song[1]
> > and Ehab Ababneh[2]. Many thanks to both for reporting these
> > issues!
> >
> > The long-term fix is Kairui's MGLRU-FG, but in the meantime,
> > we also need a quick fix that can be backported to stable.
> >
> > This patch reverts all of the original commit's behavior that
> > abuses `PG_referenced`, while preserving the semantics of
> > avoiding over-protection of fault-around read-ahead file
> > folios by adjusting only `lru_gen_folio_seq()`.
> > Specifically, fault-around read-ahead file folios with
> > `PG_active` set but `PG_workingset` clear are placed in the
> > second-oldest generation.
> > Activated file folios and refaulted workingset file folios
> > are still placed in the youngest generations.
> >
> > Build kernel cases in a memcg shows it not only perserve
> > the benefit of original patch, but also even improve
> > performance further.
> >
> > w/o fix:
> >
> > real 1m49.094s
> > user 25m31.484s
> > sys 3m40.959s
> > pswpin: 662112
> > pswpout: 2504415
> > pgpgin: 4424336
> > pgpgout: 10355273
> > refault_file: 359626
> > refault_anon: 670724
> >
> > w/ fix:
> >
> > real 1m48.913s
> > user 25m31.639s
> > sys 3m30.875s
> > pswpin: 689815
> > pswpout: 2689894
> > pgpgin: 4422659
> > pgpgout: 11043464
> > refault_file: 326566
> > refault_anon: 697138
> >
> > Revert original patch:
> >
> > real 1m50.044s
> > user 25m37.137s
> > sys 4m4.493s
> > pswpin: 1349087
> > pswpout: 4317933
> > pgpgin: 7083597
> > pgpgout: 17602735
> > refault_file: 337676
> > refault_anon: 1368542
> >
> > [1] https://lore.kernel.org/linux-mm/20261003-mglru-fg-v3-0-cbd4546a5bd9@xxxxxxxxxxx/
> > [2] https://lore.kernel.org/linux-mm/20260901180704.168106-1-ehab.ababneh@xxxxxxxxx/
> > Fixes: 6cbdd9726fb5 ("mm/mglru: use folio_mark_accessed to replace folio_set_active")
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > ---
> > include/linux/mm_inline.h | 8 +++++---
> > mm/folio.c | 16 +++-------------
> > mm/vmscan.c | 9 ++-------
> > mm/workingset.c | 10 ++++------
> > 4 files changed, 14 insertions(+), 29 deletions(-)
> >
> > diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> > index ab69b9930893..7aaff4e06e80 100644
> > --- a/include/linux/mm_inline.h
> > +++ b/include/linux/mm_inline.h
> > @@ -311,7 +311,8 @@ static inline unsigned long lru_gen_folio_seq(const struct lruvec *lruvec,
> > * | Accessed through page tables and | Accessed through file descriptors |
> > * | promoted by folio_update_gen() | and protected by folio_inc_gen() |
> > * +-----------------------------------+-----------------------------------+
> > - * | PG_active (set while isolated) | |
> > + * | PG_active (set while isolated) | PG_active (files| |
> > + * | | fault-readahead)| |
>
> This comment is not correct I think, faulting file folios are not
> access through fd, we may just better leave it uncommented.
>
> > * +-----------------+-----------------+-----------------+-----------------+
> > * | PG_workingset | PG_referenced | PG_workingset | LRU_REFS_FLAGS |
> > * +-----------------------------------+-----------------------------------+
> > @@ -319,7 +320,8 @@ static inline unsigned long lru_gen_folio_seq(const struct lruvec *lruvec,
> > * |<---------------------------- MAX_NR_GENS ---------------------------->|
> > */
> > if (folio_test_active(folio))
> > - gen = MIN_NR_GENS - folio_test_workingset(folio);
> > + gen = MIN_NR_GENS - folio_test_workingset(folio) +
> > + (type ? !folio_test_workingset(folio) : 0);
>
> This looks a bit hard to follow. Does it mean that for active but
> non-workingset files, they are put into the third newest generation?
> It creates more divergence of file / anon but if it work for now, I'm
> fine with it.
>
> > else if (reclaiming)
> > gen = MAX_NR_GENS;
> > else if ((!folio_is_file_lru(folio) && !folio_test_swapcache(folio)) ||
> > @@ -327,7 +329,7 @@ static inline unsigned long lru_gen_folio_seq(const struct lruvec *lruvec,
> > (folio_test_dirty(folio) || folio_test_writeback(folio))))
> > gen = MIN_NR_GENS;
> > else
> > - gen = MAX_NR_GENS - (folio_test_workingset(folio) || folio_test_referenced(folio));
> > + gen = MAX_NR_GENS - folio_test_workingset(folio);
>
> This is a nice change, but meanwhile the LevelDB and cerain workload
> gain are gone too, we will again have a ~40% regression for similiar
> workloads, I just double checked it. But that's fine, though, it was
> never meant to be optimized that way, and the long-term fix can be
> done later.
Here I mean the "optimization" of 6cbdd9726fb5 is more like a side
effect, and not reliable, that is simply not the goal of 6cbdd9726fb5.
So I think it stands well as a fixes for 6cbdd9726fb5.