Re: [PATCH RFC 09/15] mm/mglru: frequency guided workingset promotion (MGLRU-FG)
Barry Song <[email protected]>
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <CAGsJ_4zVhK9SvHEd+OFAHSu-OgTiAtk-ZU5OQVx5RwBYmc9r7w@mail.gmail.com> |
On Tue, Aug 4, 2026 at 3:47 AM Kairui Song via B4 Relay <[email protected]> wrote: > > From: Kairui Song <[email protected]> > > 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); BTW