From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.6 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6E5BC07E95 for ; Tue, 13 Jul 2021 13:16:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BFD596127C for ; Tue, 13 Jul 2021 13:16:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236450AbhGMNTp (ORCPT ); Tue, 13 Jul 2021 09:19:45 -0400 Received: from szxga08-in.huawei.com ([45.249.212.255]:11271 "EHLO szxga08-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236283AbhGMNTo (ORCPT ); Tue, 13 Jul 2021 09:19:44 -0400 Received: from dggeme703-chm.china.huawei.com (unknown [172.30.72.55]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4GPLcM2LLNz1CJQh; Tue, 13 Jul 2021 21:11:15 +0800 (CST) Received: from [10.174.178.125] (10.174.178.125) by dggeme703-chm.china.huawei.com (10.1.199.99) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Tue, 13 Jul 2021 21:16:51 +0800 Subject: Re: [PATCH 1/5] mm/vmscan: put the redirtied MADV_FREE pages back to anonymous LRU list To: Yu Zhao CC: Andrew Morton , Johannes Weiner , Vlastimil Babka , Michal Hocko , Jens Axboe , Joonsoo Kim , Alex Shi , , Matthew Wilcox , Minchan Kim , David Hildenbrand , , , Linux-MM , linux-kernel References: <20210710100329.49174-1-linmiaohe@huawei.com> <20210710100329.49174-2-linmiaohe@huawei.com> <022f2f7c-fc03-182a-1f8f-4b77c0731d4f@huawei.com> From: Miaohe Lin Message-ID: Date: Tue, 13 Jul 2021 21:16:50 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.174.178.125] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To dggeme703-chm.china.huawei.com (10.1.199.99) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2021/7/13 15:25, Yu Zhao wrote: > On Mon, Jul 12, 2021 at 1:12 AM Miaohe Lin wrote: >> >> On 2021/7/11 7:22, Yu Zhao wrote: >>> On Sat, Jul 10, 2021 at 4:03 AM Miaohe Lin 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. Otherwise MADV_FREE pages >>>> won't be reclaimed as expected. >>>> >>>> Fixes: 802a3a92ad7a ("mm: reclaim MADV_FREE pages") >>> >>> This is not a bug -- the dirty check isn't needed but it was copied >>> from __remove_mapping(). >> >> Yes, this is not a bug and harmless. When we reach here, page should not be >> dirtied because PageDirty is handled above and there is no way to redirty it >> again as pagetable references are all gone and it's not in the swap cache. >> >>> >>> 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. >> >> But it looks buggy as it didn't perform the expected ops from code view. >> Should I drop the Fixes tag and send a v2 version? > > I don't understand the logic here -- it looks pretty obvious to me > that, if we want to change anything, we should delete the dirty check, > not add another line that would enforce the belief that the dirty > check is needed. > The dirty check could be removed even with the page_ref_freeze check because no one can grab the page refcnt after the page is successfully unmapped. Does the change below makes sense for you? Many Thanks. diff --git a/mm/vmscan.c b/mm/vmscan.c index 6e26b3c93242..c31925320b33 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c @@ -1624,15 +1624,11 @@ static unsigned int shrink_page_list(struct list_head *page_list, } if (PageAnon(page) && !PageSwapBacked(page)) { - /* follow __remove_mapping for reference */ - if (!page_ref_freeze(page, 1)) - goto keep_locked; - if (PageDirty(page)) { - SetPageSwapBacked(page); - page_ref_unfreeze(page, 1); - goto keep_locked; - } - + /* + * No one can grab the page refcnt or redirty the page + * after the page is successfully unmapped. + */ + WARN_ON_ONCE(!page_ref_freeze(page, 1)); count_vm_event(PGLAZYFREED); count_memcg_page_event(page, PGLAZYFREED); } else if (!mapping || !__remove_mapping(mapping, page, true, >> >> Many thanks for reply! >> >>> >>>> Signed-off-by: Miaohe Lin >>>> --- >>>> mm/vmscan.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/mm/vmscan.c b/mm/vmscan.c >>>> index a7602f71ec04..6483fe0e2065 100644 >>>> --- a/mm/vmscan.c >>>> +++ b/mm/vmscan.c >>>> @@ -1628,6 +1628,7 @@ static unsigned int shrink_page_list(struct list_head *page_list, >>>> if (!page_ref_freeze(page, 1)) >>>> goto keep_locked; >>>> if (PageDirty(page)) { >>>> + SetPageSwapBacked(page); >>>> page_ref_unfreeze(page, 1); >>>> goto keep_locked; >>>> } >>>> -- >>>> 2.23.0 >>>> >>>> >>> . >>> >> > . >