Re: [PATCH v6 7/7] mm/mglru: add tracepoint for inc_max_seq()

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

On 11/3/2026 6:00 PM, Barry Song wrote:
> On Tue, Nov 3, 2026 at 4:39 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
>>
>>
>> On 11/3/2026 2:48 PM, Barry Song wrote:
>>> On Tue, Nov 3, 2026 at 10:58 AM 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.
>>>
>>> We might be more interested in why aging is triggered rather
>>> than the exact moment when max_seq is advanced.
>>>
>>> With your patch, we get:
>>>
>>> ~ # cat /sys/kernel/tracing/trace | grep mglru_inc_max_seq
>>>              a.out-93      [001] d..1.   278.276539:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=4 anon_min_seq=2
>>> file_min_seq=1 nr_anon={0x0,0xf016,0x12,0x0}
>>> nr_file={0x1886,0xb,0x4c,0x0}
>>>            kswapd0-50      [000] d..1.   278.422400:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=5 anon_min_seq=2
>>> file_min_seq=2 nr_anon={0x5,0xccb6,0x3439,0x0}
>>> nr_file={0x4ae,0x4f,0x55,0x0}
>>>            kswapd0-50      [000] d..1.   278.508003:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=6 anon_min_seq=3
>>> file_min_seq=4 nr_anon={0xccb6,0x3d29,0x1,0x0}
>>> nr_file={0x0,0x59,0x7,0x0}
>>>            kswapd0-50      [000] d..1.   280.046056:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=7 anon_min_seq=4
>>> file_min_seq=5 nr_anon={0x3bf3,0xcbbd,0x23e,0x0}
>>> nr_file={0x0,0xb,0x1,0x0}
>>>            kswapd0-50      [000] d..1.   280.124876:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=8 anon_min_seq=5
>>> file_min_seq=6 nr_anon={0xcafd,0x3e31,0x0,0x0}
>>> nr_file={0x0,0x5,0x7,0x0}
>>>              a.out-96      [001] d..1.   290.629014:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=9 anon_min_seq=7
>>> file_min_seq=6 nr_anon={0x0,0x10615,0xb,0x0}
>>> nr_file={0x331,0xb,0x9a,0x0}
>>>            kswapd0-50      [000] d..1.   290.653098:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=10 anon_min_seq=7
>>> file_min_seq=7 nr_anon={0x15,0xef23,0x18c4,0x0}
>>> nr_file={0xb,0xb8,0x55,0x0}
>>>              a.out-96      [001] d..1.   290.684875:
>>> mm_mglru_inc_max_seq: nid=0 memcg_id=1 max_seq=11 anon_min_seq=8
>>> file_min_seq=9 nr_anon={0xeee3,0x1afb,0x2,0x0}
>>> nr_file={0x0,0x76,0x4,0x0}
>>>
>>> You can see that the youngest generation is always zero,
>>> because you're just getting the newly added youngest
>>> generation :-)
>>>
>>
>> Hi Barry,
>>
>> Thank you very much for testing and reviewing.
>>
>>> This somehow suggests that the information is redundant, as
>>> there is always a zero there.
>>>
>>> I suspect we'll need an additional tracepoint to show why
>>> aging is triggered and at what sc->priority it happens.
>>>
>>> But do you know why you trace the moment just after aging
>>> is completed, rather than the reason why aging is needed?
>>>
>> My original goal was to pair it with mm_mglru_scan_folios, to see how
>> the aging window of each memcg moves. The per-generation anon/file
>> nr_pages were added following your suggestion on v1. I put it at the
>> end of inc_max_seq() because of the restart there, as max_seq and
>> min_seq are only final after that.
> 
> The per-generation anon/file nr_pages are perfectly good, as
> they can also help explain why aging is needed, especially
> now that we're introducing new code for imbalance-based
> aging.
> 
>>
>> You make a very good point that the reason for aging is also
>> important. So I'd like to discuss which way is better:
>>
>> 1) Keep mm_mglru_inc_max_seq, and add another tracepoint at the aging
>>    decision in try_to_shrink_lruvec(), reporting why aging is needed
>>    and at what sc->priority.
>>
>> 2) Replace mm_mglru_inc_max_seq with a single tracepoint in
>>    try_to_shrink_lruvec(), right before try_to_inc_max_seq(), reporting
>>    the reason, sc->priority, swappiness, max_seq/min_seq and the
>>    per-generation nr_pages before aging. The youngest generation is
>>    not empty at that point, and the new window after aging can still
>>    be seen from the following mm_mglru_scan_folios.
>>
>> Which one do you prefer?
> 
> Maybe we should keep option 2 for now. Understanding why
> we are aging and at what priority is much more important,
> especially since we're still introducing some
> imbalance-based aging code.
> 
> It should be quite simple, just a matter of moving the code
> around a little.
> 
> If we later find a good reason to have option 1, we can
> always add it, right?
> 

Hi Barry,

Agreed, I'll go with option 2. Two things I'd like to confirm first:

1) To report the reason, should_run_aging() would need to return it
  instead of a bool, e.g. an enum like:

   enum lru_gen_aging_reason {
	AGING_NONE,
	AGING_NO_EVICTABLE,     /* eviction is not possible anymore */
	AGING_MIN_GENS,		/* better to age, eviction still possible */
   };
	
Or just report sc->priority, swappiness, max_seq/min_seq and the
per-generation nr_pages, and let users derive the reason from them?
The former is more direct, the latter keeps should_run_aging()
unchanged.

2) try_to_shrink_lruvec() has changed quite a lot recently, e.g.
Kairui's reclaim loop rework, and your swappiness series also touches it, 
including the aging part. Is it stable enough now to carry a tracepoint there?

-- 
Best regards
Ridong