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 autolearn=unavailable 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 BDBFBC55178 for ; Fri, 6 Nov 2020 22:02:16 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 12BF620882 for ; Fri, 6 Nov 2020 22:02:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="NL260D+H" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 12BF620882 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 988156B005D; Fri, 6 Nov 2020 17:02:15 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 9393C6B0068; Fri, 6 Nov 2020 17:02:15 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7DA336B006C; Fri, 6 Nov 2020 17:02:15 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 455916B005D for ; Fri, 6 Nov 2020 17:02:15 -0500 (EST) Received: from smtpin30.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id C6EE2181AEF07 for ; Fri, 6 Nov 2020 22:02:14 +0000 (UTC) X-FDA: 77455367388.30.mist98_1001dde272d6 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin30.hostedemail.com (Postfix) with ESMTP id A21EC180B3AA7 for ; Fri, 6 Nov 2020 22:02:14 +0000 (UTC) X-HE-Tag: mist98_1001dde272d6 X-Filterd-Recvd-Size: 10017 Received: from mail-ed1-f67.google.com (mail-ed1-f67.google.com [209.85.208.67]) by imf22.hostedemail.com (Postfix) with ESMTP for ; Fri, 6 Nov 2020 22:02:13 +0000 (UTC) Received: by mail-ed1-f67.google.com with SMTP id a15so2789230edy.1 for ; Fri, 06 Nov 2020 14:02:13 -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=9tJLva6MxGP0RlF4GQXWhwyd6Ld/npyROEV+eHgx1qU=; b=NL260D+Hdtyhvv37KWi1AuDd8qUNq/YasMfWp6VTY5JdVJq4FSlN8W6hy5jt9gPkQl dyfo9NQ9aFO8I4ysr/fZTyBcTFr62NjApqRkingoWZNWX6OOKds2YuicgLMPFy/AJJis QmLKO/uXOzsxFfQU3jO/kLgEBBqPwnfT8NjUSxQ2nT9cHWjP0cvshnVaejd9v5S6GGmq Cseyj41TSNviHkQ2Qt/DOetiP6jEBIWbt2u06rEkcvKNOq2s+c3UlEX8u/Ei/0RZuGgF J3CZQbD9fhbVBTLpDG6QywOaJ0jgCqkiDkknntf+SFewp5utn10WjkbkUvEAe8L7xY5L FsWA== 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=9tJLva6MxGP0RlF4GQXWhwyd6Ld/npyROEV+eHgx1qU=; b=sWrFMSYUk20U76wLSBI09XRxUeb5feDveKHVHaCXCUoVZqnPjlPUaDG5V5BoNeyGOU NHVMwopWdUS0btkH3GgtQsAHX2R1BFlgxdGeASxSxHGQxHS45Tjzhcx+suYwKPmNH2FP Vh0wwzwoDebol7WTUk0AiOPdkqEMjU2UnGtYTNjORHxGL1bL1dr//IUgK5p8FOevnMpR Oiy7HIWl2UP5jmbAnEumOcAVV+8SeYYO28fXhFq7r4X/d7SGegGULm9iw9B0pGqLrLPW QOM+9rUqdZXxtPnah65qeCZzdEyfUkse+nFHaIxRsNA2ppdUTr1fGSzrfsSR7lVocnRL w2pw== X-Gm-Message-State: AOAM532GdZlfSgxaOtWqtJTMFvoNZXesAeZOALkKQjFIYo78CUcPy39S 5EjthxhLzx0jG9x2YfF+7It4vuVwPb45nbj6pLQ= X-Google-Smtp-Source: ABdhPJwFQ073nDMggBsmgEfd39lbF92t9uUAYCP5BmrAGgUpF7AONYkQCBwtU0qnzXIxFtXNmKzJ/7FLeqoWd37ey4w= X-Received: by 2002:aa7:d2d2:: with SMTP id k18mr4140229edr.290.1604700132754; Fri, 06 Nov 2020 14:02:12 -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 14:02:00 -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 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 if > > THP migration is unsupported, then the THP will be split. If split is > > failed just exit without trying to migrate other pages. It doesn't mak= e > > 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_page= , > > 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 g= et_new_page, > > return rc; > > } > > > > +static inline int try_split_thp(struct page *page, struct page *page2, > > + 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 pointer,= the change > does not return back the caller. You need to use the pointer to page2 her= e. > > > + > > + return rc; > > +} > > + > > /* > > * migrate_pages - migrate the pages specified in a list, to the free = pages > > * supplied as the target for the page migration > > @@ -1445,24 +1459,40 @@ int migrate_pages(struct list_head *from, new_p= age_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, f= rom)) { > > + 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 or = the > > - * allocation could've failed so we shoul= d > > - * retry on the same page with the THP sp= lit > > - * to base pages. > > - * > > - * Head page is retried immediately and t= ail > > - * pages are added to the tail of the lis= t so > > - * we encounter them after the rest of th= e list > > - * is processed. > > + * When memory is low, don't bother to tr= y to migrate > > + * other pages, just exit. > > The comment does not match the code below. For THPs, the code tries to sp= lit 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. > > > */ > > if (is_thp) { > > - lock_page(page); > > - rc =3D split_huge_page_to_list(pa= ge, from); > > - unlock_page(page); > > - if (!rc) { > > - list_safe_reset_next(page= , page2, lru); > > + if (!try_split_thp(page, page2, f= rom)) { > > nr_thp_split++; > > goto retry; > > } > > @@ -1490,7 +1520,7 @@ int migrate_pages(struct list_head *from, new_pag= e_t get_new_page, > > break; > > default: > > /* > > - * Permanent failure (-EBUSY, -ENOSYS, et= c.): > > + * Permanent failure (-EBUSY, etc.): > > * unlike -EAGAIN case, the failed page i= s > > * removed from migration page list and n= ot > > * retried in the next outer loop. > > -- > > 2.26.2 > > > =E2=80=94 > Best Regards, > Yan Zi