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

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 8/31/26 2:12 AM, Kairui Song wrote:
> On Thu, Aug 27, 2026 at 03:32:18PM +0800, Baolin Wang wrote:
>>
>>
>> On 8/26/26 1:53 AM, 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 a folio's refs and generation info, making 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. There is
>>> no measurable performance impact, and some paths even look slightly
>>> better in the generated assembly.
>>>
>>> Signed-off-by: Kairui Song <kasong@tencent.com>
>>> ---
>>>    include/linux/mm_inline.h | 76 ++++++++++++++++++++++++++++++++++++++++++-----
>>>    include/linux/mmzone.h    |  1 +
>>>    mm/folio.c                | 19 +++++++-----
>>>    mm/vmscan.c               | 61 ++++++++++++++++++++-----------------
>>>    4 files changed, 114 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/mm/folio.c b/mm/folio.c
>>> index c02dcea9c03c..a932059057ac 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);
>>
>> Actually this clears the folio's refs counter. Would it be more readable to
>> introduce a folio_clear_lru_refs(folio) helper and use it for all the other
>> folio_set_lru_refs(folio, 0) calls too?
> 
> Thanks for the review! And yeah, that's right. But a standalone
> folio_clear_lru_refs seems a bit bloated? I'll add some comment on the
> relationship of refs and PG_referenced for the helpers first, that
> might be helpful enough I guess?

OK, I'm fine with that.