On Wed, Sep 9, 2026 at 8:00 PM Ridong Chen <[email protected]> wrote: > > > > On 9/9/2026 2:40 PM, Barry Song wrote: > > On Mon, Sep 7, 2026 at 12:25 PM Ridong Chen <[email protected]> wrote: > >> > >> From: Ridong Chen <[email protected]> > >> > >> MGLRU's scan_folios() and evict_folios() emit the classic-LRU > >> tracepoints trace_mm_vmscan_lru_isolate() and > >> trace_mm_vmscan_lru_shrink_inactive(). Those predate MGLRU and are > >> indistinguishable from the classic-LRU path: they carry no generation, > >> sequence, memcg or swappiness context, so a trace of an MGLRU run cannot > >> tell which memcg a given scan/evict belongs to, nor how far reclaim has > >> progressed through the generations. > >> > >> Add mm_mglru_isolate_folios, emitted once per isolate_folios() call with > >> the memcg id, the type actually scanned, the effective swappiness, the > >> scanned/isolated counts, and the anon/file min_seq and max_seq. The > >> min_seq/max_seq triplet ties each isolation to the generation layout it > >> ran against, which the classic-LRU tracepoints cannot express. > >> > >> This is emitted at the isolate_folios() layer, which is MGLRU-specific > >> and has no classic-LRU counterpart, so it neither changes nor duplicates > >> the existing scan/evict tracepoints. > >> > > > > I have no objection to MGLRU having some tracepoints. However, one > > major concern is that the code is changing rapidly, so the tracepoints > > may not be stable. We would need to maintain them as the code evolves. > > > > Indeed. GEN-LRU has been available for several years now, and many vendors are > already using it, so adding tracepoints is a necessary step. > > >> Assisted-by: Claude:claude-opus-4-8 > >> Signed-off-by: Ridong Chen <[email protected]> > >> --- > >> include/trace/events/vmscan.h | 47 +++++++++++++++++++++++++++++++++++ > >> mm/vmscan.c | 9 ++++++- > >> 2 files changed, 55 insertions(+), 1 deletion(-) > >> > > [...] > >> diff --git a/mm/vmscan.c b/mm/vmscan.c > >> index 8409ea4bbf37..771fe6827939 100644 > >> --- a/mm/vmscan.c > >> +++ b/mm/vmscan.c > > [...] > >> @@ -4876,6 +4877,12 @@ static int isolate_folios(unsigned long nr_to_scan, > >> struct lruvec *lruvec, > >> goto retry; > >> } > >> > >> +done: > >> + trace_mm_mglru_isolate_folios(mem_cgroup_id(lruvec_memcg(lruvec)), > >> + type, swappiness, total_scanned, > >> *isolated, > >> + lrugen->min_seq[LRU_GEN_ANON], > >> + lrugen->min_seq[LRU_GEN_FILE], > >> + lrugen->max_seq); > > > > Maybe `scan_folios()` would be a better place for this, as it can more > > directly reflect what we're doing for each type. > > > > scan_folios() already contains the trace_mm_vmscan_lru_isolate > tracepoint(traditional tracepoint), but it lacks any GEN-LRU-specific > information. Adding another tracepoint there would be redundant. Therefore, I > placed the new tracepoint in isolate_folios(), where it can capture GEN-LRU's > specific details. Additionally, it can indicate whether a fallback to the > other > type has occurred.
if `trace_mm_vmscan_lru_isolate` is located in `scan_folios()`, that would be strong evidence that `scan_folios()` is a better place for adding a new tracepoint for LRU gen. There is no need to avoid this location just because we already have a tracepoint here. In fact, I think this is evidence that it is a better place, not the opposite. BTW, `isolate_folios()` might be deleted soon, depending on how we improve MGLRU and on some ongoing work from Kairui. But `scan_folios()` should be long-lived, as we need to scan folios in any case. Therefore, adding tracepoints to `isolate_folios()` doesn't seem to be related to the stable ABI concern mentioned in your cover letter. Best Regards Barry
