RE: [RFC PATCH] mm/mglru: dynamically protect readahead fault folios under refault pressure

[Ababneh, Ehab]

> -----Original Message-----
> From: Barry Song <baohua@xxxxxxxxxx>
> Sent: Wednesday, September 2, 2026 2:58 PM
> To: Ababneh, Ehab <ehab.ababneh@xxxxxxxxx>
> Cc: ryncsn@xxxxxxxxx; akpm@xxxxxxxxxxxxxxxxxxxx;
> axelrasmussen@xxxxxxxxxx; kasong@xxxxxxxxxxx; lance.yang@xxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; qi.zheng@xxxxxxxxx;
> shakeel.butt@xxxxxxxxx; weixugc@xxxxxxxxxx; yuanchu@xxxxxxxxxx;
> yuzhao@xxxxxxxxxx
> Subject: Re: [RFC PATCH] mm/mglru: dynamically protect readahead fault
> folios under refault pressure
> 
> On Thu, Sep 3, 2026 at 5:52 AM Ababneh, Ehab <ehab.ababneh@xxxxxxxxx>
> wrote:
> >
> > Hi Barry, Kairui,
> >
> > Thanks for the feedback and suggestions.
> >
> > > -----Original Message-----
> > > From: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > > Sent: Tuesday, September 1, 2026 3:05 PM
> > > To: ryncsn@xxxxxxxxx; Ababneh, Ehab <ehab.ababneh@xxxxxxxxx>
> > > Cc: akpm@xxxxxxxxxxxxxxxxxxxx; axelrasmussen@xxxxxxxxxx;
> > > baohua@xxxxxxxxxx; kasong@xxxxxxxxxxx; lance.yang@xxxxxxxxx; linux-
> > > kernel@xxxxxxxxxxxxxxx; linux-mm@xxxxxxxxx; qi.zheng@xxxxxxxxx;
> > > shakeel.butt@xxxxxxxxx; weixugc@xxxxxxxxxx; yuanchu@xxxxxxxxxx;
> > > yuzhao@xxxxxxxxxx
> > > Subject: Re: [RFC PATCH] mm/mglru: dynamically protect readahead
> > > fault folios under refault pressure
> > >
> > > On Wed, Sep 2, 2026 at 2:21 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
> > > >
> > > > On Tue, Sep 01, 2026 at 11:06:43AM +0800, Ehab Ababneh wrote:
> > > > > Commit 6cbdd9726fb5 ("mm/mglru: use folio_mark_accessed to
> > > > > replace
> > > > > folio_set_active") introduced a regression for workloads that
> > > > > rely on readahead to keep sequential file access efficient.
> > > > >
> > > > > The problem is that MGLRU can place fault-path file folios in
> > > > > older generations, so memory pressure can reclaim readahead
> > > > > folios before the workload touches them. In our Cassandra read
> > > > > benchmark, this raised p99 latency to about 9.2-9.5 ms and cut
> > > > > throughput to roughly 41.8k-43.6k op/s; the revert restored the
> > > > > workload to about 5.5-5.6 ms and 51.9k-53.1k op/s.
> > > > >
> > > > > Readahead is important for sequential I/O and mmap scans, but it
> > > > > should not be retained when the workload does not benefit from it.
> > > > > The goal is to keep the optimization without keeping readahead
> > > > > pages alive
> > > forever.
> > > > >
> > > > > This patch provides a middle ground: keep the original behavior
> > > > > by default, but temporarily protect fault-path file folios when
> > > > > repeated file refaults show that readahead is actually helping.
> > > > >
> > > > > The mechanism is dynamic and self-tuning:
> > > > >
> > > > > - add a per-lruvec readahead/refault credit
> > > > > - accumulate credit on file refaults in the MGLRU refault path
> > > > > - consume credit in folio_add_lru() for fault-path file folios
> > > > > - keep the folio active while credit is available, and otherwise
> > > > > let the
> > > > >   original behavior stand
> > > > > - decay/reset the credit as generations advance and when an
> > > > > lruvec is
> > > > >   initialized
> > > > >
> > > > > This means we only protect fault-path file folios when refault
> > > > > pressure shows that the workload is actively benefiting from
> > > > > readahead. If the workload does not need that protection, the
> > > > > original optimization remains intact and we do not keep
> > > > > readahead pages
> > > around unnecessarily.
> > > > >
> > > > > Benchmark results for the Cassandra read workload
> > > > > (4 nodes, 720s, 100 readers):
> > > > >
> > > > > - with commit 6cbdd9726fb5 ("mm/mglru: use folio_mark_accessed to
> > > > >   replace folio_set_active"):
> > > > >   p99 ~9.2-9.5 ms, throughput ~41.8k-43.6k op/s
> > > > > - with revert of commit 6cbdd9726fb5 ("mm/mglru: use
> > > > > folio_mark_accessed to
> > > > >   replace folio_set_active"):
> > > > >   p99 ~5.5-5.6 ms, throughput ~51.9k-53.1k op/s
> > > > > - with this fix: p99 ~5.8 ms, throughput ~51.9k-52.7k op/s
> > > >
> > > [...]
> > > >
> > > > Just an idea. For an short term and easy fix, what if we simply
> > > > revert than, then only protect in_fault && folio_test_swapbacked
> > > > folios with PG_active?
> > >
> > > Hi Ehab and Kairui,
> > >
> > > Thanks very much for your reports and discussion.
> > >
> > > I'm not quite sure we want to revert it entirely, as that would
> > > immediately regress the workloads improved by the commit, such as the
> kernel build.
> > > Also, for example, Kairui's cover letter mentioned the LevelDB benchmark:
> > >  "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."
> > >
> > > I guess we could instead try to mitigate the cases where some
> > > workloads are negatively affected while preserving the original
> > > intention. Does the fix below address both of your cases, or is Ehab's case
> actually different from Kairui's?
> > > (The kernel-build test on my machine looks quite positive. It not
> > > only preserves the original optimization, but even provides further
> > > gains.)
> > >
> >
> > I agree that reverting the commit that caused the regression is not
> > the optimal path. I expect there are many workloads and scenarios that
> > benefit from the behavior introduced by that commit, so reverting it
> > could unnecessarily regress those workloads.
> >
> > I will run the Cassandra benchmark with Kairui's MGLRU-FG patches to
> > see whether they address the issue I am seeing. I will send the
> > results when they are ready.
> >
> 
> Thanks very much, Ehab. Could you please also run my proposed quick fix:
> 
> https://lore.kernel.org/linux-mm/20260901220430.79810-1-
> baohua@xxxxxxxxxx/
> 

Sure, I will do. I will report back when I have the results.

Thanks,
Ehab

> Best Regards
> Barry