Re: [PATCH v2 05/12] mm/mglru: scan and count the exact number of folios

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 3/29/26 3:52 AM, Kairui Song via B4 Relay wrote:
> From: Kairui Song <kasong@tencent.com>
> 
> Make the scan helpers return the exact number of folios being scanned
> or isolated. Since the reclaim loop now has a natural scan budget that
> controls the scan progress, returning the scan number directly should
> make the scan more accurate and easier to follow.
> 
> The number of scanned folios for each iteration is always positive and
> larger than 0, unless the reclaim must stop for a forced aging, so
> there is no more need for any special handling when there is no
> progress made:
> 
> - `return isolated || !remaining ? scanned : 0` in scan_folios: both
>    the function and the call now just return the exact scan count,
>    combined with the scan budget introduced in the previous commit to
>    avoid livelock or under scan.

Make sense to me.

> 
> - `scanned += try_to_inc_min_seq` in evict_folios: adding a bool as a
>    scan count was kind of confusing and no longer needed too, as scan
>    number will never be zero even if none of the folio in oldest
>    generation is isolated.

Yes, agree.

> 
> - `evictable_min_seq + MIN_NR_GENS > max_seq` guard in evict_folios:
>    the per-type get_nr_gens == MIN_NR_GENS check in scan_folios
>    naturally returns 0 when only two gens remain and breaks the loop.
> 
> Also move try_to_inc_min_seq before isolate_folios, so that any empty
> gens created by external folio freeing are also skipped.

This part is somewhat confusing. You probably mean the case where the 
list of that gen becomes empty via isolate_folio(), right?

If that's the case, the original logic would remove the empty gens 
produced by isolate_folio() after calling try_to_inc_min_seq().

However, with your changes, this removal won't happen until the next 
eviction. Does this provide any additional benefits? Or could you 
describe how this change impacts your testing?

> The scan still stops if there are only two gens left as the scan number
> will be zero, this behavior is same as before. This force gen protection
> may get removed or softened later to improve the reclaim a bit more.
> 
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>   mm/vmscan.c | 46 +++++++++++++++++++++++-----------------------
>   1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ab81ffdb241a..c5361efa6776 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4686,7 +4686,7 @@ static bool isolate_folio(struct lruvec *lruvec, struct folio *folio, struct sca
>   
>   static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   		       struct scan_control *sc, int type, int tier,
> -		       struct list_head *list)
> +		       struct list_head *list, int *isolatedp)
>   {
>   	int i;
>   	int gen;
> @@ -4756,11 +4756,9 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   				type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
>   	if (type == LRU_GEN_FILE)
>   		sc->nr.file_taken += isolated;
> -	/*
> -	 * There might not be eligible folios due to reclaim_idx. Check the
> -	 * remaining to prevent livelock if it's not making progress.
> -	 */
> -	return isolated || !remaining ? scanned : 0;
> +
> +	*isolatedp = isolated;
> +	return scanned;
>   }
>   
>   static int get_tier_idx(struct lruvec *lruvec, int type)
> @@ -4804,33 +4802,36 @@ static int get_type_to_scan(struct lruvec *lruvec, int swappiness)
>   
>   static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   			  struct scan_control *sc, int swappiness,
> -			  int *type_scanned, struct list_head *list)
> +			  struct list_head *list, int *isolated,
> +			  int *isolate_type, int *isolate_scanned)
>   {

8 parameters:), can we reduce some of them?

>   	int i;
> +	int scanned = 0;
>   	int type = get_type_to_scan(lruvec, swappiness);
>   
>   	for_each_evictable_type(i, swappiness) {
> -		int scanned;
> +		int type_scan;
>   		int tier = get_tier_idx(lruvec, type);
>   
> -		*type_scanned = type;
> +		type_scan = scan_folios(nr_to_scan, lruvec, sc,
> +					type, tier, list, isolated);
>   
> -		scanned = scan_folios(nr_to_scan, lruvec, sc, type, tier, list);
> -		if (scanned)
> -			return scanned;
> +		scanned += type_scan;
> +		if (*isolated) {
> +			*isolate_type = type;
> +			*isolate_scanned = type_scan;
> +			break;
> +		}
>   
>   		type = !type;
>   	}
>   
> -	return 0;
> +	return scanned;
>   }
>   
>   static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   			struct scan_control *sc, int swappiness)
>   {
> -	int type;
> -	int scanned;
> -	int reclaimed;
>   	LIST_HEAD(list);
>   	LIST_HEAD(clean);
>   	struct folio *folio;
> @@ -4838,19 +4839,18 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   	enum node_stat_item item;
>   	struct reclaim_stat stat;
>   	struct lru_gen_mm_walk *walk;
> +	int scanned, reclaimed;
> +	int isolated = 0, type, type_scanned;
>   	bool skip_retry = false;
> -	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>   	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
>   	struct pglist_data *pgdat = lruvec_pgdat(lruvec);
>   
>   	lruvec_lock_irq(lruvec);
>   
> -	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness, &type, &list);
> -
> -	scanned += try_to_inc_min_seq(lruvec, swappiness);
> +	try_to_inc_min_seq(lruvec, swappiness);
>   
> -	if (evictable_min_seq(lrugen->min_seq, swappiness) + MIN_NR_GENS > lrugen->max_seq)
> -		scanned = 0;
> +	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness,
> +				 &list, &isolated, &type, &type_scanned);
>   
>   	lruvec_unlock_irq(lruvec);
>   
> @@ -4861,7 +4861,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>   	sc->nr.unqueued_dirty += stat.nr_unqueued_dirty;
>   	sc->nr_reclaimed += reclaimed;
>   	trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id,
> -			scanned, reclaimed, &stat, sc->priority,
> +			type_scanned, reclaimed, &stat, sc->priority,
>   			type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
>   
>   	list_for_each_entry_safe_reverse(folio, next, &list, lru) {
>