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

On Wed, Aug 19, 2026 at 5:46 PM Baolin Wang
<[email protected]> wrote:
> On 8/19/26 5:37 PM, Kairui Song wrote:
> > 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).
>
> 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.

Right, I think let me just drop LRU_GEN_MAX then, there is no real
user, it's just sanity check macro.