Re: [PATCH v5 6/6] mm/mglru: add tracepoint for inc_max_seq()

[Baoquan He <baoquan.he@linux.dev>]

On 11/01/26 at 05:21am, Barry Song wrote:
> On Sat, Oct 31, 2026 at 5:02 PM Ridong Chen <ridong.chen@linux.dev> wrote:
> >
> > From: Ridong Chen <chenridong@xiaomi.com>
> >
> > Aging in MGLRU advances max_seq via inc_max_seq(), creating a new
> > youngest generation. There is currently no tracepoint on this path, so
> > the moment a new generation is created, and how the min_seq of each type
> > trails behind it, cannot be observed as it happens.
> >
> > Add mm_mglru_inc_max_seq, emitted right after max_seq is bumped while
> > the lruvec lock is still held, with the node id, memcg id, the new
> > max_seq, the anon and file min_seq, and the number of pages in each
> > generation for both types, summed over zones and clamped to >= 0 the
> > same way the debugfs lru_gen file reports them. The nr_anon/nr_file
> > arrays are indexed by generation slot, so the emitted max_seq/min_seq
> > say which slot holds which seq. They are printed with __print_array(),
> > so the output is hex and its length follows MAX_NR_GENS automatically.
> >
> > Only the lruvec is passed in; all of the above is collected in
> > TP_fast_assign(), so inc_max_seq() does no extra work when the event is
> > disabled. Paired with mm_mglru_scan_folios it makes the full
> > aging-to-eviction window observable per memcg.
> >
> > Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
> > ---
> >  mm/trace_vmscan.h | 47 +++++++++++++++++++++++++++++++++++++++++++++++
> >  mm/vmscan.c       |  2 ++
> >  2 files changed, 49 insertions(+)
> >
> > diff --git a/mm/trace_vmscan.h b/mm/trace_vmscan.h
> > index 8148b8224422..cddf42b5849a 100644
> > --- a/mm/trace_vmscan.h
> > +++ b/mm/trace_vmscan.h
> > @@ -456,6 +456,53 @@ TRACE_EVENT(mm_mglru_scan_folios,
> >                 __entry->min_seq)
> >  );
> >
> > +TRACE_EVENT(mm_mglru_inc_max_seq,
> > +
> > +       TP_PROTO(struct lruvec *lruvec),
> > +
> > +       TP_ARGS(lruvec),
> > +
> > +       TP_STRUCT__entry(
> > +               __field(u64, memcg_id)
> > +               __field(unsigned long, max_seq)
> > +               __field(unsigned long, anon_min_seq)
> > +               __field(unsigned long, file_min_seq)
> > +               __array(unsigned long, nr_anon, MAX_NR_GENS)
> > +               __array(unsigned long, nr_file, MAX_NR_GENS)
> > +               __field(int, nid)
> > +       ),
> > +
> > +       TP_fast_assign(
> > +               struct lru_gen_folio *lrugen = &lruvec->lrugen;
> > +               int gen, zone;
> > +
> > +               __entry->memcg_id = mem_cgroup_id(lruvec_memcg(lruvec));
> > +               __entry->max_seq = lrugen->max_seq;
> > +               __entry->anon_min_seq = lrugen->min_seq[LRU_GEN_ANON];
> > +               __entry->file_min_seq = lrugen->min_seq[LRU_GEN_FILE];
> > +               /* summed over zones and clamped to >= 0, as debugfs lru_gen does */
> > +               for (gen = 0; gen < MAX_NR_GENS; gen++) {
> > +                       __entry->nr_anon[gen] = 0;
> > +                       __entry->nr_file[gen] = 0;
> > +                       for (zone = 0; zone < MAX_NR_ZONES; zone++) {
> > +                               __entry->nr_anon[gen] +=
> > +                                       max(lrugen->nr_pages[gen][LRU_GEN_ANON][zone], 0L);
> > +                               __entry->nr_file[gen] +=
> > +                                       max(lrugen->nr_pages[gen][LRU_GEN_FILE][zone], 0L);
> > +                       }
> > +               }
> 
> I feel this is wrong. Can we calculate the newest and
> oldest generations from seq, then print them in order
> from newest to oldest?

Agree, from newest to oldest or oldest to newest makes it clearer.

Printing from lrugen->max_seq - MAX_NR_GENS + 1, or from MAX_NR_GENS,
both looks good.

> 
> The nr_pages[] array isn't ordered from newest to
> oldest or vice versa, as we have:
> 
> static inline int lru_gen_from_seq(unsigned long seq)
> {
>         return seq % MAX_NR_GENS;
> }
> 
> And I added a helper here:
> https://lore.kernel.org/linux-mm/20261022220203.91911-11-baohua@kernel.org/
> 
> +static inline unsigned long lruvec_gen_size(struct lru_gen_folio *lrugen,
> + int type, unsigned long seq)
> +{
> +     int gen = lru_gen_from_seq(seq);
> +     unsigned long size = 0;
> +
> +     for (int zone = 0; zone < MAX_NR_ZONES; zone++)
> +          size += max(READ_ONCE(lrugen->nr_pages[gen][type][zone]), 0L);
> +     return size;
> +}
> 
> Maybe you could port it here as a separate patch so it
> can land earlier as part of your tracepoints series. We
> could then use it in the next version of the swappiness
> series.
> 
> Best,
> Barry