Re: [PATCH 1/6] mm/mglru: batch update lrugen->nr_pages in inc_min_seq()

[Barry Song]

On Thu, Aug 27, 2026 at 11:21 AM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> On Fri, Aug 21, 2026 at 7:09 PM Barry Song (Xiaomi) <baohua@xxxxxxxxxx> wrote:
> >
> > Currently, folio_inc_gen() updates lrugen->nr_pages for every folio
> > as it advances generations. Instead, accumulate the size changes
> > and update lrugen->nr_pages in a batch after scanning the entire
> > oldest generation, or when the scan stops because remaining reaches
> > zero.
> >
> > Since we only move folios from the oldest generation to the second
> > oldest generation, the active/inactive state cannot change. We can
> > therefore skip __lru_update_size().
> >
> > Signed-off-by: Barry Song (Xiaomi) <baohua@xxxxxxxxxx>
> > ---
> >  mm/vmscan.c | 46 +++++++++++++++++++++++++++++++++++-----------
> >  1 file changed, 35 insertions(+), 11 deletions(-)
>
> Hello Barry
>
> Thanks for the patch!
>
> >
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index c1404a59523d..0d74fc00abd3 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -3296,20 +3296,21 @@ static int folio_update_gen(struct folio *folio, int gen, const vma_flags_t *vma
> >  }
> >
> >  /* protect pages accessed multiple times through file descriptors */
> > -static int folio_inc_gen(struct lruvec *lruvec, struct folio *folio)
> > +static int __folio_inc_gen(struct folio *folio, int old_gen, bool *increased)
>
> I feel the naming is a bit confusing, the __ prefix doesn't tell how
> it differs from folio_inc_gen very well, maybe just name one
> folio_inc_gen (the old gen could be any gen), another one is
> folio_inc_min_gen (the old gen can only be min_seq), and with sanity
> check in folio_inc_min_gen that expects get_nr_gens == 4, and
> lru_gen_is_active(min_seq) == lru_gen_is_active(min_seq + 1)? This
> could be a build-time sanity check instead of a runtime debug check.

Thanks very much for your suggestion, Kairui.

I tried splitting this into two functions, folio_inc_min_gen() and
folio_inc_gen(). I tried a couple of approaches, but I think I’ll give up
on this direction.
folio_inc_min_gen() and folio_inc_gen() share some common code while
also having some differences. It’s hard to avoid ugly code duplication
because folio_inc_gen() cannot call folio_inc_min_gen(). However,
folio_inc_gen() can call __folio_inc_gen().

The __ prefix currently indicates that we don’t update the LRU sizes in
this case. The sizes can either be updated in a batch later or updated once
by folio_inc_gen().
Maybe we could add a comment to make this easier to read, or use a better
name for __folio_inc_gen()?

>
> I saw baoquan also mentioned the debug check on
> lru_gen_is_active(min_seq) == lru_gen_is_active(min_seq + 1), which I
> agree.
>

Agreed. But I feel we only need to do this once in inc_min_seq(),
rather than for each folio.

Best Regards
Barry