[PATCH v2 0/2] some cleanups for pageout()

[PATCH v2 0/2] some cleanups for pageout()

[Baolin Wang]

Since we no longer attempt to write back filesystem folios in pageout(),
and only tmpfs/shmem folios and anonymous swapcache folios can be written
back, we can remove the redundant folio_test_private() related logic to
simplify the logic of pageout(), as tmpfs/shmem and swapcache folios do
not use the PG_private flag.

Changes from v1:
 - Re-write the commit message for patch 1 after discussing with Hugh. Thanks.
 - Add a warning to catch this unlikely case.
 - Add acked tags. Thanks.

Baolin Wang (2):
  mm: vmscan: remove folio_test_private() check in pageout()
  mm: vmscan: simplify the folio refcount check in pageout()

 mm/vmscan.c | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

-- 
2.43.7

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

[Baolin Wang]

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@redhat.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 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.
 		 */
-		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;
 	}
 
-- 
2.43.7

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@redhat.com>
> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> ---
>   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

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

[Baolin Wang]

On 2025/9/18 14:00, David Hildenbrand wrote:
> 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@redhat.com>
>> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>> ---
>>   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.
>   */

LGTM.

Andrew, could you help squash these comments into this patch? Thanks.

>> -        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;
>>       }
> 
> 

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

[Shakeel Butt]

On Thu, Sep 18, 2025 at 05:36:17PM +0800, Baolin Wang wrote:
> 
> 
> On 2025/9/18 14:00, David Hildenbrand wrote:
> > 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@redhat.com>
> > > Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
> > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > ---
> > >   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.
> >   */
> 
> LGTM.
> 
> Andrew, could you help squash these comments into this patch? Thanks.
> 
> > > -        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);

Unexpected but better to use VM_WARN_ON_ONCE_FOLIO here.

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

[Baolin Wang]

On 2025/9/19 09:06, Shakeel Butt wrote:
> On Thu, Sep 18, 2025 at 05:36:17PM +0800, Baolin Wang wrote:
>>
>>
>> On 2025/9/18 14:00, David Hildenbrand wrote:
>>> 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@redhat.com>
>>>> Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
>>>> Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
>>>> ---
>>>>    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.
>>>    */
>>
>> LGTM.
>>
>> Andrew, could you help squash these comments into this patch? Thanks.
>>
>>>> -        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);
> 
> Unexpected but better to use VM_WARN_ON_ONCE_FOLIO here.

Um, I don't think it makes much difference, because we should no longer 
hit this.

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

[David Hildenbrand]

>>>>> -        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);
>>
>> Unexpected but better to use VM_WARN_ON_ONCE_FOLIO here.
> 
> Um, I don't think it makes much difference, because we should no longer
> hit this.

I mean, all VM_WARN_ON are not expected to be hit. But if it ever 
happens, it's usually going to be a lot.

(I recall Lorenzo wanted to look into cleaning a lot of that up and 
possibly unifying both helpers)

-- 
Cheers

David / dhildenb

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

[Hugh Dickins]

