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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 38130ECAAA1 for ; Mon, 5 Sep 2022 06:53:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236267AbiIEGxv (ORCPT ); Mon, 5 Sep 2022 02:53:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49812 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236568AbiIEGxq (ORCPT ); Mon, 5 Sep 2022 02:53:46 -0400 Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3ECAE3C15A for ; Sun, 4 Sep 2022 23:53:44 -0700 (PDT) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.53]) by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4MLfKG1c7yzlWs6; Mon, 5 Sep 2022 14:50:10 +0800 (CST) Received: from [10.174.177.76] (10.174.177.76) by canpemm500002.china.huawei.com (7.192.104.244) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Mon, 5 Sep 2022 14:53:41 +0800 Subject: Re: [PATCH 2/6] mm, hwpoison: use __PageMovable() to detect non-lru movable pages To: =?UTF-8?B?SE9SSUdVQ0hJIE5BT1lBKOWggOWPoyDnm7TkuZ8p?= CC: "akpm@linux-foundation.org" , "linux-mm@kvack.org" , "linux-kernel@vger.kernel.org" References: <20220830123604.25763-1-linmiaohe@huawei.com> <20220830123604.25763-3-linmiaohe@huawei.com> <20220905052243.GA1355682@hori.linux.bs1.fc.nec.co.jp> From: Miaohe Lin Message-ID: <1f7ee86e-7d28-0d8c-e0de-b7a5a94519e8@huawei.com> Date: Mon, 5 Sep 2022 14:53:41 +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: <20220905052243.GA1355682@hori.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.76] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2022/9/5 13:22, HORIGUCHI NAOYA(堀口 直也) wrote: > Hi Miaohe, > > On Tue, Aug 30, 2022 at 08:36:00PM +0800, Miaohe Lin wrote: >> It's more recommended to use __PageMovable() to detect non-lru movable >> pages. We can avoid bumping page refcnt via isolate_movable_page() for >> the isolated lru pages. Also if pages become PageLRU just after they're >> checked but before trying to isolate them, isolate_lru_page() will be >> called to do the right work. > > Good point, non-lru movable page is currently handled by isolate_lru_page(), > which always fails. This means that we lost the chance of soft-offlining > for any non-lru movable page. So this patch improves the situation. Non-lru movable page will still be handled by isolate_movable_page() before the code change as they don't have PageLRU set. The current situation is that the isolated LRU pages are passed to isolate_movable_page() uncorrectly. This might not hurt. But the chance that pages become un-isolated and thus available just after checking could be seized with this patch. > >> >> Signed-off-by: Miaohe Lin >> --- >> mm/memory-failure.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/memory-failure.c b/mm/memory-failure.c >> index a923a6dde871..3966fa6abe03 100644 >> --- a/mm/memory-failure.c >> +++ b/mm/memory-failure.c >> @@ -2404,7 +2404,7 @@ EXPORT_SYMBOL(unpoison_memory); >> static bool isolate_page(struct page *page, struct list_head *pagelist) >> { >> bool isolated = false; >> - bool lru = PageLRU(page); >> + bool lru = !__PageMovable(page); > > It seems that PAGE_MAPPING_MOVABLE is not set for hugetlb pages, so > lru becomes true for them. Then, if isolate_hugetlb() succeeds, > inc_node_page_state() is called for hugetlb pages, maybe that's not expected. Yes, that's unexpected. Thanks for pointing this out. > >> >> if (PageHuge(page)) { >> isolated = !isolate_hugetlb(page, pagelist); > } else { > if (lru) > isolated = !isolate_lru_page(page); > else > isolated = !isolate_movable_page(page, ISOLATE_UNEVICTABLE); > > if (isolated) > list_add(&page->lru, pagelist); > } > > if (isolated && lru) > inc_node_page_state(page, NR_ISOLATED_ANON + > page_is_file_lru(page)); > > so, how about moving this if block into the above else block? > Then, the automatic variable lru can be moved into the else block. Do you mean something like below? diff --git a/mm/memory-failure.c b/mm/memory-failure.c index df3bf266eebf..48780f3a61d3 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -2404,24 +2404,25 @@ EXPORT_SYMBOL(unpoison_memory); static bool isolate_page(struct page *page, struct list_head *pagelist) { bool isolated = false; - bool lru = !__PageMovable(page); if (PageHuge(page)) { isolated = !isolate_hugetlb(page, pagelist); } else { + bool lru = !__PageMovable(page); + if (lru) isolated = !isolate_lru_page(page); else isolated = !isolate_movable_page(page, ISOLATE_UNEVICTABLE); - if (isolated) + if (isolated) { list_add(&page->lru, pagelist); + if (lru) + inc_node_page_state(page, NR_ISOLATED_ANON + + page_is_file_lru(page)); + } } - if (isolated && lru) - inc_node_page_state(page, NR_ISOLATED_ANON + - page_is_file_lru(page)); - /* * If we succeed to isolate the page, we grabbed another refcount on * the page, so we can safely drop the one we got from get_any_pages(). > > Thanks, > Naoya Horiguchi Thanks a lot for your review and comment on this series, Naoya. Thanks, Miaohe Lin.