Re: [RFC PATCH v2 1/5] mm: mglru: avoid scanning empty generations in scan_folios()

[Barry Song]

On Thu, Jul 30, 2026 at 2:41 PM Lian Wang <lianux.mm@xxxxxxxxx> wrote:
>
> Hi Barry,
>
> I tested a list-based exhaustion check on top of the v2 series.
>
> Using remaining > 0 still misses the case where remaining reaches zero
> exactly when the last folio leaves the oldest generation. Since
> scan_folios() still holds the lruvec lock, checking the generation's zone
> lists directly avoids both this boundary and the scanned/nr_to_scan
> page-versus-folio mismatch.
>
> The change below only replaces the exhaustion check. It keeps the
> priority-gated fallback unchanged, and reports the MIN_NR_GENS early
> return as exhausted to preserve the current v2 behavior.
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6a73be7590cd..59b8725a0650 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -4635,7 +4635,8 @@ 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, int *isolatedp)
> +                      struct list_head *list, int *isolatedp,
> +                      bool *scan_exhausted)
>  {
>         int i;
>         int gen;
> @@ -4650,8 +4651,10 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>         VM_WARN_ON_ONCE(nr_to_scan > MAX_LRU_BATCH);
>         VM_WARN_ON_ONCE(!list_empty(list));
>
> -       if (get_nr_gens(lruvec, type) == MIN_NR_GENS)
> +       if (get_nr_gens(lruvec, type) == MIN_NR_GENS) {
> +               *scan_exhausted = true;
>                 return 0;
> +       }
>
>         gen = lru_gen_from_seq(lrugen->min_seq[type]);
>
> @@ -4704,6 +4707,14 @@ static int scan_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>                                 scanned, skipped, isolated,
>                                 type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
>
> +       *scan_exhausted = true;
> +       for (i = 0; i < MAX_NR_ZONES; i++) {
> +               if (!list_empty(&lrugen->folios[gen][type][i])) {
> +                       *scan_exhausted = false;
> +                       break;
> +               }
> +       }

Thanks very much. Your understanding is correct. We do have the
case where the last folio leaves the list while all the other
scan targets have already been satisfied. My understanding is
that this should be a very low-probability case, so it may not
be worth the added code complexity?

> +
>         *isolatedp = isolated;
>         return scanned;
>  }
> @@ -4757,11 +4768,13 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>         int type = get_type_to_scan(lruvec, swappiness);
>
>         for_each_evictable_type(i, swappiness) {
> +               bool scan_exhausted;
>                 int scanned;
>                 int tier = get_tier_idx(lruvec, type);
>
>                 scanned = scan_folios(nr_to_scan, lruvec, sc,
> -                                     type, tier, list, isolated);
> +                                     type, tier, list, isolated,
> +                                     &scan_exhausted);
>
>                 total_scanned += scanned;
>                 if (*isolated) {
> @@ -4770,15 +4783,12 @@ static int isolate_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
>                         break;
>                 }
>                 /*
> -                * If scanned >= nr_to_scan or isolated >= MIN_LRU_BATCH,
> -                * avoid falling back to the other type. The preferred
> -                * type is still reclaimable; otherwise, it would have
> -                * already run out of reclaimable generations. Falling
> -                * back too readily can disrupt the positive_ctrl_err()
> -                * bias. Also, only fall back when reclaim is running at
> -                * a high priority.
> +                * The preferred type is still reclaimable unless this scan
> +                * target is exhausted. Preserve the existing priority-gated
> +                * fallback without inferring exhaustion from scanned, which
> +                * counts base pages rather than iterations.
>                  */
> -               if (scanned < nr_to_scan && *isolated < MIN_LRU_BATCH) {
> +               if (scan_exhausted) {
>                         if (sc->priority > 2)
>                                 break;
>                         type = !type;
>
> I built and booted this on an arm64 VM with MGLRU enabled. A fresh
> swappiness=200 pressure smoke with a 2 GiB file-cache setup and about
> 7 GiB of touched anonymous memory completed without a
> WARN/Oops/BUG/panic.

Thanks! I'm going to send out RFC v3. I'd really appreciate any
further testing you can do.

>
> Would this be worth considering for the next revision? If I misunderstood
> the intended semantics, please ignore this.

Thanks! Your understanding of the corner case is 100% correct.
It's just that, given how unlikely it is to occur, I don't
think it would have any noticeable impact, so it may not be
worth the added complexity.

Best Regards
Barry