Re: [PATCH v4 06/17] mm/mglru: move add/del LRU size accounting out of lru_gen_update_size()

[Barry Song]

On Mon, Nov 9, 2026 at 5:34 PM Kairui Song <ryncsn@xxxxxxxxx> wrote:
>
> On Mon, Nov 9, 2026 at 4:48 AM Barry Song <baohua@xxxxxxxxxx> wrote:
> >
> > On Fri, Oct 16, 2026 at 2:15 AM Kairui Song via B4 Relay
> > <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> > >
> > > From: Kairui Song <kasong@xxxxxxxxxxx>
> > >
> > > Pure code shuffle, no behavior change.  lru_gen_update_size() now only
> > > updates the per-generation counters plus the promotion move; the
> > > addition and deletion active/inactive accounting move to
> > > lru_gen_add_folio() and lru_gen_del_folio(), which know the generation
> > > and can apply the active-window test directly.
> > >
> > > The unevictable case is untouched: unevictable folios never enter
> > > lru_gen_add_folio() and keep being accounted by lruvec_add_folio() as
> > > before.
> > >
> > > Tested-by: zhaozhengzhuo <zhaozhengzhuo@xxxxxxxxxxxxx>
> > > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> > > ---
> >
> > The patch looks good to me.
> >
> > Meanwhile, I've been struggling with the names of
> > __update_lru_size() and lru_gen_update_size() for a long
> > time.
> >
> > Given that lru_gen_update_size() now only updates
> > nr_pages:
> > static inline void lru_gen_update_size(struct lruvec *lruvec, int type,
> >                 struct folio *folio, int old_gen, int new_gen)
> > {
> >        ...
> >
> >         if (old_gen >= 0)
> >                 atomic_long_sub(delta, &lrugen->nr_pages[old_gen][type][zone]);
> >         if (new_gen >= 0)
> >                 atomic_long_add(delta, &lrugen->nr_pages[new_gen][type][zone]);
> > }
> >
> > while __update_lru_size() only handles
> > active/inactive accounting, could we rename them to make
> > their roles clearer? For example:
> >
> > lru_gen_update_nr_pages()
> > lru_gen_update_active_inactive_size()
>
> Hmm but __update_lru_size is also used by CLRU, so it's not a
> "lru_gen" specific helper. If we can merge CLRU/MGLRU, then maybe we
> can rename them?

I did a quick search in mm/vmscan.c and found that
everything is MGLRU. But you're right, I missed
update_lru_size(), which is part of CLRU.

   1     62  include/linux/mm_inline.h <<update_lru_size>>
             __update_lru_size(lruvec, lru, zid, nr_pages);
   2    279  include/linux/mm_inline.h <<lru_gen_update_size>>
             __update_lru_size(lruvec, lru, zone, delta);
   3    287  include/linux/mm_inline.h <<lru_gen_update_size>>
             __update_lru_size(lruvec, lru, zone, -delta);
   4    293  include/linux/mm_inline.h <<lru_gen_update_size>>
             __update_lru_size(lruvec, lru, zone, -delta);
   5    294  include/linux/mm_inline.h <<lru_gen_update_size>>
             __update_lru_size(lruvec, lru + LRU_ACTIVE, zone, delta);
   6   3355  mm/vmscan.c <<reset_batch_size>>
             __update_lru_size(lruvec, lru, zone, delta);
   7   4101  mm/vmscan.c <<inc_max_seq>>
             __update_lru_size(lruvec, lru, zone, delta);
   8   4102  mm/vmscan.c <<inc_max_seq>>
             __update_lru_size(lruvec, lru + LRU_ACTIVE, zone, -delta);
   9   4632  mm/vmscan.c <<__lru_gen_reparent_memcg>>
             __update_lru_size(child_lruvec, lru + child_lru_active,
zone, -nr_pages);
  10   4633  mm/vmscan.c <<__lru_gen_reparent_memcg>>
             __update_lru_size(parent_lruvec, lru + parent_lru_active,
zone, nr_pages);

Thanks,
Barry