Re: [RFC PATCH v5 05/14] mm/mglru: skip zones above reclaim_idx instead of rotating them

[Barry Song <baohua@kernel.org>]

On Mon, Nov 2, 2026 at 6:00 PM KunWu Chan <kunwu.chan@gmail.com> wrote:
>
> On Sun, Oct 25, 2026 at 12:19 AM Kairui Song <ryncsn@gmail.com> wrote:
> >
> > On Sat, Oct 24, 2026 at 6:03 AM <sashiko-bot@kernel.org> wrote:
> > >
> > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > > - [High] mm/mglru: pinned min_seq in higher zones stalls aging in scan_folios()
> > > --
> > >
> > > commit 1b2f683d6b8bd1e171eb9677315ba8a3d9431b9b
> > > Author: Kairui Song <kasong@tencent.com>
> > >
> > > mm/mglru: skip zones above reclaim_idx instead of rotating them
> > >
> > > This commit restricts scan_folios() and lruvec_populated_min_seq() during
> > > eviction to zones at or below sc->reclaim_idx, and removes the ineligible-zone
> > > rotation branch from sort_folio(). try_to_inc_min_seq() continues to check all
> > > zones up to MAX_NR_ZONES before advancing min_seq.
> > >
> > > > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > > > index a365a4bc8c988..09ea72b354a56 100644
> > > > --- a/mm/vmscan.c
> > > > +++ b/mm/vmscan.c
> > > > @@ -4132,7 +4132,7 @@ static bool inc_min_seq(struct lruvec *lruvec, int type, int swappiness)
> > > >
> > > >  /* 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 zone_limit, unsigned long max_seq)
> > > >  {
> > > >       int gen, zone;
> > > >       unsigned long min_seq;
> > > > @@ -4143,7 +4143,7 @@ static unsigned long lruvec_populated_min_seq(struct lruvec *lruvec, int type,
> > > >       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++) {
> > > > +             for (zone = 0; zone < zone_limit; zone++) {
> > > >                       if (!list_empty(&lrugen->folios[gen][type][zone]))
> > > >                               return min_seq;
> > > >               }
> > > > @@ -4166,7 +4166,7 @@ static void try_to_inc_min_seq(struct lruvec *lruvec, int swappiness)
> > > >       /* Requires at least MIN_NR_GENS gens for basic functionality */
> > > >       seq = lrugen->max_seq - MIN_NR_GENS;
> > > >       for_each_evictable_type(type, swappiness) {
> > > > -             min_seq[type] = lruvec_populated_min_seq(lruvec, type, seq + 1);
> > > > +             min_seq[type] = lruvec_populated_min_seq(lruvec, type, MAX_NR_ZONES, seq + 1);
> > > >               if (min_seq[type] > lrugen->min_seq[type])
> > > >                       seq_inc_flag = true;
> > > >       }
> > >
> > > [ ... ]
> > >
> > > > @@ -4863,10 +4864,8 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> > > >        * longer be promoted or protected, so just isolate them. The one
> > > >        * exception is lru_gen_reparent_memcg(), which is handled above.
> > > >        */
> > > > -     if (min_gen == max_gen) {
> > > > -             VM_WARN_ON_ONCE(zone > sc->reclaim_idx);
> > > > +     if (min_gen == max_gen)
> > > >               return false;
> > > > -     }
> > > >
> > > >       /* protected */
> > > >       if (tier > tier_idx || refs + workingset == BIT(LRU_REFS_WIDTH) + 1) {
> > > > @@ -4883,13 +4882,6 @@ static bool sort_folio(struct lruvec *lruvec, struct folio *folio, struct scan_c
> > > >               return true;
> > > >       }
> > > >
> > > > -     /* ineligible */
> > > > -     if (zone > sc->reclaim_idx) {
> > > > -             gen = folio_inc_gen(lruvec, folio, min_gen);
> > > > -             list_move_tail(&folio->lru, &lrugen->folios[gen][type][zone]);
> > > > -             return true;
> > > > -     }
> > > > -
> > > >       return false;
> > > >  }
> > > >
> > > > @@ -4921,16 +4913,17 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> > > >                      struct scan_control *sc, int type, int tier,
> > > >                      struct list_head *list, int *isolatedp)
> > > >  {
> > > > -     int i;
> > > >       enum node_stat_item item;
> > > > +     int zone_idx;
> > > >       int sorted = 0;
> > > >       int scanned = 0;
> > > >       int isolated = 0;
> > > >       int skipped = 0;
> > > >       int gen, max_gen;
> > > > -     unsigned long min_seq;
> > > > +     unsigned long seq;
> > > >       unsigned long remaining = nr_to_scan;
> > > >       struct lru_gen_folio *lrugen = &lruvec->lrugen;
> > > > +     struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> > > >       DEFINE_MAX_SEQ(lruvec);
> > > >
> > > >       VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
> > > > @@ -4942,24 +4935,22 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> > > >        * when under pressure to respect swappiness more and reduce IO cost.
> > > >        */
> > > >       if (sc->priority == DEF_PRIORITY)
> > > > -             min_seq = lrugen->min_seq[type];
> > > > +             seq = lrugen->min_seq[type];
> > > >       else
> > > > -             min_seq = lruvec_populated_min_seq(lruvec, type, lrugen->max_seq);
> > > > +             seq = lrugen->max_seq;
> > > >
> > > > -     gen = lru_gen_from_seq(min_seq);
> > > > +     seq = lruvec_populated_min_seq(lruvec, type, sc->reclaim_idx + 1, seq);
> > > > +     gen = lru_gen_from_seq(seq);
> > > >       max_gen = lru_gen_from_seq(max_seq);
> > > >
> > > > -     for (i = MAX_NR_ZONES; i > 0; i--) {
> > > > +     /* Scan eligible zones in descending order. */
> > > > +     for (zone_idx = sc->reclaim_idx; zone_idx >= 0; zone_idx--) {
> > >
> > > [Severity: High]
> > > During zone-constrained reclaim in mm/vmscan.c (where sc->reclaim_idx is below
> > > MAX_NR_ZONES - 1 and zones above sc->reclaim_idx are populated, such as
> > > GFP_DMA32 reclaim with a populated ZONE_NORMAL or GFP_KERNEL reclaim with a
> > > populated ZONE_MOVABLE), can keeping MAX_NR_ZONES in try_to_inc_min_seq()
> > > while removing the ineligible-zone rotation from sort_folio() stall aging and
> > > memcg rotation?
> >
> > Yeah it can stall aging, but this series lifted the restriction of
> > MIN_NR_GEN on eviction, and eviction will start the the last populated
> > gen of a zone instead of min_seq, and we have more proactive aging
> > when the gens are imbalanced, so this won't be a problem, it won't
> > cause OOM. I'm not sure if it will cause any performance issue though,
> > if this is concerning we can keep the old behavior, or trigger aging
> > when the populated gen that satisfy a reclain is below (MIN_NR_GEN -
> > 1) or imbalanced, this seems a better solution to me, still avoid a
> > lot of redundant work while ensure there is no reclaim effenciency
> > issue. We already have the right helper to do so in this series, so
> > seem not hard or dirty.
>
> Thanks, that makes sense regarding OOM.
> Could lru_gen_imbalanced() trigger aging soon enough when few
> populated generations are eligible for reclaim, or could reclaim
> efficiency suffer while min_seq remains pinned?
>
> Would the additional trigger you suggested for this case be worthwhile?

Hi Kunwu,

Thanks very much for reviewing this series.
I'd really appreciate it if you could keep going :-)

I'm trying an incremental patch like this while rebasing the
series on top of mm-new. It may resolve the problem.

diff --git a/mm/trace_vmscan.h b/mm/trace_vmscan.h
index 565c56428364..9dafdb5bd3ce 100644
--- a/mm/trace_vmscan.h
+++ b/mm/trace_vmscan.h
@@ -488,9 +488,9 @@ TRACE_EVENT(mm_mglru_aging,
 			unsigned long seq = max_seq - MAX_NR_GENS + 1 + i;
 
 			__entry->nr_anon[i] =
-				lruvec_gen_size(lrugen, LRU_GEN_ANON, seq);
+				lruvec_gen_size(lrugen, LRU_GEN_ANON, seq, MAX_NR_ZONES);
 			__entry->nr_file[i] =
-				lruvec_gen_size(lrugen, LRU_GEN_FILE, seq);
+				lruvec_gen_size(lrugen, LRU_GEN_FILE, seq, MAX_NR_ZONES);
 		}
 		__entry->nid = lruvec_pgdat(lruvec)->node_id;
 		__entry->priority = sc->priority;
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4ca04812eea3..fa0245a07caa 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4217,7 +4217,7 @@ static void lruvec_evictable_size(struct lruvec *lruvec, struct scan_control *sc
 		if (type == LRU_GEN_ANON && !can_reclaim_anon_pages(memcg, nid, sc))
 			continue;
 		for (seq = min_seq[type]; seq <= max_seq; seq++)
-			nr[type] += lruvec_gen_size(lrugen, type, seq);
+			nr[type] += lruvec_gen_size(lrugen, type, seq, sc->reclaim_idx + 1);
 	}
 }
 
@@ -5082,9 +5082,9 @@ static bool lru_gen_imbalanced(struct lruvec *lruvec, struct scan_control *sc,
 	for_each_evictable_type(type, swappiness) {
 		unsigned long seq;
 
-		youngest[type] = lruvec_gen_size(lrugen, type, max_seq);
+		youngest[type] = lruvec_gen_size(lrugen, type, max_seq, sc->reclaim_idx + 1);
 		for (seq = min_seq[type]; seq < max_seq; seq++)
-			old[type] += lruvec_gen_size(lrugen, type, seq);
+			old[type] += lruvec_gen_size(lrugen, type, seq, sc->reclaim_idx + 1);
 	}
 
 	/* the vast majority of pages sit in the youngest gen */
@@ -5835,7 +5835,7 @@ static int lru_gen_seq_show(struct seq_file *m, void *v)
 			char mark = full && seq < min_seq[type] ? 'x' : ' ';
 
 			seq_printf(m, " %10lu%c",
-				   lruvec_gen_size(lrugen, type, seq), mark);
+				   lruvec_gen_size(lrugen, type, seq, MAX_NR_ZONES), mark);
 		}
 
 		seq_putc(m, '\n');
diff --git a/mm/vmscan.h b/mm/vmscan.h
index c6b583fc7493..2599d7c614ac 100644
--- a/mm/vmscan.h
+++ b/mm/vmscan.h
@@ -119,12 +119,13 @@ struct scan_control {
 #ifdef CONFIG_LRU_GEN
 /* The number of pages of @type in the generation @seq, summed over all zones */
 static inline unsigned long lruvec_gen_size(struct lru_gen_folio *lrugen,
-					    int type, unsigned long seq)
+					    int type, unsigned long seq,
+					    int max_zone)
 {
 	int gen = lru_gen_from_seq(seq);
 	unsigned long size = 0;
 
-	for (int zone = 0; zone < MAX_NR_ZONES; zone++)
+	for (int zone = 0; zone < max_zone; zone++)
 		size += max(READ_ONCE(lrugen->nr_pages[gen][type][zone]), 0L);
 
 	return size;

>
> Thanks,
> Kunwu

Thanks,
Barry