Re: [PATCH] mm/mglru: fix and remove redundant unevictable folio handling

On Wed, Aug 12, 2026 at 6:06 PM Baolin Wang
<[email protected]> wrote:
>
> On 8/11/26 5:40 PM, Kairui Song via B4 Relay wrote:
> > From: Kairui Song <[email protected]>
> >
> > sort_folio() has a shortcut for moving folios that are no longer
> > evictable but are still sitting on a generation list.  However, this
> > shortcut is buggy.  It does not follow the PG_lru usage convention,
> > and it has a more serious issue.
> >
> > Unevictable folios are not threaded on lists[LRU_UNEVICTABLE], so that
> > folio->lru can be reused to hold folio->mlock_count (see the comment in
> > lruvec_init()).  Hence lruvec_add_folio() skips the list_add() for them,
> > and every other place that turns a folio unevictable initialises
> > mlock_count explicitly: lru_add() sets it to 0, __mlock_folio() and
> > __mlock_new_folio() set it to !!folio_test_mlocked(folio).
> > sort_folio() sets nothing, and the lru_gen_del_folio() right above it
> > may have already poisoned folio->lru via list_del(), so mlock_count
> > ends up aliasing LIST_POISON2, which reads as 0x122, i.e. 290.  The
> > result is user visible.  On munlock, __munlock_folio() decrements that
> > bogus count, finds it still non-zero and bails out before clearing
> > PG_mlocked, so the folio remains unevictable and the Mlocked
> > accounting stays inflated until the folio is freed.
> >
> > The shortcut also touches the LRU flags in the wrong order.  It calls
> > lru_gen_del_folio() while PG_lru is still set, so a concurrent
> > folio_test_clear_lru() (e.g. compaction, folio_isolate_lru()) can succeed
> > on a folio that has already been taken off the generation list, may
> > lead to unexpected behavior.  The generic path gets this right:
> > isolate_folio() clears PG_lru first, so a racing isolator loses the
> > atomic and bails.
> >
> > And the shortcut is redundant.  A folio left on the generation list is
> > picked up by isolate_folio(), shrink_folio_list() sends it to
> > activate_locked on the !folio_evictable() check, and evict_folios()
> > then hands it to folio_putback_lru(), which sets PG_unevictable and
> > counts UNEVICTABLE_PGCULLED from lru_add(), with mlock_count
> > initialised properly.
> >
> > There is no performance concern either: such a folio goes through this
> > once, and then it is off the generation lists for good, since
> > lru_gen_add_folio() refuses unevictable folios.
> >
> > So just remove the shortcut.  This consolidates unevictable handling in
> > the generic path, and makes maintenance easier.
> >
> > Fixes: ac35a4902374 ("mm: multi-gen LRU: minimal implementation")
> > Signed-off-by: Kairui Song <[email protected]>
> > ---
>
> Good catch. Make sense to me.
> Reviewed-by: Baolin Wang <[email protected]>

Hi Baolin

Thank you very mcuh for the review!

I just sent a V2 here:
https://lore.kernel.org/linux-mm/[email protected]/T/#u

This is a bit different, idea is still the same but I changed the
code, since the patch is very small so it's basically a rewrite, hence
I didn't include this review by. Can you help have a look at v2 as
well?