[-- Attachment #1: Type: text/plain, Size: 5733 bytes --]

On Fri, 19 Sep 2025, Baolin Wang wrote:
> On 2025/9/19 09:06, Shakeel Butt wrote:
> > On Thu, Sep 18, 2025 at 05:36:17PM +0800, Baolin Wang wrote:
> >> On 2025/9/18 14:00, David Hildenbrand wrote:
> >>> On 18.09.25 05:46, Baolin Wang wrote:
...
> >>>> 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.
> >>>    */
> >>
> >> LGTM.
> >>
> >> Andrew, could you help squash these comments into this patch? Thanks.

(Myself, I would delete the comment entirely: it's justifying the change,
which is good history to go into the commit message, but not so good in
the final source. And we don't fully understand what to say here anyway.)

> >>
> >>>> -        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);
> > 
> > Unexpected but better to use VM_WARN_ON_ONCE_FOLIO here.
> 
> Um, I don't think it makes much difference, because we should no longer hit
> this.

We do hit it.  Observe, there was no WARNING before in the !mapping
case, just a pr_info in a particular instance of the !mapping case.
We believe that instance went away with reiserfs, but that does not
imply that there are no more !mapping cases left.

We do hit that WARNING: not often, and I've not seen it repeatedly
(as ONCE-advisors fear), but a few cutdown examples from my testing
appended below.

I'm sure you'd like me to explain how it comes about: I did spend a
while looking to do so, but then found better things to do.  By all
means go in search of the explanation if it's worth your time: it
might indicate a bug somewhere; but more likely it's just a race
against code elsewhere cleaning up the folio.

The fact that it does not appear repeatedly suggests that the folio
is successfully dealt with afterwards.  I didn't think to check at
first, but in later runs did check back on such folios after, and
verified that they had moved on to being freed and reused, not leaked.

The examples I've seen have all been swapbacked, though that may
just reflect my tmpfs swapping load.  With mapping NULLed, there's
not much to go on: but index 0x7ff5ce103 is likely to have been
anon, and index 0xcff more likely to have been tmpfs.

My vote is simply to delete the warning (and the comment).

Hugh

[  206.559451] page:ffffea0000f6eb40 refcount:2 mapcount:0 mapping:0000000000000000 index:0x7ff5ce103 pfn:0x3dbad
[  206.577039] memcg:ffff888012358000
[  206.584624] flags: 0xffe000000020219(locked|uptodate|dirty|workingset|swapbacked|node=0|zone=0|lastcpupid=0x7ff)
[  206.590603] raw: 0ffe000000020219 dead000000000100 dead000000000122 0000000000000000
[  206.597680] raw: 00000007ff5ce103 0000000000000000 00000002ffffffff ffff888012358000
[  206.604406] page dumped because: VM_WARN_ON_FOLIO(true)
[  206.617553] WARNING: CPU: 3 PID: 17918 at mm/vmscan.c:699 shrink_folio_list+0x87a/0xbb0
[  206.623211] CPU: 3 UID: 1000 PID: 17918 Comm: fixdep Not tainted 6.17.0-rc4-m19 #3 PREEMPT(full) 
[  206.634664] RIP: 0010:shrink_folio_list+0x87a/0xbb0

[ 2023.016300] page:ffffea0000d2f640 refcount:2 mapcount:0 mapping:0000000000000000 index:0x7f5c34549 pfn:0x34bd9
[ 2023.031613] memcg:ffff888012358000
[ 2023.049601] flags: 0xffe000000020019(locked|uptodate|dirty|swapbacked|node=0|zone=0|lastcpupid=0x7ff)
[ 2023.059626] raw: 0ffe000000020019 dead000000000100 dead000000000122 0000000000000000
[ 2023.073965] raw: 00000007f5c34549 0000000000000000 00000002ffffffff ffff888012358000
[ 2023.091613] page dumped because: VM_WARN_ON_FOLIO(true)
[ 2023.111727] WARNING: CPU: 4 PID: 30543 at mm/vmscan.c:699 shrink_folio_list+0x87a/0xbb0
[ 2023.111749] CPU: 4 UID: 1000 PID: 30543 Comm: cc1 Tainted: G        W           6.17.0-rc4-m19 #3 PREEMPT(full) 
[ 2023.111761] RIP: 0010:shrink_folio_list+0x87a/0xbb0

[ 7604.465422] page:ffffea000092c480 refcount:2 mapcount:0 mapping:0000000000000000 index:0xcff pfn:0x24b12
[ 7604.488416] memcg:ffff888015410000
[ 7604.506284] flags: 0xffe000000020019(locked|uptodate|dirty|swapbacked|node=0|zone=0|lastcpupid=0x7ff)
[ 7604.528118] raw: 0ffe000000020019 dead000000000100 dead000000000122 0000000000000000
[ 7604.564338] raw: 0000000000000cff 0000000000000000 00000002ffffffff ffff888015410000
[ 7604.564790] page dumped because: VM_WARN_ON_FOLIO(true)
[ 7604.564822] WARNING: CPU: 5 PID: 78 at mm/vmscan.c:699 shrink_folio_list+0x87a/0xbb0
[ 7604.564852] CPU: 5 UID: 0 PID: 78 Comm: kswapd0 Not tainted 6.17.0-rc4-m19 #4 PREEMPT(full) 
[ 7604.564871] RIP: 0010:shrink_folio_list+0x87a/0xbb0

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

[Baolin Wang]

On 2025/9/22 13:32, Hugh Dickins wrote:
> On Fri, 19 Sep 2025, Baolin Wang wrote:
>> On 2025/9/19 09:06, Shakeel Butt wrote:
>>> On Thu, Sep 18, 2025 at 05:36:17PM +0800, Baolin Wang wrote:
>>>> On 2025/9/18 14:00, David Hildenbrand wrote:
>>>>> On 18.09.25 05:46, Baolin Wang wrote:
> ...
>>>>>> 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.
>>>>>     */
>>>>
>>>> LGTM.
>>>>
>>>> Andrew, could you help squash these comments into this patch? Thanks.
> 
> (Myself, I would delete the comment entirely: it's justifying the change,
> which is good history to go into the commit message, but not so good in
> the final source. And we don't fully understand what to say here anyway.)
> 
>>>>
>>>>>> -        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);
>>>
>>> Unexpected but better to use VM_WARN_ON_ONCE_FOLIO here.
>>
>> Um, I don't think it makes much difference, because we should no longer hit
>> this.
> 
> We do hit it.  Observe, there was no WARNING before in the !mapping
> case, just a pr_info in a particular instance of the !mapping case.
> We believe that instance went away with reiserfs, but that does not
> imply that there are no more !mapping cases left.
> 
> We do hit that WARNING: not often, and I've not seen it repeatedly
> (as ONCE-advisors fear), but a few cutdown examples from my testing
> appended below.

Thanks for reporting. That surprises me how that happened.

> I'm sure you'd like me to explain how it comes about: I did spend a
> while looking to do so, but then found better things to do.  By all
> means go in search of the explanation if it's worth your time: it
> might indicate a bug somewhere; but more likely it's just a race
> against code elsewhere cleaning up the folio.

Interesting. I'll try to reproduce this issue.

> The fact that it does not appear repeatedly suggests that the folio
> is successfully dealt with afterwards.  I didn't think to check at
> first, but in later runs did check back on such folios after, and
> verified that they had moved on to being freed and reused, not leaked.

No leaks, good.

> The examples I've seen have all been swapbacked, though that may
> just reflect my tmpfs swapping load.  With mapping NULLed, there's
> not much to go on: but index 0x7ff5ce103 is likely to have been
> anon, and index 0xcff more likely to have been tmpfs.
> 
> My vote is simply to delete the warning (and the comment).

Thanks for your examples and sound reasonable to me.

Andrew, could you help squash the following changes (if you need a new 
version, please let me know)? Thanks.


diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4907e255857a..aadbee50a851 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -689,16 +689,8 @@ static pageout_t pageout(struct folio *folio, 
struct address_space *mapping,
          * A freeable shmem or swapcache folio is referenced only by the
          * caller that isolated the folio and the page cache.
          */
-       if (folio_ref_count(folio) != 1 + folio_nr_pages(folio))
+       if (folio_ref_count(folio) != 1 + folio_nr_pages(folio) || !mapping)
                 return PAGE_KEEP;
-       if (!mapping) {
-               /*
-                * We should no longer have dirty folios with clean 
buffers and
-                * a NULL mapping. However, let's be careful for now.
-                */
-               VM_WARN_ON_FOLIO(true, folio);
-               return PAGE_KEEP;
-       }

         if (!shmem_mapping(mapping) && !folio_test_anon(folio))
                 return PAGE_ACTIVATE;

[PATCH v2 2/2] mm: vmscan: simplify the folio refcount check in pageout()

[Baolin Wang]

Since we no longer attempt to write back filesystem folios in pageout()
(they will be filtered out by the following check in pageout()), and only
tmpfs/shmem folios and anonymous swapcache folios can be written back,
we can remove the redundant folio_test_private() when checking the folio's
refcount, as tmpfs/shmem and swapcache folios do not use the PG_private flag.

While we're at it, we can open-code the folio refcount check instead of
adding a simple helper that has only one user.

Acked-by: David Hildenbrand <david@redhat.com>
Acked-by: Shakeel Butt <shakeel.butt@linux.dev>
Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
---
 mm/vmscan.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 930add6d90ab..b3a57db6a445 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -477,17 +477,6 @@ static int reclaimer_offset(struct scan_control *sc)
 	return PGSTEAL_DIRECT - PGSTEAL_KSWAPD;
 }
 
-static inline int is_page_cache_freeable(struct folio *folio)
-{
-	/*
-	 * A freeable page cache folio is referenced only by the caller
-	 * that isolated the folio, the page cache and optional filesystem
-	 * private data at folio->private.
-	 */
-	return folio_ref_count(folio) - folio_test_private(folio) ==
-		1 + folio_nr_pages(folio);
-}
-
 /*
  * We detected a synchronous write error writing a folio out.  Probably
  * -ENOSPC.  We need to propagate that into the address_space for a subsequent
@@ -696,8 +685,11 @@ static pageout_t pageout(struct folio *folio, struct address_space *mapping,
 	 * block, for some throttling. This happens by accident, because
 	 * swap_backing_dev_info is bust: it doesn't reflect the
 	 * congestion state of the swapdevs.  Easy to fix, if needed.
+	 *
+	 * A freeable shmem or swapcache folio is referenced only by the
+	 * caller that isolated the folio and the page cache.
 	 */
-	if (!is_page_cache_freeable(folio))
+	if (folio_ref_count(folio) != 1 + folio_nr_pages(folio))
 		return PAGE_KEEP;
 	if (!mapping) {
 		/*
-- 
2.43.7