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

[Kairui Song <ryncsn@gmail.com>]

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

Hello Ehab,

We ran into the same issue on our side too. I hesitated to report or fix
that as I'm working on MGLRU-FG which fixed the problem on my side:

https://lore.kernel.org/linux-mm/20260804-mglru-fg-v1-0-4d8dad39dad6@tencent.com/

I especially mentioned it, see the parts after:
"recent change in lru_gen_folio_seq that bumps new folios with refs == 1"

Latest version still being tested which you can use directly:
https://github.com/ryncsn/linux/commits/b4/mglru-fg-v1.8/

Do you mind have a look of that as well? I think in the long term that is
the right direction. With our test the regression is gone and performance
is even better.

And is there any easy way to reproduce the specific case you are reporting?

> The fix restores the readahead protection lost by the regression while
> preserving the original intent of the optimization: do not keep
> readahead pages around if the workload does not need them.
> 
> Fixes: 6cbdd9726fb5 ("mm/mglru: use folio_mark_accessed to replace folio_set_active")
> Signed-off-by: Ehab Ababneh <ehab.ababneh@intel.com>
> ---
>  include/linux/mmzone.h |  2 ++
>  mm/swap.c              | 82 ++++++++++++++++++++++++++++++++++++++----
>  mm/vmscan.c            |  7 ++++
>  mm/workingset.c        | 18 ++++++++++
>  4 files changed, 102 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index ca2712187147..c998b1e0b8a7 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -578,6 +578,8 @@ struct lru_gen_folio {
>  	/* can be modified without holding the LRU lock */
>  	atomic_long_t evicted[NR_HIST_GENS][ANON_AND_FILE][MAX_NR_TIERS];
>  	atomic_long_t refaulted[NR_HIST_GENS][ANON_AND_FILE][MAX_NR_TIERS];
> +	/* credit: file refaults indicate fault-path file folios need protection */
> +	atomic_long_t ra_refaults;
>  	/* whether the multi-gen LRU is enabled */
>  	bool enabled;
>  	/* the memcg generation this lru_gen_folio belongs to */
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..a31c9000868a 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -70,6 +70,70 @@ static DEFINE_PER_CPU(struct cpu_fbatches, cpu_fbatches) = {
>  	.lock_irq = INIT_LOCAL_LOCK(lock_irq),
>  };
>  
> +#ifdef CONFIG_LRU_GEN
> +/* Refill two default readahead windows to amortize shared-counter updates. */
> +#define RA_REFAULT_LOCAL_BATCH	(VM_READAHEAD_PAGES * 2)
> +
> +struct ra_credit_cache {
> +	/* Batch shared credit per CPU to avoid a contended atomic RMW per folio. */
> +	/* only compared for identity, never dereferenced */
> +	struct lru_gen_folio *lrugen;
> +	long credit;
> +};
> +
> +static DEFINE_PER_CPU(struct ra_credit_cache, ra_credit_cache);
> +
> +/*
> + * Spend readahead protection credit from a per-CPU bucket, refilled in batches
> + * from the shared per-lruvec counter, so the fault path avoids an atomic RMW on
> + * a contended cacheline for every folio.
> + */
> +static bool lru_gen_take_ra_credit(struct folio *folio)
> +{
> +	struct lru_gen_folio *lrugen;
> +	long nr_pages = folio_nr_pages(folio);
> +	struct ra_credit_cache *cache;
> +	bool taken = false;
> +	long old, new;
> +
> +	rcu_read_lock();
> +	lrugen = &folio_lruvec(folio)->lrugen;
> +	cache = get_cpu_ptr(&ra_credit_cache);
> +
> +	/* credit cached for a different lruvec is forfeited, bounded by the batch */
> +	if (cache->lrugen != lrugen) {
> +		cache->lrugen = lrugen;
> +		cache->credit = 0;
> +	}
> +
> +	if (cache->credit < nr_pages) {
> +		old = atomic_long_read(&lrugen->ra_refaults);
> +		while (old > 0) {
> +			new = old - min_t(long, old, RA_REFAULT_LOCAL_BATCH);
> +			if (atomic_long_try_cmpxchg(&lrugen->ra_refaults, &old, new)) {
> +				cache->credit += old - new;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (cache->credit >= nr_pages) {
> +		cache->credit -= nr_pages;
> +		taken = true;
> +	}
> +
> +	put_cpu_ptr(&ra_credit_cache);
> +	rcu_read_unlock();
> +
> +	return taken;
> +}
> +#else
> +static bool lru_gen_take_ra_credit(struct folio *folio)
> +{
> +	return false;
> +}
> +#endif /* CONFIG_LRU_GEN */
> +

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?