Re: [PATCH v2] mm/mglru: fix a no-op lru_gen_clear_refs() call in lru_lazyfree()

[Barry Song]

On Sat, Oct 31, 2026 at 3:27 PM Kairui Song <ryncsn@xxxxxxxxx> wrote:
[...]
> > > > >
> > > > >  mm/folio.c | 6 ++----
> > > > >  1 file changed, 2 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/mm/folio.c b/mm/folio.c
> > > > > index 35e242b48870..ea3adb6a89d7 100644
> > > > > --- a/mm/folio.c
> > > > > +++ b/mm/folio.c
> > > > > @@ -597,12 +597,10 @@ static void lru_lazyfree(struct lruvec *lruvec, struct folio *folio)
> > > > >             folio_test_swapcache(folio) || folio_test_unevictable(folio))
> > > > >                 return;
> > > > >
> > > > > +       lru_gen_clear_refs(folio);
> > > > >         lruvec_del_folio(lruvec, folio);
> > > > >         folio_clear_active(folio);
> > > > > -       if (lru_gen_enabled())
> > > > > -               lru_gen_clear_refs(folio);
> > > > > -       else
> > > > > -               folio_clear_referenced(folio);
> > > > > +       folio_clear_referenced(folio);
> > > >
> > > > Why can't we just call folio_set_lru_refs(.., 0), and should we clear
> > > > PG_workingset? Keeping it helps PSI on refualt / redirty tracking (not
> > > > sure if meaningful but CLRU does this), clearing it makes eviction
> > > > better, but the eviction issue is MGLRU-only, CLRU keeps it.
> > >
> > > I recall that in v1 [1], Hui mentioned:
> > >
> > > "Use folio_clear_referenced() rather than
> > > folio_set_lru_refs(), which is not defined without
> > > CONFIG_LRU_GEN."
> > >
> >
> > Or prehaps we can have a folio_clear_lru_refs() do in a unified way
> > for For both CLRU / MGLRU? (Just like folio_migrate_lru_refs).
>
> And it will be better if we can keep this operation off-list just like
> right now :), some later works could benefit from it.

lru_gen_clear_refs() is currently multifunctional. It clears
refs/workingset, but also returns true to skip
folio_batch_add_and_move() in both deactivate_file_folio()
and folio_deactivate().

This actually looks a bit weird. However, following the current
logic, I've drafted a version that we can use as a starting point
for discussion:

diff --git a/mm/folio.c b/mm/folio.c
index dc2c796a4ea7..bc17cd3b6b6c 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -444,12 +444,12 @@ static bool lru_gen_clear_refs(struct folio *folio)
        int type = folio_is_file_lru(folio);
        unsigned long seq;

-       if (gen < 0)
-               return true;
-
        folio_set_lru_refs(folio, 0);
        folio_clear_workingset(folio);

+       if (gen < 0)
+               return true;
+
        rcu_read_lock();
        seq = READ_ONCE(folio_lruvec(folio)->lrugen.min_seq[type]);
        rcu_read_unlock();
@@ -465,6 +465,8 @@ static void lru_gen_inc_refs(struct folio *folio)

 static bool lru_gen_clear_refs(struct folio *folio)
 {
+       folio_clear_referenced(folio);
+       folio_clear_workingset(folio);
        return false;
 }

The another option is:

diff --git a/mm/folio.c b/mm/folio.c
index dc2c796a4ea7..8d1fd788188c 100644
--- a/mm/folio.c
+++ b/mm/folio.c
@@ -438,6 +438,12 @@ static void lru_gen_inc_refs(struct folio *folio)
        } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
 }

+static inline void __lru_gen_clear_refs(struct folio *folio)
+{
+       folio_set_lru_refs(folio, 0);
+       folio_clear_workingset(folio);
+}
+
 static bool lru_gen_clear_refs(struct folio *folio)
 {
        int gen = folio_lru_gen(folio);
@@ -447,8 +453,7 @@ static bool lru_gen_clear_refs(struct folio *folio)
        if (gen < 0)
                return true;

-       folio_set_lru_refs(folio, 0);
-       folio_clear_workingset(folio);
+       __lru_gen_clear_refs(folio);

        rcu_read_lock();
        seq = READ_ONCE(folio_lruvec(folio)->lrugen.min_seq[type]);
@@ -463,9 +468,10 @@ static void lru_gen_inc_refs(struct folio *folio)
 {
 }

-static bool lru_gen_clear_refs(struct folio *folio)
+static inline void __lru_gen_clear_refs(struct folio *folio)
 {
-       return false;
+       folio_clear_referenced(folio);
+       folio_clear_workingset(folio);
 }

 #endif /* CONFIG_LRU_GEN */
@@ -659,10 +665,7 @@ static void lru_lazyfree(struct lruvec *lruvec,
struct folio *folio)

        lruvec_del_folio(lruvec, folio);
        folio_clear_active(folio);
-       if (lru_gen_enabled())
-               lru_gen_clear_refs(folio);
-       else
-               folio_clear_referenced(folio);
+       __lru_gen_clear_refs(folio);
        /*
         * Lazyfree folios are clean anonymous folios.  They have
         * the swapbacked flag cleared, to distinguish them from normal


I would personally prefer the latter. What do you think, Hui and Kairui?

Best,
Barry