[PATCH RESEND] mm/vmscan: remove redundant folio_test_swapbacked()

[PATCH RESEND] mm/vmscan: remove redundant folio_test_swapbacked()

[Jialin Wang]

When !folio_is_file_lru(folio) is false, it implies that
!folio_test_swapbacked(folio) must be true. Therefore, the additional
check for !folio_test_swapbacked(folio) is redundant and can be safely
removed.

This cleanup simplifies the code without changing any functionality.

Signed-off-by: Jialin Wang <wjl.linux@gmail.com>
---
 mm/vmscan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 7de11524a936..9d4745ad5e23 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -985,8 +985,7 @@ static void folio_check_dirty_writeback(struct folio *folio,
 	 * They could be mistakenly treated as file lru. So further anon
 	 * test is needed.
 	 */
-	if (!folio_is_file_lru(folio) ||
-	    (folio_test_anon(folio) && !folio_test_swapbacked(folio))) {
+	if (!folio_is_file_lru(folio) || folio_test_anon(folio)) {
 		*dirty = false;
 		*writeback = false;
 		return;
-- 
2.50.0

Re: [PATCH RESEND] mm/vmscan: remove redundant folio_test_swapbacked()

[Harry Yoo]

This is marked as RESEND, but which patch is it a resend of?
I can’t find the original one.

On Tue, Aug 12, 2025 at 02:25:00AM +0800, Jialin Wang wrote:
> When !folio_is_file_lru(folio) is false, it implies that
> !folio_test_swapbacked(folio) must be true.

That is not true.

MADV_FREE pages are anonymous pages that are not swapbacked
(and thus can be reclaimed without pageout if they are clean).

See below commit that added the condition and the patch series
that introduced it:
https://lore.kernel.org/all/cover.1487965799.git.shli@fb.com/

commit 802a3a92ad7ac0b9be9df229dee530a1f0a8039b
Author: Shaohua Li <shli@fb.com>
Date:   Wed May 3 14:52:32 2017 -0700

    mm: reclaim MADV_FREE pages

    When memory pressure is high, we free MADV_FREE pages.  If the pages are
    not dirty in pte, the pages could be freed immediately.  Otherwise we
    can't reclaim them.  We put the pages back to anonumous LRU list (by
    setting SwapBacked flag) and the pages will be reclaimed in normal
    swapout way.

    We use normal page reclaim policy.  Since MADV_FREE pages are put into
    inactive file list, such pages and inactive file pages are reclaimed
    according to their age.  This is expected, because we don't want to
    reclaim too many MADV_FREE pages before used once pages.

    Based on Minchan's original patch

    [minchan@kernel.org: clean up lazyfree page handling]
      Link: http://lkml.kernel.org/r/20170303025237.GB3503@bbox
    Link: http://lkml.kernel.org/r/14b8eb1d3f6bf6cc492833f183ac8c304e560484.1487965799.git.shli@fb.com
    Signed-off-by: Shaohua Li <shli@fb.com>
    Signed-off-by: Minchan Kim <minchan@kernel.org>
    Acked-by: Minchan Kim <minchan@kernel.org>
    Acked-by: Michal Hocko <mhocko@suse.com>
    Acked-by: Johannes Weiner <hannes@cmpxchg.org>
    Acked-by: Hillf Danton <hillf.zj@alibaba-inc.com>
    Cc: Hugh Dickins <hughd@google.com>
    Cc: Rik van Riel <riel@redhat.com>
    Cc: Mel Gorman <mgorman@techsingularity.net>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

-- 
Cheers,
Harry / Hyeonggon

> Therefore, the additional
> check for !folio_test_swapbacked(folio) is redundant and can be safely
> removed.
> 
> This cleanup simplifies the code without changing any functionality.
> 
> Signed-off-by: Jialin Wang <wjl.linux@gmail.com>
> ---
>  mm/vmscan.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 7de11524a936..9d4745ad5e23 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -985,8 +985,7 @@ static void folio_check_dirty_writeback(struct folio *folio,
>  	 * They could be mistakenly treated as file lru. So further anon
>  	 * test is needed.
>  	 */
> -	if (!folio_is_file_lru(folio) ||
> -	    (folio_test_anon(folio) && !folio_test_swapbacked(folio))) {
> +	if (!folio_is_file_lru(folio) || folio_test_anon(folio)) {
>  		*dirty = false;
>  		*writeback = false;
>  		return;
> -- 
> 2.50.0
> 
> 

Re: [PATCH RESEND] mm/vmscan: remove redundant folio_test_swapbacked()

[Jialin Wang]

On Tue, Aug 12, 2025 at 2:53 AM Harry Yoo <harry.yoo@oracle.com> wrote:
>
> This is marked as RESEND, but which patch is it a resend of?
> I can’t find the original one.
>
I sent the original one to the wrong mailing list and missed proper cc's, sorry.
https://lore.kernel.org/lkml/20250811181839.40336-1-wjl.linux@gmail.com/

> On Tue, Aug 12, 2025 at 02:25:00AM +0800, Jialin Wang wrote:
> > When !folio_is_file_lru(folio) is false, it implies that
> > !folio_test_swapbacked(folio) must be true.
>
> That is not true.
>
This is the definition of folio_is_file_lru() in
include/linux/mm_inline.h line 28:
static inline int folio_is_file_lru(struct folio *folio)
{
return !folio_test_swapbacked(folio);
}

> MADV_FREE pages are anonymous pages that are not swapbacked
> (and thus can be reclaimed without pageout if they are clean).
>
Thanks for the explanation! I'm new to memory management, so this is
really helpful for me to learn.

Best regards,
Jialin Wang

Re: [PATCH RESEND] mm/vmscan: remove redundant folio_test_swapbacked()

[Harry Yoo]

On Tue, Aug 12, 2025 at 03:21:12AM +0800, Jialin Wang wrote:
> On Tue, Aug 12, 2025 at 2:53 AM Harry Yoo <harry.yoo@oracle.com> wrote:
> >
> > This is marked as RESEND, but which patch is it a resend of?
> > I can’t find the original one.
> >
> I sent the original one to the wrong mailing list and missed proper cc's, sorry.
> https://lore.kernel.org/lkml/20250811181839.40336-1-wjl.linux@gmail.com/

Ahh I see. I couldn't find it because it was not sent to linux-mm.
I assumed you sent it a while ago but didn't get any feedback.

> > On Tue, Aug 12, 2025 at 02:25:00AM +0800, Jialin Wang wrote:
> > > When !folio_is_file_lru(folio) is false, it implies that
> > > !folio_test_swapbacked(folio) must be true.
> >
> > That is not true.

Oops, I completely misread the message and really sorry for that.

I thought you're saying "anonymous pages are always swapbacked",
(without considering ! in the expression), but you weren't saying that.
Probably I shouldn't have read it so late at night ;) 

> This is the definition of folio_is_file_lru() in
> include/linux/mm_inline.h line 28:
> static inline int folio_is_file_lru(struct folio *folio)
> {
> return !folio_test_swapbacked(folio);
> }

You're right that technically this can be simplified, but I see Miaohe
tried this exactly same simplication a few years ago and it got some
negative feedbacks from people as it makes it harder to read the code.

https://lore.kernel.org/linux-mm/87lewqbpad.fsf@yhuang6-desk2.ccr.corp.intel.com

So maybe not worth it?

> > MADV_FREE pages are anonymous pages that are not swapbacked
> > (and thus can be reclaimed without pageout if they are clean).
> >
> Thanks for the explanation! I'm new to memory management, so this is
> really helpful for me to learn.

You're welcome!

> Best regards,
> Jialin Wang

-- 
Cheers,
Harry / Hyeonggon