Re: [PATCH 4/8] mm/mglru: scan and count the exact number of folios

[Kairui Song]

On Tue, Mar 24, 2026 at 3:22 PM Chen Ridong <chenridong@xxxxxxxxxxxxxxx> wrote:
> On 2026/3/23 0:20, Kairui Song wrote:
> > On Sat, Mar 21, 2026 at 4:59 AM Axel Rasmussen <axelrasmussen@xxxxxxxxxx> wrote:
> >>
> >> On Tue, Mar 17, 2026 at 12:11 PM Kairui Song via B4 Relay
> >> <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> >>>
> >>> From: Kairui Song <kasong@xxxxxxxxxxx>
> >>>
> >>> Make the scan helpers return the exact number of folios being scanned
> >>> or isolated. This should make the scan more accurate and easier to
> >>> follow.
> >>>
> >>> Now there is no more need for special handling when there is no
> >>> progress made. The old livelock prevention `(return isolated ||
> >>> !remaining ? scanned : 0)` is replaced by the natural scan budget
> >>> exhaustion in try_to_shrink_lruvec, and sort_folio moves ineligible
> >>> folios to newer generations.
> >>>
> >>> Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>

...

> >>>  static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >>> @@ -4852,7 +4851,6 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >>>         struct reclaim_stat stat;
> >>>         struct lru_gen_mm_walk *walk;
> >>>         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);
> >>>
> >>> @@ -4860,10 +4858,7 @@ static int evict_folios(unsigned long nr_to_scan, struct lruvec *lruvec,
> >>>
> >>>         scanned = isolate_folios(nr_to_scan, lruvec, sc, swappiness, &type, &list);
> >>>
> >>> -       scanned += try_to_inc_min_seq(lruvec, swappiness);
> >>> -
> >>> -       if (evictable_min_seq(lrugen->min_seq, swappiness) + MIN_NR_GENS > lrugen->max_seq)
> >>> -               scanned = 0;
> >>> +       try_to_inc_min_seq(lruvec, swappiness);
> >>
> >> IIUC, this change is what introduces the issue patch 6 is trying to
> >> resolve. Is it worth squashing patch 6 in to this one, so we don't
> >> have this non-ideal intermediate state?
> >
> > Well it's not, patch 6 is fixing an existing problem, see the cover
> > letter about the OOM issue.
> >
> > This part of changing is just cleanup the loop code. It looks really
> > strange to me that increasing min_seq is considered as scanning one
> > folio. Aborting the scan if there is only 2 gen kind of make sense but
> > this doesn't seems the right place. These strange parts to avoid
> > livelock can be dropped since we have an exact count of folios being
> > scanned now. I'll add more words in the commit message.
>
> This change confused me too.
>
> IIUC, this change looks conceptually tied to patch 3. The following change means
> that evict_folios should not be invoked if aging is needed. So the judge can be
> dropped there, right?
>
>
> ```
>  static bool try_to_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
>  {
> ...
> +               if (should_run_aging(lruvec, max_seq, sc, swappiness)) {
> +                       if (try_to_inc_max_seq(lruvec, max_seq, swappiness, false))
> +                               need_rotate = true;
> +                       break;
> +               }
> ```
>

Hi Ridong,

Ahh yes, as you pointed out, the explicit should_run_aging kind of
guards the evict_folio. That's not everything, besides, previously
isolate_folios may return 0 if there is no folio isolated. But now it
always return the number of folios being scanned, unless there are
only two genes left and hit the force protection, which also makes the
judge here can be dropped.

But not invoking evict_folios if aging is needed is an existing
behavior, that commit (patch 3) didn't change it, just made it cleaner
so we can see it well.

Now the folio scan number combines well with the scan budget
introduced in the previous commit.

And I just noticed it might be even better to move try_to_inc_min_seq
before isolate_folios, to avoid an empty gen blocking isolate_folios.
Usually this won't be an issue since calling try_to_inc_min_seq after
isolate_folios also ensures reclaim won't generate any problematic
empty gen, but removing folio by things like freeing could introduce
one or two empty gens.

The forced gen protection may cause other problems but that's
irrelevant to this commit, should be improved later.