Re: [PATCH mm-new v2] mm: vmscan: put rotation-missed folios at the LRU tail

[Ridong Chen <ridong.chen@linux.dev>]

On 9/21/2026 5:31 AM, Barry Song wrote:
> On Sun, Sep 20, 2026 at 9:25 PM Ridong Chen <ridong.chen@linux.dev> wrote:
>>
> [...]
>>
>>   mm/vmscan.c | 24 ++++++++++++++++++------
>>   1 file changed, 18 insertions(+), 6 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index e200ce3eb056..91295070ca33 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -1971,7 +1971,7 @@ static bool too_many_isolated(struct pglist_data *pgdat, int file,
>>    *
>>    * Note: The caller must not hold any lruvec lock.
>>    */
>> -static unsigned int move_folios_to_lru(struct list_head *list)
>> +static unsigned int move_folios_to_lru(struct list_head *list, bool do_rotate)
>>   {
>>          int nr_pages, nr_moved = 0;
>>          struct lruvec *lruvec = NULL;
>> @@ -2018,7 +2018,19 @@ static unsigned int move_folios_to_lru(struct list_head *list)
>>                          continue;
>>                  }
>>
>> -               lruvec_add_folio(lruvec, folio);
>> +               /*
>> +                * Put clean, unreferenced and unpinned folios that may have
>> +                * missed folio_rotate_reclaimable() at the tail to avoid
>> +                * cold/hot inversion.
>> +                */
>> +               if (do_rotate && !folio_test_active(folio) && !folio_mapped(folio) &&
>> +                   !folio_test_dirty(folio) && !folio_test_writeback(folio) &&
>> +                   !folio_test_referenced(folio) &&
>> +                   folio_ref_count(folio) == folio_expected_ref_count(folio))
> 
> 
> I guess this is wrong. We hold an extra reference while isolating
> the folio, so I think this should be:
> 
> `folio_ref_count(folio) == folio_expected_ref_count(folio) + 1`
> 
> Am I missing something here?
> 
Hi Barry,

Thank you for your review.

The folios we want to check have the following lifecycle:

1. In isolate_lru_folios, we take an extra reference (i.e., +1).

2. In __remove_mapping, the folio can only be frozen successfully when refcount 
== 1 + folio_nr_pages(folio). We need to exclude folios whose refcount is not 1 
+ folio_nr_pages(folio) (e.g., those pinned by GUP), as reported by Sashiko.

```
	...
	refcount = 1 + folio_nr_pages(folio);
	if (!folio_ref_freeze(folio, refcount))
		goto cannot_free;
	...
``` 	

3. In move_folios_to_lru, we drop the extra reference and move the folio back to 
the lruvec.

```
...
		if (unlikely(folio_put_testzero(folio))) {
			__folio_clear_lru_flags(folio);
...
		}

		if (do_rotate && !folio_test_active(folio) && !folio_mapped(folio) &&
		    !folio_test_dirty(folio) && !folio_test_writeback(folio) &&
		    !folio_test_referenced(folio) &&
		    folio_ref_count(folio) == folio_expected_ref_count(folio))
			lruvec_add_folio_tail(lruvec, folio);
		else
			lruvec_add_folio(lruvec, folio);
```

The refcount check is added after the extra reference has been dropped.

Therefore, I believe it should be 'folio_ref_count(folio) == 
folio_expected_ref_count(folio)', not 'folio_ref_count(folio) == 
folio_expected_ref_count(folio) + 1'.

-- 
Best regards
Ridong