Re: [PATCH RFC 1/2] mm/mglru: add tracepoint for folio isolation

[Ridong Chen <ridong.chen@linux.dev>]

On 9/9/2026 8:11 PM, Barry Song wrote:
> On Wed, Sep 9, 2026 at 8:00 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>>
>>
>> On 9/9/2026 2:40 PM, Barry Song wrote:
>>> On Mon, Sep 7, 2026 at 12:25 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>>>
>>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>>
>>>> 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 <chenridong@xiaomi.com>
>>>> ---
>>>>    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.
> 

Good point.

Thank you very much, that does make sense.

> 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

-- 
Best regards
Ridong