Re: [PATCH v3 2/6] mm/mglru: introduce helpers for manipulating gen and refs flags

[Baolin Wang]

On 8/31/26 2:12 AM, Kairui Song wrote:
> On Thu, Aug 27, 2026 at 03:32:18PM +0800, Baolin Wang wrote:
> > On 8/26/26 1:53 AM, Kairui Song via B4 Relay wrote:
> > > From: Kairui Song <kasong@xxxxxxxxxxx>
> > >
> > > Instead of doing bit ops on folio->flags.f, introduce helpers for
> > > adjusting a folio's refs and generation info, making the code easier
> > > to debug and understand.
> > >
> > > No functional change is intended: some combined atomic operations are
> > > split into two, which only creates harmless transient states. There is
> > > no measurable performance impact, and some paths even look slightly
> > > better in the generated assembly.
> > >
> > > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> > > ---
> > >    include/linux/mm_inline.h | 76 ++++++++++++++++++++++++++++++++++++++++++-----
> > >    include/linux/mmzone.h    |  1 +
> > >    mm/folio.c                | 19 +++++++-----
> > >    mm/vmscan.c               | 61 ++++++++++++++++++++-----------------
> > >    4 files changed, 114 insertions(+), 43 deletions(-)
> > >
> > > diff --git a/mm/folio.c b/mm/folio.c
> > > index c02dcea9c03c..a932059057ac 100644
> > > --- a/mm/folio.c
> > > +++ b/mm/folio.c
> > > @@ -353,26 +353,28 @@ static void __lru_cache_activate_folio(struct folio *folio)
> > >    static void lru_gen_inc_refs(struct folio *folio)
> > >    {
> > > -	unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f);
> > > +	unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0));
> > > +	int refs;
> > >    	if (folio_test_unevictable(folio))
> > >    		return;
> > >    	/* see the comment on LRU_REFS_FLAGS */
> > > -	if (!folio_test_referenced(folio)) {
> > > -		set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
> > > +	if (!folio_lru_refs(folio)) {
> > > +		folio_set_lru_refs(folio, 1);
> > >    		return;
> > >    	}
> > >    	do {
> > > -		if ((old_flags & LRU_REFS_MASK) == LRU_REFS_MASK) {
> > > +		new_flags = old_flags;
> > > +		refs = lru_refs_from_flags(old_flags);
> > > +		if (refs == LRU_REFS_MAX) {
> > >    			if (!folio_test_workingset(folio))
> > >    				folio_set_workingset(folio);
> > >    			return;
> > >    		}
> > > -
> > > -		new_flags = old_flags + BIT(LRU_REFS_PGOFF);
> > > -	} while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags));
> > > +		lru_refs_set_flags(&new_flags, refs + 1);
> > > +	} while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags));
> > >    }
> > >    static bool lru_gen_clear_refs(struct folio *folio)
> > > @@ -384,7 +386,8 @@ static bool lru_gen_clear_refs(struct folio *folio)
> > >    	if (gen < 0)
> > >    		return true;
> > > -	set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS | BIT(PG_workingset), 0);
> > > +	folio_set_lru_refs(folio, 0);
> >
> > Actually this clears the folio's refs counter. Would it be more readable to
> > introduce a folio_clear_lru_refs(folio) helper and use it for all the other
> > folio_set_lru_refs(folio, 0) calls too?
>
> Thanks for the review! And yeah, that's right. But a standalone
> folio_clear_lru_refs seems a bit bloated? I'll add some comment on the
> relationship of refs and PG_referenced for the helpers first, that
> might be helpful enough I guess?

OK, I'm fine with that.