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

[Barry Song (Xiaomi)]

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.)

diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 621c8653d8f7..91396c796a34 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -239,7 +239,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);
 	else if (reclaiming)
 		gen = MAX_NR_GENS;
 	else if ((!folio_is_file_lru(folio) && !folio_test_swapcache(folio)) ||
@@ -247,7 +248,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);
 
 	return max(READ_ONCE(lrugen->max_seq) - gen + 1, READ_ONCE(lrugen->min_seq[type]));
 }
diff --git a/mm/folio.c b/mm/folio.c
index c02dcea9c03c..2fd835b3b50c 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -470,20 +470,10 @@ void folio_add_lru(struct folio *folio)
 			folio_test_unevictable(folio), folio);
 	VM_BUG_ON_FOLIO(folio_test_lru(folio), folio);
 
-	/*
-	 * For refaulted workingset folios, set PG_active so they
-	 * can be added to active generations.
-	 * For prefaulted file folios, folio_mark_accessed() sets
-	 * PG_referenced so lru_gen_folio_seq() places them into
-	 * the second oldest generation.
-	 */
+	/* see the comment in lru_gen_folio_seq() */
 	if (lru_gen_enabled() && !folio_test_unevictable(folio) &&
-	    lru_gen_in_fault() && !(current->flags & PF_MEMALLOC)) {
-		if (folio_test_workingset(folio))
-			folio_set_active(folio);
-		else if (!folio_test_referenced(folio))
-			folio_mark_accessed(folio);
-	}
+	    lru_gen_in_fault() && !(current->flags & PF_MEMALLOC))
+		folio_set_active(folio);
 
 	folio_batch_add_and_move(folio, lru_add);
 }
diff --git a/mm/vmscan.c b/mm/vmscan.c
index f11491ee9ed5..aa500ae9371a 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -851,11 +851,7 @@ static bool lru_gen_set_refs(struct folio *folio, const vma_flags_t *vma_flags)
 		return false;
 	}
 
-	/* Promote on second access */
-	if (folio_lru_refs(folio) > 1)
-		set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset));
-	else
-		folio_mark_accessed(folio);
+	set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset));
 	return true;
 }
 #else
diff --git a/mm/workingset.c b/mm/workingset.c
index 7ac2b88c80ae..a79b73ee9762 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -319,13 +319,11 @@ static void lru_gen_refault(struct folio *folio, void *shadow)
 
 	atomic_long_add(delta, &lrugen->refaulted[hist][type][tier]);
 
+	/* see folio_add_lru() where folio_set_active() will be called */
+	if (lru_gen_in_fault())
+		mod_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + type, delta);
+
 	if (workingset) {
-		/*
-		 * see folio_add_lru(), where folio_set_active() is
-		 * called for workingset folios
-		 */
-		if (lru_gen_in_fault())
-			mod_lruvec_state(lruvec, WORKINGSET_ACTIVATE_BASE + type, delta);
 		folio_set_workingset(folio);
 		mod_lruvec_state(lruvec, WORKINGSET_RESTORE_BASE + type, delta);
 	} else
-- 
2.34.1