Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Ridong Chen <ridong.chen@linux.dev>
To: Barry Song <baohua@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	David Hildenbrand <david@kernel.org>,
	Michal Hocko <mhocko@kernel.org>, Qi Zheng <qi.zheng@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	Lorenzo Stoakes <ljs@kernel.org>,
	Kairui Song <kasong@tencent.com>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Yuanchu Xie <yuanchu@google.com>, Wei Xu <weixugc@google.com>,
	Brian Geffon <bgeffon@google.com>,
	"Jan Alexander Steffens (heftig)" <heftig@archlinux.org>,
	Steven Barrett <steven@liquorix.net>, Yu Zhao <yuzhao@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Ridong Chen <chenridong@xiaomi.com>
Subject: Re: [RFC PATCH] mm/mglru: preserve inactive placement when enabling MGLRU
Date: Fri, 14 Aug 2026 10:22:23 +0800	[thread overview]
Message-ID: <c10dd51b-b439-4415-acfb-83a4024346cb@linux.dev> (raw)
In-Reply-To: <CAGsJ_4wEpmxZyDFj5eXwEDY_Rbveue552OqqiyZ2MOJw2TTrrA@mail.gmail.com>



On 8/13/2026 7:37 PM, Barry Song wrote:
> On Thu, Aug 13, 2026 at 7:24 PM Barry Song <baohua@kernel.org> wrote:
>>
>> On Thu, Aug 13, 2026 at 7:02 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>>
>>> From: Ridong Chen <chenridong@xiaomi.com>
>>>
>>> When the LRU is switched to MGLRU (echo y > /sys/kernel/mm/lru_gen/
>>> enabled), fill_evictable() re-inserts every folio via
>>> lru_gen_add_folio(..., false).  With reclaiming hardcoded to false, an
>>> inactive anonymous folio (no PG_active, not in the swapcache) takes the
>>> "gen = MIN_NR_GENS" branch in lru_gen_folio_seq() and is seeded at
>>> seq = max_seq - 1, which lru_gen_is_active() treats as active.  Its
>>> inactive placement is lost and NR_INACTIVE_ANON is folded into
>>> NR_ACTIVE_ANON.
>>>
>>> Pass reclaiming=!active so a folio from an inactive list is seeded into
>>> an older generation.  Folios from the active list carry PG_active and
>>> hit the first branch either way, so they are unchanged.
>>>
>>> Tested on x86_64, next-20260812, 2G VM + 1G swap, ~1.5G anon pushed onto
>>> the inactive list before enabling MGLRU:
>>>
>>>                           Active(anon)  Inactive(anon)
>>>    before switch (legacy)        2952         1548792   kB
>>>    after `echo y`, unpatched  1552052               0   kB
>>>    after `echo y`, patched      15144         1536636   kB
>>>
>>> Inactive file folios stay inactive either way (NR_INACTIVE_FILE is
>>> preserved).
>>>
>>> Fixes: 354ed5974429 ("mm: multi-gen LRU: kill switch")
>>> Assisted-by: Claude:claude-opus-4-8
>>> Signed-off-by: Ridong Chen <chenridong@xiaomi.com>
>>> ---
>>>   mm/vmscan.c | 7 ++++++-
>>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 94fc4f25e99f..2befc8d7dd3f 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -5319,7 +5319,12 @@ static bool fill_evictable(struct lruvec *lruvec)
>>>                          VM_WARN_ON_ONCE_FOLIO(folio_lru_gen(folio) != -1, folio);
>>>
>>>                          lruvec_del_folio(lruvec, folio);
>>> -                       success = lru_gen_add_folio(lruvec, folio, false);
>>> +                       /*
>>> +                        * Keep a folio from the inactive list inactive:
>>> +                        * pass reclaiming=!active so it is not seeded as
>>> +                        * active.  See lru_gen_folio_seq().
>>> +                        */
>>> +                       success = lru_gen_add_folio(lruvec, folio, !active);
>>

Hi Barry, Thank you for your reply.

>> This is a very interesting use of the reclaim argument, as it is not
>> intended to serve this MGLRU switch purpose. It is really only meant
>> for `folio_rotate_reclaimable()`.
>>

Yeah, I realize tying the reclaim argument to the MGLRU switch is a bit of a 
hack, since it was really only designed for folio_rotate_reclaimable(). That 
said, I'm not blind to it, I just posted an RFC patch to get the discussion 
rolling and see what people think.

>> However, the change itself seems to be *partially* correct and
>> *partially* wrong.
>>
>> One real issue is that inactive is always placed in the oldest
>> generation, while we have two old generations. Maybe we can ignore
>> this for now.
>>
>> but somehow, are we also inverting the cold/hot ordering in the
>> inactive list?
>>
>> `lru_to_folio(head)` always takes the tail, but now we are putting the
>> tail before the head folios.
>>
>> Because reclaim == true will use list_add_tail().
>>
>> if (reclaiming)
>> list_add_tail(&folio->lru, &lrugen->folios[gen][type][zone]);
> 
> I guess we can fix this by iterating in `fill_evictable()` from head
> to tail order.
> 
>    struct list_head *pos = head->next;
> 
>    while (pos != head) {
>     struct folio *folio = list_entry(pos, struct folio, lru);
>      ...
>   }
> 
> Then the oldest generation will maintain the same folio order as the
> inactive list.
> 
Thanks for the suggestion. Traversing head to tail does fix the inversion, but 
we need to distinguish active from inactive during iteration, because they are 
inserted differently: list_add_tail() when reclaiming, and list_add() otherwise. 
That said, the reclaim argument usage still feels off to me—I'd like to hear if 
others have a better idea before we proceed with further changes.

-- 
Best regards
Ridong



  reply	other threads:[~2026-08-14  2:22 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260813110230.2124785-1-ridong.chen@linux.dev>
2026-08-13 11:24 ` [RFC PATCH] mm/mglru: preserve inactive placement when enabling MGLRU Barry Song
2026-08-13 11:37   ` Barry Song
2026-08-14  2:22     ` Ridong Chen [this message]
2026-08-14  2:31       ` Barry Song
2026-08-20  7:31         ` Ridong Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c10dd51b-b439-4415-acfb-83a4024346cb@linux.dev \
    --to=ridong.chen@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=bgeffon@google.com \
    --cc=chenridong@xiaomi.com \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=heftig@archlinux.org \
    --cc=kasong@tencent.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=qi.zheng@linux.dev \
    --cc=shakeel.butt@linux.dev \
    --cc=steven@liquorix.net \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.com \
    --cc=yuzhao@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox