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=-9.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 E6E5CC4741F for ; Fri, 6 Nov 2020 23:15:52 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 2F59A2087E for ; Fri, 6 Nov 2020 23:15:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="a1e1CoOT" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2F59A2087E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2B1016B005C; Fri, 6 Nov 2020 18:15:51 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2624E6B005D; Fri, 6 Nov 2020 18:15:51 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 177336B006C; Fri, 6 Nov 2020 18:15:51 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0246.hostedemail.com [216.40.44.246]) by kanga.kvack.org (Postfix) with ESMTP id D745E6B005C for ; Fri, 6 Nov 2020 18:15:50 -0500 (EST) Received: from smtpin05.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 863D18249980 for ; Fri, 6 Nov 2020 23:15:50 +0000 (UTC) X-FDA: 77455552860.05.eggs20_4b01907272d6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin05.hostedemail.com (Postfix) with ESMTP id 622361826B6AB for ; Fri, 6 Nov 2020 23:15:50 +0000 (UTC) X-HE-Tag: eggs20_4b01907272d6 X-Filterd-Recvd-Size: 10799 Received: from mail-ed1-f67.google.com (mail-ed1-f67.google.com [209.85.208.67]) by imf43.hostedemail.com (Postfix) with ESMTP for ; Fri, 6 Nov 2020 23:15:49 +0000 (UTC) Received: by mail-ed1-f67.google.com with SMTP id y15so2744141ede.11 for ; Fri, 06 Nov 2020 15:15:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=Ai8EmKDHyiLBVZh++sfBm1AUReXQa1dAN4Sl9vvvSDA=; b=a1e1CoOTUklIGwbuJOs2oxCqSMI1pORvNQka6iht4Ejy6vsvYudNfLr04WXM4PguS7 dgFUHgBlXJcPtGS4uNtPSWB52gxCAkXb6rAHIkxls3yWgtiyt+Okp7Tkma4f3hO/2NjH 6cFqdycCyrt7FTswfBrVzkC40CxUpNpLyqODt47Gj1ebW7xP1qV3KdEGqKP5STwZF4Q/ S0cUVgEvEeswBUvEawVzNwOF3Ic4il3FhDsxQRkT2p7+hP4OkFmvNvlifxspZZZuETbz P6uaLmIryLVFTA/h4I5gG+Ku7qBqu34upTJnipxpxNu8sP8wmyHWWaKorHCaznzJaT+Z Upqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=Ai8EmKDHyiLBVZh++sfBm1AUReXQa1dAN4Sl9vvvSDA=; b=ekyD9W3cSFkm2PcWsWssK8D2sZs2FFpUqqO0miLGGMZmwo2HOmvaDO0fCQPX3fX9XD 5Ic5uGC1Y3DDZsbQaFc4TJOU10n7zVsta8tU/CIWNpbGzO3SzOzjlC13Dvl5QH1NVgkO neO+yDEaIZYvH+JdaYxgWQccVBVblcExvy4B1iqRgwjuQ+Ya1gNAY6vH3sio1JbwCvXi ZcQotKTjT12LJDbYOZA1tUlzxfjubPcsty3Pv9EFguHqZw4XQ0fxsJlHWggpo3QXcR7U 1nKYquTshZCfrhN5RSTB5cLEGglySVxbp3/yH4U+qc/B/3JB6c99j1SsI205nOybS43i Uo1Q== X-Gm-Message-State: AOAM531ClgDoE4Qe1pEfumzgM3ZmkXN9eUahMojoJOiTERh4qcBts29i gJ3MWQJ4BDiHvMJtgdvRqFCnqm6pVTDOVuWxNc4= X-Google-Smtp-Source: ABdhPJySZotRc2Ycq94PJUT63mDsWNbcJBmv0qYea980pivG0amtvZ2eeHdjGY8jQWLiQK7jSYldswu3sJFKLWK1Q10= X-Received: by 2002:aa7:cdd9:: with SMTP id h25mr4616889edw.294.1604704548633; Fri, 06 Nov 2020 15:15:48 -0800 (PST) MIME-Version: 1.0 References: <20201103130334.13468-1-shy828301@gmail.com> <20201103130334.13468-6-shy828301@gmail.com> In-Reply-To: From: Yang Shi Date: Fri, 6 Nov 2020 15:15:36 -0800 Message-ID: Subject: Re: [PATCH 5/5] mm: migrate: return -ENOSYS if THP migration is unsupported To: Zi Yan Cc: Michal Hocko , Song Liu , Mel Gorman , Jan Kara , Andrew Morton , Linux MM , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" 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 Fri, Nov 6, 2020 at 2:02 PM Yang Shi wrote: > > On Fri, Nov 6, 2020 at 12:17 PM Zi Yan wrote: > > > > On 3 Nov 2020, at 8:03, Yang Shi wrote: > > > > > In the current implementation unmap_and_move() would return -ENOMEM i= f > > > THP migration is unsupported, then the THP will be split. If split i= s > > > failed just exit without trying to migrate other pages. It doesn't m= ake > > > too much sense since there may be enough free memory to migrate other > > > pages and there may be a lot base pages on the list. > > > > > > Return -ENOSYS to make consistent with hugetlb. And if THP split is > > > failed just skip and try other pages on the list. > > > > > > Just skip the whole list and exit when free memory is really low. > > > > > > Signed-off-by: Yang Shi > > > --- > > > mm/migrate.c | 62 ++++++++++++++++++++++++++++++++++++++------------= -- > > > 1 file changed, 46 insertions(+), 16 deletions(-) > > > > > > diff --git a/mm/migrate.c b/mm/migrate.c > > > index 8f6a61c9274b..b3466d8c7f03 100644 > > > --- a/mm/migrate.c > > > +++ b/mm/migrate.c > > > @@ -1172,7 +1172,7 @@ static int unmap_and_move(new_page_t get_new_pa= ge, > > > struct page *newpage =3D NULL; > > > > > > if (!thp_migration_supported() && PageTransHuge(page)) > > > - return -ENOMEM; > > > + return -ENOSYS; > > > > > > if (page_count(page) =3D=3D 1) { > > > /* page was freed from under us. So we are done. */ > > > @@ -1376,6 +1376,20 @@ static int unmap_and_move_huge_page(new_page_t= get_new_page, > > > return rc; > > > } > > > > > > +static inline int try_split_thp(struct page *page, struct page *page= 2, > > > + struct list_head *from) > > > +{ > > > + int rc =3D 0; > > > + > > > + lock_page(page); > > > + rc =3D split_huge_page_to_list(page, from); > > > + unlock_page(page); > > > + if (!rc) > > > + list_safe_reset_next(page, page2, lru); > > > > This does not work as expected, right? After macro expansion, we have > > page2 =3D list_next_entry(page, lru). Since page2 is passed as a pointe= r, the change > > does not return back the caller. You need to use the pointer to page2 h= ere. > > > > > + > > > + return rc; > > > +} > > > + > > > /* > > > * migrate_pages - migrate the pages specified in a list, to the fre= e pages > > > * supplied as the target for the page migration > > > @@ -1445,24 +1459,40 @@ int migrate_pages(struct list_head *from, new= _page_t get_new_page, > > > reason, &ret_pages); > > > > > > switch(rc) { > > > + /* > > > + * THP migration might be unsupported or the > > > + * allocation could've failed so we should > > > + * retry on the same page with the THP split > > > + * to base pages. > > > + * > > > + * Head page is retried immediately and tail > > > + * pages are added to the tail of the list so > > > + * we encounter them after the rest of the list > > > + * is processed. > > > + */ > > > + case -ENOSYS: > > > + /* THP migration is unsupported */ > > > + if (is_thp) { > > > + if (!try_split_thp(page, page2,= from)) { > > > + nr_thp_split++; > > > + goto retry; > > > + } > > > + > > > + nr_thp_failed++; > > > + nr_failed +=3D nr_subpages; > > > + break; > > > + } > > > + > > > + /* Hugetlb migration is unsupported */ > > > + nr_failed++; > > > + break; > > > case -ENOMEM: > > > /* > > > - * THP migration might be unsupported o= r the > > > - * allocation could've failed so we sho= uld > > > - * retry on the same page with the THP = split > > > - * to base pages. > > > - * > > > - * Head page is retried immediately and= tail > > > - * pages are added to the tail of the l= ist so > > > - * we encounter them after the rest of = the list > > > - * is processed. > > > + * When memory is low, don't bother to = try to migrate > > > + * other pages, just exit. > > > > The comment does not match the code below. For THPs, the code tries to = split the THP > > and migrate the base pages if the split is successful. > > BTW, actually I don't think -ENOSYS is a proper return value for this > case since it typically means "the syscall doesn't exist". IMHO, it > should return -EINVAL. And actually the return value doesn't matter > since all callers of migrate_pages() just check if the return value !=3D > 0. And, neither -ENOSYS nor -EINVAL won't be visible by userspace > since migrate_pages() just returns the number of failed pages for this > case. > > Anyway the return value is a little bit messy, it may return -ENOMEM, > 0 or nr_failed. But it looks the callers just care about if ret !=3D 0, > so it may be better to let it return nr_failed for -ENOMEM case too. By relooking all the callsites, it seems I overlooked two cases: * compaction: it checks if the return value is -ENOMEM if so it may return COMPACT_CONTENDED * migrate_pages syscall: it may return -ENOMEM to the userspace, but the man page doesn't document this case, not sure if it is legitimate > > > > > > */ > > > if (is_thp) { > > > - lock_page(page); > > > - rc =3D split_huge_page_to_list(= page, from); > > > - unlock_page(page); > > > - if (!rc) { > > > - list_safe_reset_next(pa= ge, page2, lru); > > > + if (!try_split_thp(page, page2,= from)) { > > > nr_thp_split++; > > > goto retry; > > > } > > > @@ -1490,7 +1520,7 @@ int migrate_pages(struct list_head *from, new_p= age_t get_new_page, > > > break; > > > default: > > > /* > > > - * Permanent failure (-EBUSY, -ENOSYS, = etc.): > > > + * Permanent failure (-EBUSY, etc.): > > > * unlike -EAGAIN case, the failed page= is > > > * removed from migration page list and= not > > > * retried in the next outer loop. > > > -- > > > 2.26.2 > > > > > > =E2=80=94 > > Best Regards, > > Yan Zi