linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ridong Chen <ridong.chen@linux.dev>
To: kasong@tencent.com, linux-mm@kvack.org
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Barry Song <baohua@kernel.org>,
	Axel Rasmussen <axelrasmussen@google.com>,
	Yuanchu Xie <yuanchu@google.com>, Wei Xu <weixugc@google.com>,
	Baoquan He <baoquan.he@linux.dev>,
	Shakeel Butt <shakeel.butt@linux.dev>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Michal Hocko <mhocko@kernel.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Muchun Song <muchun.song@linux.dev>, Chris Li <chrisl@kernel.org>,
	Baolin Wang <baolin.wang@linux.alibaba.com>,
	David Hildenbrand <david@kernel.org>,
	Lorenzo Stoakes <ljs@kernel.org>,
	"Liam R. Howlett" <liam@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>, Yu Zhao <yuzhao@google.com>,
	Zi Yan <ziy@nvidia.com>, Qi Zheng <qi.zheng@linux.dev>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	Kairui Song <ryncsn@gmail.com>
Subject: Re: [PATCH 2/7] mm/mglru: introduce helpers for manipulating gen and refs flags
Date: Thu, 20 Aug 2026 10:05:15 +0800	[thread overview]
Message-ID: <d9cc6abb-ac30-4936-ae2e-891dfe62e6c5@linux.dev> (raw)
In-Reply-To: <a13c3895-cc53-4f91-aac8-2bfef765eca5@linux.dev>



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 <kasong@tencent.com>
>>
>> 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 <kasong@tencent.com>
>> ---
>>   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


  parent reply	other threads:[~2026-08-20  2:05 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-18  5:38 [PATCH 0/7] mm/mglru: clean up folio counters and flag usage Kairui Song via B4 Relay
2026-08-18  5:38 ` [PATCH 1/7] mm/memcontrol: make lru_zone_size atomic and simplify sanity check Kairui Song via B4 Relay
2026-08-19  2:05   ` Ridong Chen
2026-08-18  5:38 ` [PATCH 2/7] mm/mglru: introduce helpers for manipulating gen and refs flags Kairui Song via B4 Relay
2026-08-19  9:03   ` Baolin Wang
2026-08-19  9:37     ` Kairui Song
2026-08-19  9:46       ` Baolin Wang
2026-08-19  9:49         ` Kairui Song
2026-08-20  1:43   ` Ridong Chen
2026-08-20  2:04     ` Ridong Chen
2026-08-20  2:05     ` Ridong Chen [this message]
2026-08-18  5:38 ` [PATCH 3/7] mm/migrate: copy the referenced state via folio_migrate_refs() Kairui Song via B4 Relay
2026-08-19 10:12   ` Baoquan He
2026-08-18  5:38 ` [PATCH 4/7] mm/mglru: move max_seq read into walk_update_folio Kairui Song via B4 Relay
2026-08-19  9:18   ` Baolin Wang
2026-08-20  2:13   ` Ridong Chen
2026-08-18  5:38 ` [PATCH 5/7] mm/mglru: use explicit tier range in read_ctrl_pos() Kairui Song via B4 Relay
2026-08-19  9:25   ` Baolin Wang
2026-08-19 10:16   ` Baoquan He
2026-08-19 21:24   ` Barry Song
2026-08-20  2:33   ` Ridong Chen
2026-08-20  3:22     ` Kairui Song
2026-08-18  5:38 ` [PATCH 6/7] mm/mglru: fix potential generation folio number leak Kairui Song via B4 Relay
2026-08-20  1:52   ` Baolin Wang
2026-08-20  3:45     ` Kairui Song
2026-08-20  8:53       ` Baolin Wang
2026-08-20  9:40         ` Kairui Song
2026-08-18  5:38 ` [PATCH 7/7] mm/mglru: improve code readability and harden folio_inc_gen Kairui Song via B4 Relay
2026-08-19 21:30   ` Barry Song
2026-08-20  0:53   ` Baoquan He
2026-08-20  0:57     ` Baoquan He
2026-08-20  1:02       ` Baolin Wang
2026-08-20  2:11         ` Kairui Song
2026-08-20  2:27         ` Baoquan He

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d9cc6abb-ac30-4936-ae2e-891dfe62e6c5@linux.dev \
    --to=ridong.chen@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=baoquan.he@linux.dev \
    --cc=cgroups@vger.kernel.org \
    --cc=chrisl@kernel.org \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=kasong@tencent.com \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=ljs@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=muchun.song@linux.dev \
    --cc=qi.zheng@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=ryncsn@gmail.com \
    --cc=shakeel.butt@linux.dev \
    --cc=vbabka@kernel.org \
    --cc=weixugc@google.com \
    --cc=yuanchu@google.com \
    --cc=yuzhao@google.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).