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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D135C433F5 for ; Thu, 26 May 2022 01:53:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 875458D0003; Wed, 25 May 2022 21:53:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8238C8D0001; Wed, 25 May 2022 21:53:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 710828D0003; Wed, 25 May 2022 21:53:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 5E4D48D0001 for ; Wed, 25 May 2022 21:53:21 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay12.hostedemail.com (Postfix) with ESMTP id 2BE021201EE for ; Thu, 26 May 2022 01:53:21 +0000 (UTC) X-FDA: 79506221802.07.0F08AA7 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf18.hostedemail.com (Postfix) with ESMTP id 062AC1C0012 for ; Thu, 26 May 2022 01:53:02 +0000 (UTC) Received: from canpemm500002.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4L7rXw1mvHzjX5h; Thu, 26 May 2022 09:52:32 +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; Thu, 26 May 2022 09:53:15 +0800 Subject: Re: [PATCH v3 3/4] mm/migration: return errno when isolate_huge_page failed To: Oscar Salvador CC: , , , , , , , , , , , , References: <20220525081822.53547-1-linmiaohe@huawei.com> <20220525081822.53547-4-linmiaohe@huawei.com> From: Miaohe Lin Message-ID: <5ed70fbc-c582-6e85-22bc-3ccafa0d7a3f@huawei.com> Date: Thu, 26 May 2022 09:53:15 +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.177.76] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To canpemm500002.china.huawei.com (7.192.104.244) X-CFilter-Loop: Reflected X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 062AC1C0012 X-Stat-Signature: iwbkmngsqipjfdasxm5ioi6hzq317thw X-Rspam-User: Authentication-Results: imf18.hostedemail.com; dkim=none; dmarc=pass (policy=quarantine) header.from=huawei.com; spf=pass (imf18.hostedemail.com: domain of linmiaohe@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=linmiaohe@huawei.com X-HE-Tag: 1653529982-727178 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 2022/5/25 16:41, Oscar Salvador wrote: > On Wed, May 25, 2022 at 04:18:21PM +0800, Miaohe Lin wrote: >> We might fail to isolate huge page due to e.g. the page is under migration >> which cleared HPageMigratable. We should return errno in this case rather >> than always return 1 which could confuse the user, i.e. the caller might >> think all of the memory is migrated while the hugetlb page is left behind. >> We make the prototype of isolate_huge_page consistent with isolate_lru_page >> as suggested by Huang Ying and rename isolate_huge_page to isolate_hugetlb >> as suggested by Muchun to improve the readability. >> >> Fixes: e8db67eb0ded ("mm: migrate: move_pages() supports thp migration") >> Suggested-by: Huang Ying >> Signed-off-by: Miaohe Lin > > Looks good to me, one thing below though: > >> --- >> include/linux/hugetlb.h | 6 +++--- >> mm/gup.c | 2 +- >> mm/hugetlb.c | 11 +++++------ >> mm/memory-failure.c | 2 +- >> mm/mempolicy.c | 2 +- >> mm/migrate.c | 5 +++-- >> 6 files changed, 14 insertions(+), 14 deletions(-) >> > ... >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1627,8 +1627,9 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr, >> >> if (PageHuge(page)) { >> if (PageHead(page)) { >> - isolate_huge_page(page, pagelist); >> - err = 1; >> + err = isolate_hugetlb(page, pagelist); >> + if (!err) >> + err = 1; >> } > > We used to always return 1 which means page has been queued for migration, as we > did not check isolate_huge_page() return value. > Now, we either return 1 or 0 depending on isolate_hugetlb(). Return 1 or -EBUSY just as normal page case. > I guess that was fine because in the end, if isolate_huge_page() failed, > the page just does not get added to 'pagelist', right? So, it is just > confusing for the user because he might not get an error so he thinks > the page will be migrated, and yet will not? Yes, the hugetlb page might not be migrated due to error while it's not reported in the __user *status. So the caller might think all of the memory is migrated and thus does not retry to migrate the hugetlb page in the next round. Many thanks for your review and comment! :) > >