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

[Baolin Wang]

On 8/19/26 5:37 PM, Kairui Song wrote:
> On Wed, Aug 19, 2026 at 5:03 PM Baolin Wang
> <baolin.wang@xxxxxxxxxxxxxxxxx> wrote:
> > On 8/18/26 1:38 PM, Kairui Song via B4 Relay wrote:
> > > From: Kairui Song <kasong@xxxxxxxxxxx>
> > >
> > > Instead of doing bit ops on folio->flags.f, introduce helpers for
> > > adjusting folio's refs and gen info, make 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.
> > >
> > > Signed-off-by: Kairui Song <kasong@xxxxxxxxxxx>
> > > ---
> >
> > Thanks for the cleanups. One comment below.
>
> Thanks for the review!
>
> > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> > > index 94f9c3ff5416..32d9354a754f 100644
> > > --- a/include/linux/mmzone.h
> > > +++ b/include/linux/mmzone.h
> > > @@ -496,7 +496,9 @@ enum lruvec_flags {
> > >    #ifndef __GENERATING_BOUNDS_H
> > >
> > >    #define LRU_GEN_MASK                ((BIT(LRU_GEN_WIDTH) - 1) << LRU_GEN_PGOFF)
> > > +#define LRU_GEN_MAX          (BIT(LRU_GEN_WIDTH - 1) - 1)
> >
> > There are still many places using MAX_NR_GENS to compare gen counters,
> > so I'm concerned that introducing a new macro would make the use of the
> > max gen counter even more confusing.
>
> I think I'd like to get rid of all LRU_GEN_MASK and LRU_REFS_MASK
> users, MAX_NR_GENS is fine I think? Manipulating the bitmask looks a
> bit hard to follow for me since:
>
> set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset));
> set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced));
> set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, <number>);
> set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_active));
> set_mask_bits(&folio->flags.f, LRU_REFS_MASK, <number>);
> new_flags = old_flags & ~(LRU_GEN_MASK | LRU_REFS_FLAGS);
> new_flags |= ((gen + 1UL) << LRU_GEN_PGOFF) | BIT(PG_workingset);
>
> Things like this are everywhere and I found it really hard to follow
> what is actually going on. And adding more operations to refs/gen
> looks ugly and painful to do, and, it's impossible to add any sanity
> check. With proper gen and refs helpers, we can ensure every time
> gen/refs is modified, the value is valid (in debug builds).

I agree LRU_GEN_MASK and LRU_REFS_MASK can be cleaned up further.

> Mean while, a "% MAX_NR_GENS" or "/ MIN_NR_GEN" seems not hard to understand.
>
> Oh and this LRU_GEN_MAX is only used for sanity check for now.

But my point is that people may not know when to use LRU_GEN_MAX vs. 
MAX_NR_GENS, since they seem to have the same semantics.