Re: [PATCH 2/7] mm/mglru: introduce helpers for manipulating gen and refs flags
Ridong Chen <[email protected]>
| Newsgroups | org.kernel.vger.cgroups,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On 8/20/2026 9:43 AM, Ridong Chen wrote: > > > On 8/18/2026 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]> >> --- >> include/linux/mm_inline.h | 79 +++++++++++++++++++++++++++++++++++++++++------ >> include/linux/mmzone.h | 2 ++ >> mm/folio.c | 19 +++++++----- >> mm/vmscan.c | 61 ++++++++++++++++++++---------------- >> 4 files changed, 117 insertions(+), 44 deletions(-) >> >> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h >> index 621c8653d8f7..93bf3fa221f8 100644 >> --- a/include/linux/mm_inline.h >> +++ b/include/linux/mm_inline.h >> @@ -142,10 +142,43 @@ static inline int lru_tier_from_refs(int refs, bool >> workingset) >> return workingset ? MAX_NR_TIERS - 1 : order_base_2(refs); >> } >> -static inline int folio_lru_refs(const struct folio *folio) >> +/** >> + * lru_gen_from_flags - Return the LRU generation number from folio flags. >> + * @flags: folio flags >> + * >> + * Returns: A number between 0 and LRU_GEN_MAX, inclusive. Returns -1 if the >> + * flags indicate the folio is off the list (e.g., isolated). >> + */ >> +static inline int lru_gen_from_flags(unsigned long flags) >> +{ >> + int gen = ((flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF); >> + >> + BUILD_BUG_ON(LRU_GEN_MASK & LRU_REFS_MASK); >> + gen -= 1; >> + VM_WARN_ON_ONCE(gen != -1 && gen > LRU_GEN_MAX); > > Perhaps we could define a macro such as GEN_OFF = -1 to make the code more self- > explanatory, I found this warning a bit confusing at first glance. > > LRU_GEN_MAX already bears some resemblance to MAX_NR_GENS, so introducing yet > another macro may add some clutter. > Ah, I just noticed Baolin already mentioned that, sorry for the noise. > Just my two cents. > >> + return gen; >> +} >> + >> +/** >> + * lru_gen_set_flags - Set the LRU generation number to specified folio flags. >> + * @flags: pointer to the folio flags >> + * @gen: generation number, between 0 and LRU_GEN_MAX, inclusive. >> + */ >> +static inline void lru_gen_set_flags(unsigned long *flags, int gen) >> { >> - unsigned long flags = READ_ONCE(folio->flags.f); >> + VM_WARN_ON_ONCE(gen > LRU_GEN_MAX || gen < 0); >> + BUILD_BUG_ON((LRU_GEN_MAX + 1) != MAX_NR_GENS); >> + >> + *flags &= ~LRU_GEN_MASK; >> + *flags |= (gen + 1UL) << LRU_GEN_PGOFF; >> +} >> +/** >> + * lru_refs_from_flags - Return LRU referenced / access count from folio flags. >> + * @flags: folio flags >> + */ >> +static inline int lru_refs_from_flags(unsigned long flags) >> +{ >> if (!(flags & BIT(PG_referenced))) >> return 0; >> /* >> @@ -155,18 +188,47 @@ static inline int folio_lru_refs(const struct folio *folio) >> return ((flags & LRU_REFS_MASK) >> LRU_REFS_PGOFF) + 1; >> } >> -static inline int folio_lru_gen(const struct folio *folio) >> +/** >> + * lru_refs_set_flags - Set the LRU referenced / access count to specified >> folio flags. >> + * @flags: pointer to the folio flags >> + * @refs: referenced / access count number, between 0 and LRU_REFS_MAX, >> inclusive. >> + */ >> +static inline void lru_refs_set_flags(unsigned long *flags, unsigned int refs) >> +{ >> + VM_WARN_ON_ONCE(refs > LRU_REFS_MAX); >> + BUILD_BUG_ON(LRU_REFS_MAX != (LRU_REFS_MASK >> LRU_REFS_PGOFF) + 1); >> + >> + *flags &= ~LRU_REFS_FLAGS; >> + if (!refs) >> + return; >> + *flags |= (BIT(PG_referenced) | ((refs - 1UL) << LRU_REFS_PGOFF)); >> +} >> + >> +static inline int folio_lru_refs(const struct folio *folio) >> { >> - unsigned long flags = READ_ONCE(folio->flags.f); >> + return lru_refs_from_flags(READ_ONCE(*const_folio_flags(folio, 0))); >> +} >> + >> +static inline void folio_set_lru_refs(struct folio *folio, unsigned int refs) >> +{ >> + unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0)); >> + >> + do { >> + new_flags = old_flags; >> + lru_refs_set_flags(&new_flags, refs); >> + } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags)); >> +} >> - return ((flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; >> +static inline int folio_lru_gen(const struct folio *folio) >> +{ >> + return lru_gen_from_flags(READ_ONCE(*const_folio_flags(folio, 0))); >> } >> static inline bool lru_gen_is_active(const struct lruvec *lruvec, int gen) >> { >> unsigned long max_seq = lruvec->lrugen.max_seq; >> - VM_WARN_ON_ONCE(gen >= MAX_NR_GENS); >> + VM_WARN_ON_ONCE(gen > LRU_GEN_MAX); >> /* see the comment on MIN_NR_GENS */ >> return gen == lru_gen_from_seq(max_seq) || gen == >> lru_gen_from_seq(max_seq - 1); >> @@ -270,7 +332,7 @@ static inline bool lru_gen_add_folio(struct lruvec >> *lruvec, struct folio *folio, >> gen = lru_gen_from_seq(seq); >> flags = (gen + 1UL) << LRU_GEN_PGOFF; >> /* see the comment on MIN_NR_GENS about PG_active */ >> - set_mask_bits(&folio->flags.f, LRU_GEN_MASK | BIT(PG_active), flags); >> + set_mask_bits(folio_flags(folio, 0), LRU_GEN_MASK | BIT(PG_active), flags); >> lru_gen_update_size(lruvec, folio, -1, gen); >> /* for folio_rotate_reclaimable() */ >> @@ -295,7 +357,7 @@ static inline bool lru_gen_del_folio(struct lruvec >> *lruvec, struct folio *folio, >> /* for folio_migrate_flags() */ >> flags = !reclaiming && lru_gen_is_active(lruvec, gen) ? BIT(PG_active) : 0; >> - flags = set_mask_bits(&folio->flags.f, LRU_GEN_MASK, flags); >> + flags = set_mask_bits(folio_flags(folio, 0), LRU_GEN_MASK, flags); >> gen = ((flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; >> lru_gen_update_size(lruvec, folio, gen, -1); >> @@ -339,7 +401,6 @@ static inline bool lru_gen_del_folio(struct lruvec >> *lruvec, struct folio *folio, >> static inline void folio_migrate_refs(struct folio *new, const struct folio >> *old) >> { >> - >> } >> #endif /* CONFIG_LRU_GEN */ >> 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) >> #define LRU_REFS_MASK ((BIT(LRU_REFS_WIDTH) - 1) << LRU_REFS_PGOFF) >> +#define LRU_REFS_MAX BIT(LRU_REFS_WIDTH) >> /* >> * For folios accessed multiple times through file descriptors, >> diff --git a/mm/folio.c b/mm/folio.c >> index 59c477120b9a..0adfe4f5ef72 100644 >> --- a/mm/folio.c >> +++ b/mm/folio.c >> @@ -353,26 +353,28 @@ static void __lru_cache_activate_folio(struct folio *folio) >> static void lru_gen_inc_refs(struct folio *folio) >> { >> - unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f); >> + unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0)); >> + int refs; >> if (folio_test_unevictable(folio)) >> return; >> /* see the comment on LRU_REFS_FLAGS */ >> - if (!folio_test_referenced(folio)) { >> - set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced)); >> + if (!folio_lru_refs(folio)) { >> + folio_set_lru_refs(folio, 1); >> return; >> } >> do { >> - if ((old_flags & LRU_REFS_MASK) == LRU_REFS_MASK) { >> + new_flags = old_flags; >> + refs = lru_refs_from_flags(old_flags); >> + if (refs == LRU_REFS_MAX) { >> if (!folio_test_workingset(folio)) >> folio_set_workingset(folio); >> return; >> } >> - >> - new_flags = old_flags + BIT(LRU_REFS_PGOFF); >> - } while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags)); >> + lru_refs_set_flags(&new_flags, refs + 1); >> + } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags)); >> } >> static bool lru_gen_clear_refs(struct folio *folio) >> @@ -384,7 +386,8 @@ static bool lru_gen_clear_refs(struct folio *folio) >> if (gen < 0) >> return true; >> - set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS | BIT(PG_workingset), 0); >> + folio_set_lru_refs(folio, 0); >> + folio_clear_workingset(folio); >> rcu_read_lock(); >> seq = READ_ONCE(folio_lruvec(folio)->lrugen.min_seq[type]); >> diff --git a/mm/vmscan.c b/mm/vmscan.c >> index c1404a59523d..080132997d87 100644 >> --- a/mm/vmscan.c >> +++ b/mm/vmscan.c >> @@ -843,19 +843,22 @@ static bool lru_gen_set_refs(struct folio *folio, const >> vma_flags_t *vma_flags) >> if (!folio_test_referenced(folio) && !folio_test_workingset(folio)) { >> /* Activate file-backed executable folios after first usage. */ >> if (is_exec_file_folio(folio, vma_flags)) { >> - set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset)); >> + folio_set_lru_refs(folio, 0); >> + folio_set_workingset(folio); >> return true; >> } >> - set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced)); >> + folio_set_lru_refs(folio, 1); >> return false; >> } >> /* Promote on second access */ >> - if (folio_lru_refs(folio) > 1) >> - set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_workingset)); >> - else >> + if (folio_lru_refs(folio) > 1) { >> + folio_set_lru_refs(folio, 0); >> + folio_set_workingset(folio); >> + } else { >> folio_mark_accessed(folio); >> + } >> return true; >> } >> #else >> @@ -3266,11 +3269,10 @@ static bool positive_ctrl_err(struct ctrl_pos *sp, >> struct ctrl_pos *pv) >> >> ******************************************************************************/ >> /* promote pages accessed through page tables */ >> -static int folio_update_gen(struct folio *folio, int gen, const vma_flags_t >> *vma_flags) >> +static int folio_update_gen(struct folio *folio, int new_gen, const >> vma_flags_t *vma_flags) >> { >> - unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f); >> - >> - VM_WARN_ON_ONCE(gen >= MAX_NR_GENS); >> + unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0)); >> + int old_gen; >> /* >> * See the comment on LRU_REFS_FLAGS, and activate file-backed >> @@ -3279,20 +3281,24 @@ static int folio_update_gen(struct folio *folio, int >> gen, const vma_flags_t *vma >> */ >> if (!folio_test_referenced(folio) && !folio_test_workingset(folio) && >> !is_exec_file_folio(folio, vma_flags)) { >> - set_mask_bits(&folio->flags.f, LRU_REFS_MASK, BIT(PG_referenced)); >> + folio_set_lru_refs(folio, 1); >> return -1; >> } >> do { >> + old_gen = lru_gen_from_flags(old_flags); >> + new_flags = old_flags; >> + >> /* lru_gen_del_folio() has isolated this page? */ >> - if (!(old_flags & LRU_GEN_MASK)) >> - return -1; >> + if (old_gen < 0) >> + break; >> - new_flags = old_flags & ~(LRU_GEN_MASK | LRU_REFS_FLAGS); >> - new_flags |= ((gen + 1UL) << LRU_GEN_PGOFF) | BIT(PG_workingset); >> - } while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags)); >> + lru_gen_set_flags(&new_flags, new_gen); >> + lru_refs_set_flags(&new_flags, 0); >> + new_flags |= BIT(PG_workingset); >> + } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags)); >> - return ((old_flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; >> + return old_gen; >> } >> /* protect pages accessed multiple times through file descriptors */ >> @@ -3301,21 +3307,20 @@ static int folio_inc_gen(struct lruvec *lruvec, struct >> folio *folio) >> int type = folio_is_file_lru(folio); >> struct lru_gen_folio *lrugen = &lruvec->lrugen; >> int new_gen, old_gen = lru_gen_from_seq(lrugen->min_seq[type]); >> - unsigned long new_flags, old_flags = READ_ONCE(folio->flags.f); >> - >> - VM_WARN_ON_ONCE_FOLIO(!(old_flags & LRU_GEN_MASK), folio); >> + unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0)); >> do { >> - new_gen = ((old_flags & LRU_GEN_MASK) >> LRU_GEN_PGOFF) - 1; >> + new_gen = lru_gen_from_flags(old_flags); >> + >> /* folio_update_gen() has promoted this page? */ >> if (new_gen >= 0 && new_gen != old_gen) >> return new_gen; >> + new_flags = old_flags; >> new_gen = (old_gen + 1) % MAX_NR_GENS; >> - >> - new_flags = old_flags & ~(LRU_GEN_MASK | LRU_REFS_FLAGS); >> - new_flags |= (new_gen + 1UL) << LRU_GEN_PGOFF; >> - } while (!try_cmpxchg(&folio->flags.f, &old_flags, new_flags)); >> + lru_gen_set_flags(&new_flags, new_gen); >> + lru_refs_set_flags(&new_flags, 0); >> + } while (!try_cmpxchg(folio_flags(folio, 0), &old_flags, new_flags)); >> lru_gen_update_size(lruvec, folio, old_gen, new_gen); >> @@ -4711,7 +4716,7 @@ static bool isolate_folio(struct lruvec *lruvec, struct >> folio *folio, struct sca >> /* see the comment on LRU_REFS_FLAGS */ >> if (!folio_test_referenced(folio)) >> - set_mask_bits(&folio->flags.f, LRU_REFS_MASK, 0); >> + folio_set_lru_refs(folio, 0); >> success = lru_gen_del_folio(lruvec, folio, true); >> VM_WARN_ON_ONCE_FOLIO(!success, folio); >> @@ -4927,8 +4932,10 @@ static int evict_folios(unsigned long nr_to_scan, >> struct lruvec *lruvec, >> } >> /* don't add rejected folios to the oldest generation */ >> - if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) >> - set_mask_bits(&folio->flags.f, LRU_REFS_FLAGS, BIT(PG_active)); >> + if (lru_gen_folio_seq(lruvec, folio, false) == min_seq[type]) { >> + folio_set_lru_refs(folio, 0); >> + folio_set_active(folio); >> + } >> } >> move_folios_to_lru(&list); >> > -- Best regards Ridong