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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2BED6C433F5 for ; Mon, 1 Nov 2021 06:53:45 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id BBB8B60FD9 for ; Mon, 1 Nov 2021 06:53:44 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org BBB8B60FD9 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 3240B6B00A2; Mon, 1 Nov 2021 02:53:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2D3FB6B00A3; Mon, 1 Nov 2021 02:53:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1C2996B00A4; Mon, 1 Nov 2021 02:53:44 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0110.hostedemail.com [216.40.44.110]) by kanga.kvack.org (Postfix) with ESMTP id 09B596B00A2 for ; Mon, 1 Nov 2021 02:53:44 -0400 (EDT) Received: from smtpin03.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id ADD5D181B04B3 for ; Mon, 1 Nov 2021 06:53:43 +0000 (UTC) X-FDA: 78759445926.03.024E7F5 Received: from out30-42.freemail.mail.aliyun.com (out30-42.freemail.mail.aliyun.com [115.124.30.42]) by imf02.hostedemail.com (Postfix) with ESMTP id 1018D7001A23 for ; Mon, 1 Nov 2021 06:53:36 +0000 (UTC) X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R811e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04357;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=5;SR=0;TI=SMTPD_---0UuSq4su_1635749616; Received: from 30.21.164.3(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0UuSq4su_1635749616) by smtp.aliyun-inc.com(127.0.0.1); Mon, 01 Nov 2021 14:53:37 +0800 Subject: Re: [PATCH] mm: migrate: Correct the hugetlb migration stats To: Zi Yan Cc: akpm@linux-foundation.org, shy828301@gmail.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <677EF981-F33E-4002-AA38-DD669C319284@nvidia.com> From: Baolin Wang Message-ID: <29aa9c6e-7191-71bb-d8a3-e2695b18fa3e@linux.alibaba.com> Date: Mon, 1 Nov 2021 14:54:23 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0 MIME-Version: 1.0 In-Reply-To: <677EF981-F33E-4002-AA38-DD669C319284@nvidia.com> Content-Type: text/plain; charset=utf-8; format=flowed Authentication-Results: imf02.hostedemail.com; dkim=none; dmarc=pass (policy=none) header.from=alibaba.com; spf=pass (imf02.hostedemail.com: domain of baolin.wang@linux.alibaba.com designates 115.124.30.42 as permitted sender) smtp.mailfrom=baolin.wang@linux.alibaba.com X-Stat-Signature: 6zhumtctko3t8iaoiiqsq4gspu1k7pe5 X-Rspamd-Queue-Id: 1018D7001A23 X-Rspamd-Server: rspam01 X-HE-Tag: 1635749616-67662 Content-Transfer-Encoding: quoted-printable 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 2021/10/29 23:43, Zi Yan wrote: > On 29 Oct 2021, at 3:42, Baolin Wang wrote: >=20 >> Now hugetlb migration is also available for some scenarios, such as >> soft offling or memory compaction. So we should correct the migration >=20 > hugetlb migration is available at the time if (PageHuge(page)) branch > is added. I am not sure what is new here. No new things actually, sorry for confusing and will update the commit=20 message in next version. >=20 >> stats for hugetlb with using compound_nr() instead of thp_nr_pages() >> to get the number of pages. >=20 > nr_failed records the number of pages, not subpages. It is returned to I also think nr_failed should record the number of pages, not the number=20 of hugetlb, if I understand you correctly. > user space when move_pages() syscall is used. After your change, > if users try to migrate a list of pages including THPs and/or hugetlb > pages and some of THPs and/or hugetlb fail to migrate, move_pages() > will return a number larger than the number of pages the users tried OK, thanks for pointing out the issue. But before my patch, we've already returned the number of pages=20 successed or failed for THP migration, instead of the number of THP.=20 That means if we just move only 1 page by move_pages() and if this page=20 is 2M THP, so move_pages() will return 512 if failed to migrate, which=20 is larger than the page count specified from user. if (err > 0) err +=3D nr_pages - i - 1; On the other hand, the stats of PGMIGRATE_SUCCESS/PGMIGRATE_FAIL should=20 stand for the number of pages, instead of the number of hugetlb. Also=20 for hugetlb migration when memory compaction, we've already counted the=20 number of pages for a hugetlb into cc->nr_migratepages, if the hugetlb=20 migration failed, the trace stat of compaction will be confusing if we=20 return the number of hugetlb. trace_mm_compaction_migratepages(cc->nr_migratepages, err,=20 &cc->migratepages); So I think the stats of hugetlb migration should be consistent with THP. > to migrate. I am not sure this is the change we want. Or at least, > the comment of migrate_pages() and the manpage of move_pages() need > to be changed and linux-api mailing list should be cc=E2=80=99d. I don't think we should update the comments of migrate_pages(), "Returns=20 the number of pages that were not migrated" makes sense to me if I=20 understand correctly. For the manpage of move_pages(), as you said, the the returned=20 non-migrate page numbers can be larger than the numbers specified from=20 user if failed to migrate a THP or a hugetlb. I am not sure if we should=20 change the manpage, since the THP already did, but I can send a patch to=20 update the manpage if you think this is still necessary. Thanks. >> >> Signed-off-by: Baolin Wang >> --- >> mm/migrate.c | 10 ++++------ >> 1 file changed, 4 insertions(+), 6 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index a11e948..2b45a29 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1475,7 +1475,7 @@ int migrate_pages(struct list_head *from, new_pa= ge_t get_new_page, >> * during migration. >> */ >> is_thp =3D PageTransHuge(page) && !PageHuge(page); >> - nr_subpages =3D thp_nr_pages(page); >> + nr_subpages =3D compound_nr(page); >> cond_resched(); >> >> if (PageHuge(page)) >> @@ -1540,7 +1540,7 @@ int migrate_pages(struct list_head *from, new_pa= ge_t get_new_page, >> nr_failed +=3D nr_subpages; >> goto out; >> } >> - nr_failed++; >> + nr_failed +=3D nr_subpages; >> goto out; >> case -EAGAIN: >> if (is_thp) { >> @@ -1550,14 +1550,14 @@ int migrate_pages(struct list_head *from, new_= page_t get_new_page, >> retry++; >> break; >> case MIGRATEPAGE_SUCCESS: >> + nr_succeeded +=3D nr_subpages; >> if (is_thp) { >> nr_thp_succeeded++; >> - nr_succeeded +=3D nr_subpages; >> break; >> } >> - nr_succeeded++; >> break; >> default: >> + nr_failed +=3D nr_subpages; >> /* >> * Permanent failure (-EBUSY, etc.): >> * unlike -EAGAIN case, the failed page is >> @@ -1566,10 +1566,8 @@ int migrate_pages(struct list_head *from, new_p= age_t get_new_page, >> */ >> if (is_thp) { >> nr_thp_failed++; >> - nr_failed +=3D nr_subpages; >> break; >> } >> - nr_failed++; >> break; >> } >> } >> --=20 >> 1.8.3.1 >=20 > -- > Best Regards, > Yan, Zi >=20