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

[Chen Ridong <chenridong@huaweicloud.com>]

On 2026/4/7 19:57, 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 and consume the
> budget 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.
> 
> - `scanned += try_to_inc_min_seq` in evict_folios: adding a bool as a
>   scan count was kind of confusing and no longer needed to, as scan
>   number should never be zero as long as there are still evictable
>   gens. We may encounter a empty old gen that return 0 scan count,
>   to avoid that, do a try_to_inc_min_seq before isolation which
>   have slight to none overhead in most cases.
> 
> - `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 change try_to_inc_min_seq to return void, as its return value is
> no longer used by any caller. Move the call before isolate_folios so
> that any empty gens created by external folio freeing are flushed, and
> add another call after isolate_folios to also flush empty gens that
> isolation itself may create.
> 
> 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.
> 
> Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
> Signed-off-by: Kairui Song <kasong@tencent.com>
> ---
>  mm/vmscan.c | 60 ++++++++++++++++++++++++++++++------------------------------
>  1 file changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 462ca0fa2ba3..d3fd5bb56cce 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3878,10 +3878,9 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
>  	return true;
>  }
>  
> -static bool try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
> +static void try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
>  {
>  	int gen, type, zone;
> -	bool success = false;
>  	bool seq_inc_flag = false;
>  	struct lru_gen_folio *lrugen = &lruvec->lrugen;
>  	DEFINE_MIN_SEQ(lruvec);
> @@ -3907,11 +3906,10 @@ static bool try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
>  
>  	/*
>  	 * If min_seq[type] of both anonymous and file is not increased,
> -	 * we can directly return false to avoid unnecessary checking
> -	 * overhead later.
> +	 * return here to avoid unnecessary checking overhead later.
>  	 */
>  	if (!seq_inc_flag)
> -		return success;
> +		return;
>  
>  	/* see the comment on lru_gen_folio */
>  	if (swappiness && swappiness <= MAX_SWAPPINESS) {
> @@ -3929,10 +3927,7 @@ static bool try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
>  
>  		reset_ctrl_pos(lruvec, type, true);
>  		WRITE_ONCE(lrugen->min_seq[type], min_seq[type]);
> -		success = true;
>  	}
> -
> -	return success;
>  }
>  
>  static bool inc_max_seq(struct lruvec *lruvec, unsigned long seq, int swappiness)
> @@ -4686,7 +4681,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 +4751,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 +4797,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)

Nit: too many parameters for this function.


>  {
>  	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 +4834,23 @@ 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);
> +	/* In case folio deletion left empty old gens, flush them */
> +	try_to_inc_min_seq(lruvec, swappiness);
>  
> -	scanned += try_to_inc_min_seq(lruvec, swappiness);
> +	scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness,
> +				 &list, &isolated, &type, &type_scanned);
>  
> -	if (evictable_min_seq(lrugen->min_seq, swappiness) + MIN_NR_GENS > lrugen->max_seq)
> -		scanned = 0;
> +	/* Isolation might create empty gen, flush them */
> +	if (scanned)
> +		try_to_inc_min_seq(lruvec, swappiness);
>  
>  	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) {
> 

This patch makes the code easier to follow. Looks good to me.

Reviewed-by: Chen Ridong <chenridong@huaweicloud.com>

-- 
Best regards,
Ridong