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

[David Hildenbrand]

On 18.09.25 05:46, Baolin Wang wrote:
> The folio_test_private() check in pageout() was introduced by commit
> ce91b575332b ("orphaned pagecache memleak fix") in 2005 (checked from
> a history tree[1]). As the commit message mentioned, it was to address
> the issue where reiserfs pagecache may be truncated while still pinned.
> To further explain, the truncation removes the page->mapping, but the
> page is still listed in the VM queues because it still has buffers.
>
> In 2008, commit a2b345642f530 ("Fix dirty page accounting leak with ext3
> data=journal") seems to be dealing with a similar issue, where the page
> becomes dirty after truncation, and it provides a very useful call stack:
> truncate_complete_page()
>        cancel_dirty_page() // PG_dirty cleared, decr. dirty pages
>        do_invalidatepage()
>          ext3_invalidatepage()
>            journal_invalidatepage()
>              journal_unmap_buffer()
>                __dispose_buffer()
>                  __journal_unfile_buffer()
>                    __journal_temp_unlink_buffer()
>                      mark_buffer_dirty(); // PG_dirty set, incr. dirty pages
>
> In this commit a2b345642f530, we forcefully clear the page's dirty flag
> during truncation (in truncate_complete_page()).
>
> Now it seems this was just a peculiar usage specific to reiserfs. Maybe
> reiserfs had some extra refcount on these pages, which caused them to pass
> the is_page_cache_freeable() check. With the fix provided by commit a2b345642f530
> and reiserfs being removed in 2024 by commit fb6f20ecb121 ("reiserfs: The
> last commit"), such a case is unlikely to occur again. So let's remove the
> redundant folio_test_private() checks and related buffer_head release logic,
> and just leave a warning here to catch such a bug.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
> Acked-by: David Hildenbrand <david@xxxxxxxxxx>
> Acked-by: Shakeel Butt <shakeel.butt@xxxxxxxxx>
> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
> ---
>   mm/vmscan.c | 12 +++---------
>   1 file changed, 3 insertions(+), 9 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index f1fc36729ddd..930add6d90ab 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -701,16 +701,10 @@ static pageout_t pageout(struct folio *folio, struct address_space *mapping,
>   		return PAGE_KEEP;
>   	if (!mapping) {
>   		/*
> -		 * Some data journaling orphaned folios can have
> -		 * folio->mapping == NULL while being dirty with clean buffers.
> +		 * Is it still possible to have a dirty folio with
> +		 * a NULL mapping? I think not.
>   		 */

I would rephrase slightly (removing the "I think not"):

/*
 * We should no longer have dirty folios with clean buffers and a NULL
 * mapping. However, let's be careful for now.
 */

> -		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;
> -			}
> -		}
> +		VM_WARN_ON_FOLIO(true, folio);
>   		return PAGE_KEEP;
>   	}
>   


--
Cheers

David / dhildenb