Re: [PATCH RFC 09/15] mm/mglru: frequency guided workingset promotion (MGLRU-FG)

[Kairui Song]

On Fri, Aug 14, 2026 at 3:19 PM Barry Song <baohua@xxxxxxxxxx> wrote:
>
> On Tue, Aug 4, 2026 at 3:47 AM Kairui Song via B4 Relay
> <devnull+kasong.tencent.com@xxxxxxxxxx> wrote:
> >
> > From: Kairui Song <kasong@xxxxxxxxxxx>
> >
> > Complement MGLRU's eviction-time tier-PID protection with access-time
> > frequency-guided promotion.  Introduce a unified set of helpers built based
> > on referenced (access) count of a folio.
> >
> > Each access increments a folio's referenced count stored in folio flags
> > (refs), refs still mappes to a logarithmic tier just like before, but with
> > more formal bit definitions, a few special thresholds are introduced:
> > LRU_REFS_REFERENCED (1), LRU_REFS_WORKINGSET (2), LRU_REFS_PROTECTED (3),
> > and LRU_REFS_MAX(7). When it reaches certain threshold, the folio is
> > promoted proactively instead of wait for the PID controller to kick in.
> >
> > Also simplify MGLRU's usage of PG_workingset and PG_referenced, now
> > these 2 flags are purely used as the lower 2 bit of refs for MGLRU. This
> > doesn't effect classical LRU in any way. This will actually simplify and
> > make MGLRU's certain metric reading more accurate, and reduced MGLRU's
> > original tier / referenced count bit by one since only one extra bit is
> > now needed to record a max referenced count of 7 (previously 2 extra bits
> > are needed). This changes make sense because MGLRU doesn't have demotion
> > so these 2 flags are never separately useful for MGLRU.
> >
> > This addresses several shortcomings of the old model:
> [...]
>
> > + * Update the folio's lru refs indicator without taking the folio lock,
> > + * isolation, or lruvec lock. Used by both page table access (@is_fault=true)
> > + * and by file access (@is_fault=false).
> > + */
> > +int folio_inc_lru_refs(struct folio *folio, bool is_fault, bool is_exec)
> > +{
>
>
> This is really hard to follow, especially with call sites like:
>
> folio_inc_lru_refs(folio, true, false);
> folio_inc_lru_refs(folio, false, false);
>
> The reader needs to remember what the first true and the second
> false represent, which makes the call sites quite opaque.
>
> Could we have something like this instead?
>
> #define LRU_REFS_FAULT BIT(0)
> #define LRU_REFS_EXEC BIT(1)
>
> folio_inc_lru_refs(folio, LRU_REFS_FAULT);
> folio_inc_lru_refs(folio, 0);
> folio_inc_lru_refs(folio, LRU_REFS_EXEC);
> folio_inc_lru_refs(folio, LRU_REFS_EXEC | LRU_REFS_FAULT);

Thanks very much for the review and suggestion!

Good idea, but to distinguish from macros like LRU_REFS_PROTECTED and
LRU_REFS_WORKINGSET, which are special referenced count number, maybe
LRU_REF_EXEC and LRU_REF_FAULT? BTW we have macros like
FOLIOREF_RECLAIM_CLEAN, FOLIOREF_KEEP, and folio_referenced, etc.,
which are commonly used already, that's one main hint behind the name
choice here.