Re: [PATCH v8 02/16] mm/mglru: extract lruvec_populated_min_seq() helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] mm/mglru: off-by-one loop bound in lruvec_populated_min_seq()
--

commit 9751ca1c29ca665233058e3a3c3cfa85cc385872
Author: Kairui Song <kasong@tencent.com>

mm/mglru: extract lruvec_populated_min_seq() helper

This commit extracts the search for the oldest populated generation of a
given type from try_to_inc_min_seq() into a new lruvec_populated_min_seq()
helper bounded by max_seq.

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 400f87fc51e12..27dfbd3ff02b3 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3992,30 +3992,45 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  	return true;
>  }
>  
> +/* Similar to lruvec_evictable_size, return the oldest populated seq of the type */
> +static unsigned long lruvec_populated_min_seq(struct lruvec *lruvec, int type,
> +					      unsigned long max_seq)
> +{
> +	int gen, zone;
> +	unsigned long min_seq;
> +	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> +
> +	lockdep_assert_held(&lruvec->lru_lock);
> +
> +	min_seq = READ_ONCE(lrugen->min_seq[type]);
> +	while (min_seq <= max_seq) {
> +		gen = lru_gen_from_seq(min_seq);
> +		for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> +			if (!list_empty(&lrugen->folios[gen][type][zone]))
> +				return min_seq;
> +		}
> +		min_seq++;
> +	}
> +
> +	return max_seq;
> +}
> +
>  static void try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
>  {
> -	int gen, type, zone;
> +	int type;
>  	bool seq_inc_flag = false;
>  	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>  	DEFINE_MIN_SEQ(lruvec);
> +	unsigned long seq;
>  
>  	VM_WARN_ON_ONCE(!seq_is_valid(lruvec));
>  
> -	/* find the oldest populated generation */
> +	/* Requires at least MIN_NR_GENS gens for basic functionality */
> +	seq = lrugen->max_seq - MIN_NR_GENS;
>  	for_each_evictable_type(type, swappiness) {
> -		while (min_seq[type] + MIN_NR_GENS <= lrugen->max_seq) {
> -			gen = lru_gen_from_seq(min_seq[type]);
> -
> -			for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> -				if (!list_empty(&lrugen->folios[gen][type][zone]))
> -					goto next;
> -			}
> -
> -			min_seq[type]++;
> +		min_seq[type] = lruvec_populated_min_seq(lruvec, type, seq + 1);
> +		if (min_seq[type] > lrugen->min_seq[type])
>  			seq_inc_flag = true;
> -		}
> -next:
> -		;
>  	}

[Severity: Low]
Should lruvec_populated_min_seq() loop while (min_seq < max_seq) and
return min_seq at the end?

When min_seq == max_seq, both the early return inside the loop and the
final return evaluate to max_seq, so scanning all zones of generation
seq + 1 under lruvec->lru_lock is redundant work that the original loop
skipped.  In addition, if lrugen->min_seq[type] > max_seq on entry (which
happens in commit "mm/mglru: lift youngest gen protection for higher
priority" when scan_folios() runs at DEF_PRIORITY while get_nr_gens() is
MIN_NR_GENS), returning max_seq yields a sequence number below
lrugen->min_seq[type].

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113185541.70203-1-baohua@kernel.org?part=2