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

On Wed, Aug 19, 2026 at 5:03 PM Baolin Wang
<[email protected]> wrote:
> On 8/18/26 1:38 PM, Kairui Song via B4 Relay wrote:
> > From: Kairui Song <[email protected]>
> >
> > 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 <[email protected]>
> > ---
>
> 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).

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.