Re: [PATCH 1/2] mm: vmscan: remove folio_test_private() check in pageout()

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

On 2025/9/13 00:13, Shakeel Butt wrote:
> On Fri, Sep 12, 2025 at 11:45:07AM +0800, Baolin Wang wrote:
>> Currently, we no longer attempt to write back filesystem folios in pageout(),
>> and only tmpfs/shmem folios and anonymous swapcache folios can be written back.
>> Moreover, tmpfs/shmem and swapcache folios do not use the PG_private flag,
>> which means no fs-private private data is used. Therefore, we can remove the
>> redundant folio_test_private() checks and related buffer_head release logic.
>>
>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>>   mm/vmscan.c | 16 +---------------
>>   1 file changed, 1 insertion(+), 15 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index f1fc36729ddd..8056fccb9cc4 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -697,22 +697,8 @@ static pageout_t pageout(struct folio *folio, struct address_space *mapping,
>>   	 * swap_backing_dev_info is bust: it doesn't reflect the
>>   	 * congestion state of the swapdevs.  Easy to fix, if needed.
>>   	 */
>> -	if (!is_page_cache_freeable(folio))
>> +	if (!is_page_cache_freeable(folio) || !mapping)
>>   		return PAGE_KEEP;
>> -	if (!mapping) {
>> -		/*
>> -		 * Some data journaling orphaned folios can have
>> -		 * folio->mapping == NULL while being dirty with clean buffers.
>> -		 */
> 
> Can this case not happen anymore and try_to_free_buffers is not needed?

For dirty file folios, pageout() will return PAGE_KEEP and put them back 
on the LRU list. So even if mapping = NULL, background workers for 
writeback will continue to handle them, rather than in shrink_folio_list().

For clean file folios, the !mapping case will be be handled later in 
shrink_folio_list(), please see the following comments:

/*
  * If the folio has buffers, try to free the buffer
  * mappings associated with this folio. If we succeed
  * we try to free the folio as well.
  *
  * We do this even if the folio is dirty.
  * filemap_release_folio() does not perform I/O, but it
  * is possible for a folio to have the dirty flag set,
  * but it is actually clean (all its buffers are clean).
  * This happens if the buffers were written out directly,
  * with submit_bh(). ext3 will do this, as well as
  * the blockdev mapping.  filemap_release_folio() will
  * discover that cleanness and will drop the buffers
  * and mark the folio clean - it can be freed.
  *
  * Rarely, folios can have buffers and no ->mapping.
  * These are the folios which were not successfully
  * invalidated in truncate_cleanup_folio().  We try to
  * drop those buffers here and if that worked, and the
  * folio is no longer mapped into process address space
  * (refcount == 1) it can be freed.  Otherwise, leave
  * the folio on the LRU so it is swappable.
  */

>> -		if (folio_test_private(folio)) {
>> -			if (try_to_free_buffers(folio)) {
>> -				folio_clear_dirty(folio);
>> -				pr_info("%s: orphaned folio\n", __func__);
>> -				return PAGE_CLEAN;
>> -			}
>> -		}
>> -		return PAGE_KEEP;
>> -	}
>>   
>>   	if (!shmem_mapping(mapping) && !folio_test_anon(folio))
>>   		return PAGE_ACTIVATE;
>> -- 
>> 2.43.7
>>