Re: [PATCH v2 1/4] mm/vmscan: remove the PageDirty check after MADV_FREE pages are page_ref_freezed

From: Yu Zhao <yuzhao@google.com>
To: Miaohe Lin <linmiaohe@huawei.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Johannes Weiner <hannes@cmpxchg.org>,
	 Vlastimil Babka <vbabka@suse.cz>, Michal Hocko <mhocko@suse.com>,
	Jens Axboe <axboe@kernel.dk>,
	 Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Alex Shi <alexs@kernel.org>,
	apopple@nvidia.com,  Matthew Wilcox <willy@infradead.org>,
	Minchan Kim <minchan@kernel.org>,
	 David Hildenbrand <david@redhat.com>,
	shli@fb.com, hillf.zj@alibaba-inc.com,
	 John Hubbard <jhubbard@nvidia.com>,
	Linux-MM <linux-mm@kvack.org>,
	 linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/4] mm/vmscan: remove the PageDirty check after MADV_FREE pages are page_ref_freezed
Date: Sat, 17 Jul 2021 18:58:29 -0600	[thread overview]
Message-ID: <CAOUHufbfXCDwhNyZ9gAgu-1jXygfN5sTahwaSOr0vo6zZ0uCVg@mail.gmail.com> (raw)
In-Reply-To: <20210717065911.61497-2-linmiaohe@huawei.com>

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

On Sat, Jul 17, 2021 at 12:59 AM Miaohe Lin <linmiaohe@huawei.com> wrote:

> If the MADV_FREE pages are redirtied before they could be reclaimed,
> put the pages back to anonymous LRU list by setting SwapBacked flag
> and the pages will be reclaimed in normal swapout way. But as Yu Zhao
> pointed out, "The page has only one reference left, which is from the
> isolation. After the caller puts the page back on lru and drops the
> reference, the page will be freed anyway. It doesn't matter which lru
> it goes." So we don't bother checking PageDirty here.
>
> [Yu Zhao's comment is also quoted in the code.]
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>

Reviewed-by: Yu Zhao <yuzhao@google.com>

> ---
>  mm/vmscan.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index a7602f71ec04..92a515e82b1b 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1627,11 +1627,14 @@ static unsigned int shrink_page_list(struct
> list_head *page_list,
>                         /* follow __remove_mapping for reference */
>                         if (!page_ref_freeze(page, 1))
>                                 goto keep_locked;
> -                       if (PageDirty(page)) {
> -                               page_ref_unfreeze(page, 1);
> -                               goto keep_locked;
> -                       }
> -
> +                       /*
> +                        * The page has only one reference left, which is
> +                        * from the isolation. After the caller puts the
> +                        * page back on lru and drops the reference, the
> +                        * page will be freed anyway. It doesn't matter
> +                        * which lru it goes. So we don't bother checking
> +                        * PageDirty here.
> +                        */
>                         count_vm_event(PGLAZYFREED);
>                         count_memcg_page_event(page, PGLAZYFREED);
>                 } else if (!mapping || !__remove_mapping(mapping, page,
> true,
> --
> 2.23.0
>
>

[-- Attachment #2: Type: text/html, Size: 3061 bytes --]