Re: [RFC PATCH 1/1] mm/mglru: age when the oldest generation is thrashing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/mglru: repeated back-to-back aging in oldest_gen_thrashing()
--

commit 17216aa0e22cf9bf70e1a34fa529d94d53089f2e
Author: Maxim Muzafarov <maxmuzaf@gmail.com>

mm/mglru: age when the oldest generation is thrashing

This commit triggers aging in should_run_aging() when the number of refaulted
pages in the oldest file generation exceeds the total evictable size of the
lruvec.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f11491ee9ed5c..f59b29723ec94 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4966,8 +4966,26 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>  	return scanned;
>  }
>  
> +static bool oldest_gen_thrashing(struct lruvec *lruvec, unsigned long *min_seq,
> +				 int swappiness, unsigned long evictable)
> +{
> +	int tier;
> +	unsigned long refaulted = 0;
> +	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> +	int hist = lru_hist_from_seq(min_seq[LRU_GEN_FILE]);
> +
> +	if (swappiness == SWAPPINESS_ANON_ONLY)
> +		return false;
> +
> +	for (tier = 0; tier < MAX_NR_TIERS; tier++)
> +		refaulted += atomic_long_read(&lrugen->refaulted[hist][LRU_GEN_FILE][tier]);
> +
> +	return refaulted > evictable;
> +}
> +
>  static bool should_run_aging(struct lruvec *lruvec, unsigned long max_seq,
> -			     struct scan_control *sc, int swappiness)
> +			     struct scan_control *sc, int swappiness,
> +			     unsigned long evictable)
>  {
>  	DEFINE_MIN_SEQ(lruvec);
>  
> @@ -4975,6 +4993,9 @@ static bool should_run_aging(struct lruvec *lruvec, unsigned long max_seq,
>  	if (evictable_min_seq(min_seq, swappiness) + MIN_NR_GENS > max_seq)
>  		return true;
>  
> +	if (oldest_gen_thrashing(lruvec, min_seq, swappiness, evictable))
> +		return true;

[Severity: Medium]
Can this trigger repeated back-to-back aging passes when the file LRU has
fewer than MAX_NR_GENS generations?

In should_run_aging(), oldest_gen_thrashing() is checked without verifying
whether the file LRU has reached MAX_NR_GENS generations (for example, when
there are 3 file generations at DEF_PRIORITY, or 2 file generations when
min_seq[LRU_GEN_ANON] lags behind min_seq[LRU_GEN_FILE]).

When oldest_gen_thrashing() returns true while get_nr_gens(lruvec,
LRU_GEN_FILE) is less than MAX_NR_GENS, try_to_shrink_lruvec() calls
try_to_inc_max_seq() -> inc_max_seq():

mm/vmscan.c:inc_max_seq() {
	...
	for (type = 0; type < ANON_AND_FILE; type++) {
		if (get_nr_gens(lruvec, type) != MAX_NR_GENS)
			continue;

		if (inc_min_seq(lruvec, type, swappiness))
			continue;
	...
}

Because get_nr_gens(lruvec, LRU_GEN_FILE) != MAX_NR_GENS, inc_max_seq() does
not call inc_min_seq() for LRU_GEN_FILE. As a result, min_seq[LRU_GEN_FILE]
does not advance, and reset_ctrl_pos(lruvec, LRU_GEN_FILE, true) is not
called to clear or decay lrugen->refaulted[hist][LRU_GEN_FILE] (while
reset_ctrl_pos(lruvec, LRU_GEN_FILE, false) only clears max_seq + 1 when
NR_HIST_GENS > 1, and does nothing when NR_HIST_GENS == 1).

On the next iteration of the loop in try_to_shrink_lruvec() (such as during
cgroup or proactive reclaim where !root_reclaim(sc) continues looping) or on
the next reclaim invocation, should_run_aging() will see the same un-reset
lrugen->refaulted[hist][LRU_GEN_FILE] > evictable and immediately run
try_to_inc_max_seq() again until the file generation count reaches
MAX_NR_GENS and forces inc_min_seq().

Should oldest_gen_thrashing() also check that min_seq[LRU_GEN_FILE] +
MAX_NR_GENS - 1 == max_seq so that aging is only triggered when
inc_max_seq() will actually advance min_seq[LRU_GEN_FILE] and reset its
refault counters?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261101160427.85701-1-maxmuzaf@gmail.com?part=1