From: Kairui Song <kasong@tencent.com>

Merge commonly used code for counting evictable folios in a lruvec.

No behavior change.

Return unsigned long instead of long as suggested [ Axel Rasmussen ]

Acked-by: Yuanchu Xie <yuanchu@google.com>
Reviewed-by: Barry Song <baohua@kernel.org>
Reviewed-by: Chen Ridong <chenridong@huaweicloud.com>
Reviewed-by: Axel Rasmussen <axelrasmussen@google.com>
Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
Signed-off-by: Kairui Song <kasong@tencent.com>
---
 mm/vmscan.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 5a8c8fcccbfc..adc07501a137 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4084,27 +4084,33 @@ static void set_initial_priority(struct pglist_data *pgdat, struct scan_control
 	sc->priority = clamp(priority, DEF_PRIORITY / 2, DEF_PRIORITY);
 }
 
-static bool lruvec_is_sizable(struct lruvec *lruvec, struct scan_control *sc)
+static unsigned long lruvec_evictable_size(struct lruvec *lruvec, int swappiness)
 {
 	int gen, type, zone;
-	unsigned long total = 0;
-	int swappiness = get_swappiness(lruvec, sc);
+	unsigned long seq, total = 0;
 	struct lru_gen_folio *lrugen = &lruvec->lrugen;
-	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
 	DEFINE_MAX_SEQ(lruvec);
 	DEFINE_MIN_SEQ(lruvec);
 
 	for_each_evictable_type(type, swappiness) {
-		unsigned long seq;
-
 		for (seq = min_seq[type]; seq <= max_seq; seq++) {
 			gen = lru_gen_from_seq(seq);
-
 			for (zone = 0; zone < MAX_NR_ZONES; zone++)
 				total += max(READ_ONCE(lrugen->nr_pages[gen][type][zone]), 0L);
 		}
 	}
 
+	return total;
+}
+
+static bool lruvec_is_sizable(struct lruvec *lruvec, struct scan_control *sc)
+{
+	unsigned long total;
+	int swappiness = get_swappiness(lruvec, sc);
+	struct mem_cgroup *memcg = lruvec_memcg(lruvec);
+
+	total = lruvec_evictable_size(lruvec, swappiness);
+
 	/* whether the size is big enough to be helpful */
 	return mem_cgroup_online(memcg) ? (total >> sc->priority) : total;
 }
@@ -4909,9 +4915,6 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
 static bool should_run_aging(struct lruvec *lruvec, unsigned long max_seq,
 			     int swappiness, unsigned long *nr_to_scan)
 {
-	int gen, type, zone;
-	unsigned long size = 0;
-	struct lru_gen_folio *lrugen = &lruvec->lrugen;
 	DEFINE_MIN_SEQ(lruvec);
 
 	*nr_to_scan = 0;
@@ -4919,18 +4922,7 @@ 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;
 
-	for_each_evictable_type(type, swappiness) {
-		unsigned long seq;
-
-		for (seq = min_seq[type]; seq <= max_seq; seq++) {
-			gen = lru_gen_from_seq(seq);
-
-			for (zone = 0; zone < MAX_NR_ZONES; zone++)
-				size += max(READ_ONCE(lrugen->nr_pages[gen][type][zone]), 0L);
-		}
-	}
-
-	*nr_to_scan = size;
+	*nr_to_scan = lruvec_evictable_size(lruvec, swappiness);
 	/* better to run aging even though eviction is still possible */
 	return evictable_min_seq(min_seq, swappiness) + MIN_NR_GENS == max_seq;
 }

-- 
2.53.0

Forwarding sashiko review in case anyone has concern about it:

https://sashiko.dev/#/patchset/20260403-mglru-reclaim-v3-0-a285efd6ff91%40tencent.com?part=1

> commit e0119c3e147f68096f59fb96e1e69e83ecdfce40
> Author: Kairui Song <kasong@tencent.com>
> mm/mglru: consolidate common code for retrieving evictable size
> This commit merges commonly used code for counting evictable folios in a
> lruvec into a new helper function lruvec_evictable_size.
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 5a8c8fcccbfc9..adc07501a1375 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> [ ... ]
> > @@ -4909,9 +4915,6 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >  static bool should_run_aging(struct lruvec *lruvec, unsigned long max_seq,
> >  			     int swappiness, unsigned long *nr_to_scan)
> >  {
> > -	int gen, type, zone;
> > -	unsigned long size = 0;
> > -	struct lru_gen_folio *lrugen = &lruvec->lrugen;
> >  	DEFINE_MIN_SEQ(lruvec);
> >  
> >  	*nr_to_scan = 0;
> > @@ -4919,18 +4922,7 @@ 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;
> >  
> > -	for_each_evictable_type(type, swappiness) {
> > -		unsigned long seq;
> > -
> > -		for (seq = min_seq[type]; seq <= max_seq; seq++) {
> > -			gen = lru_gen_from_seq(seq);
> > -
> > -			for (zone = 0; zone < MAX_NR_ZONES; zone++)
> > -				size += max(READ_ONCE(lrugen->nr_pages[gen][type][zone]), 0L);
> > -		}
> > -	}
> > -
> > -	*nr_to_scan = size;
> > +	*nr_to_scan = lruvec_evictable_size(lruvec, swappiness);
> The commit message notes there is no behavior change, but does delegating
> to lruvec_evictable_size() introduce a subtle state inconsistency?
> Previously, should_run_aging() evaluated the aging conditions and iterated
> the generations to calculate the evictable size using a single snapshot of
> max_seq (passed as an argument) and min_seq (read locally via
> DEFINE_MIN_SEQ).
> The new lruvec_evictable_size() helper re-reads both sequence numbers
> internally via its own DEFINE_MAX_SEQ and DEFINE_MIN_SEQ macros. If the
> sequence numbers advance concurrently, the calculated size might be based
> on a different snapshot than the one used just lines above to determine if
> aging should run.
> Could this lead to unintended behavioral changes during concurrent
> execution?

This is totally fine.

Right lruvec_evictable_size re-reads max_seq - and that's really trivial,
I don't think anyone will observe any different in any way. The time
windows is so tiny and even if a different value is read, the updated
value is more accurate for getting the LRU size.