Re: [PATCH 7/7] mm/mglru: improve code readability and harden folio_inc_gen

On 8/20/26 8:57 AM, Baoquan He wrote:
> On 08/20/26 at 08:53am, Baoquan He wrote:
>> On 08/18/26 at 01:38pm, Kairui Song via B4 Relay wrote:
>>> From: Kairui Song <[email protected]>
>>>
>>> The helper should never be called for an off-list folio, and it always
>>> expects the folio to be in the oldest generation before doing any
>>> cmpxchg. Add a sanity check for the off-list case: if it is ever
>>> violated, bail out and keep the folio flags untouched to minimize the
>>> damage, instead of silently treating the folio as if it were in the
>>> oldest generation and promoting it updating the flags to an unexpected
>>> status.
>>>
>>> Also rename the variables to clearly distinguish the folio's current
>>> gen from the oldest gen.
>>>
>>> Signed-off-by: Kairui Song <[email protected]>
>>> ---
>>>   mm/vmscan.c | 14 +++++++++-----
>>>   1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>>> index 7169cac60869..7e3ae0c6cba3 100644
>>> --- a/mm/vmscan.c
>>> +++ b/mm/vmscan.c
>>> @@ -3308,18 +3308,22 @@ 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]);
>>> +	int new_gen, old_gen, min_gen = lru_gen_from_seq(lrugen->min_seq[type]);
>>>   	unsigned long new_flags, old_flags = READ_ONCE(*folio_flags(folio, 0));
>>>   
>>>   	do {
>>> -		new_gen = lru_gen_from_flags(old_flags);
>>> +		old_gen = lru_gen_from_flags(old_flags);
>>> +		/* This helper should never be called for off-list folios */
>>> +		VM_WARN_ON_ONCE(old_gen < 0);
>>> +		if (old_gen < 0)
>>> +			return min_gen;
>>
>> As Barry doubted, I think this change is wrong. old_gen < 0 in folio_inc_gen()
>> could only happen inc_min_seq() call it. While inc_min_seq() call it
>> because inc_max_seq() need increase max_gen to max_gen + 1 and found
>> get_nr_gens(lruvec, type) == MAX_NR_GENS, it has to move the oldest gen to
>                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 2nd old oldest gen. Here returning min_gen for old_gen < 0 means it will
>    ~~~~~~~~~~~~~~~~
> Here, I mean it has to move folios from the oldest gen (min_gen) to the 2nd
> oldest gen (min_gen + 1). The empty min_gen will become the new max_gen.
> 
>> be put in the lastest max_gen. It may not be expected.

But how does old_gen < 0 actually happen? folio_inc_gen() is called 
under the lru lock, so how can a folio listed in MGLRU have a gen 
counter < 0? If this can happen in any case, we should fix this bug first.