Re: [PATCH v2 2/3] mm/mglru: add tracepoint for scan_folios()

[Ridong Chen]

On 9/14/2026 3:47 PM, Baolin Wang wrote:
> On 9/11/26 6:29 PM, Ridong Chen wrote:
> > From: Ridong Chen <chenridong@xxxxxxxxxx>
> >
> > MGLRU's scan_folios() emits the classic-LRU tracepoint
> > trace_mm_vmscan_lru_isolate(), which predates MGLRU. It reports the
> > scan/isolate counts and the LRU type, but carries no generation or
> > memcg context, so a trace of an MGLRU run cannot tell which memcg a
> > given scan belongs to, nor how far reclaim has progressed through the
> > generations.
> >
> > Add mm_mglru_scan_folios next to it, reporting the same counters plus
> > nr_sorted (folios moved to a younger generation by sort_folio()) and
> > the MGLRU context the classic tracepoint lacks: the memcg id, and the
> > max_seq, tier and min_seq of the type being scanned.
> >
> > scan_folios() is the MGLRU-specific layer where folios are actually
> > scanned, so it has no classic-LRU counterpart. That the classic
> > trace_mm_vmscan_lru_isolate() has lived here stably shows this is a
> > long-lived place to hook, and the new tracepoint can be enabled on its
> > own to observe the MGLRU-specific information. The existing tracepoint
> > is left unchanged.
> >
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Ridong Chen <chenridong@xxxxxxxxxx>
> > ---
> >   include/trace/events/vmscan.h | 63 +++++++++++++++++++++++++++++++++++
> >   mm/vmscan.c                   |  6 ++++
> >   2 files changed, 69 insertions(+)
> >
> > diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
> > index 8a872990b4be..5defa8f6719c 100644
> > --- a/include/trace/events/vmscan.h
> > +++ b/include/trace/events/vmscan.h
> > @@ -392,6 +392,69 @@ TRACE_EVENT(mm_vmscan_lru_isolate,
> >           __print_symbolic(__entry->lru, LRU_NAMES))
> >   );
>
> [snip]
>
> > diff --git a/mm/vmscan.c b/mm/vmscan.c
> > index 2554a6513aa8..67f59aa73fb9 100644
> > --- a/mm/vmscan.c
> > +++ b/mm/vmscan.c
> > @@ -4876,6 +4876,12 @@ static int scan_folios(unsigned long nr_to_scan, struct 
> > lruvec *lruvec,
> >       trace_mm_vmscan_lru_isolate(sc->reclaim_idx, sc->order, nr_to_scan,
> >                   scanned, skipped, isolated,
> >                   type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON);
> > +    trace_mm_mglru_scan_folios(lruvec,
> > +                   sc->reclaim_idx, sc->order, nr_to_scan,
> > +                   scanned, sorted, skipped, isolated,
> > +                   type ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON,
> > +                   lrugen->max_seq, tier,
> > +                   lrugen->min_seq[type]);
>
> Both tracepoints will print some duplicated content, and I'm not sure it's worth 
> a new tracepoint just to trace max_seq and min_seq.
>
> Anyway, I'm not against this patch, but I'd like to hear others' opinions.

Thank you for your review.

Since the lruvec is per memcg and per node, we would like to know how the memcg 
scans and reclaims its lruvec.

I am also concerned that the content may be duplicated where we place it here. 
However, with Barry's suggestion, it is stable here. What is more, the trace is 
under the control of the trace event gate. It is convenient to control and would 
not lead to any regression when they are disabled.

As my cover letter mentions, with the current trace events, we don't know when 
the per-memcg lruvec ages, when it is scanned and reclaimed, or with what 
seq(gen). With this series, we can tell the memcg aging and reclaim actions.

--
Best regards
Ridong