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=-12.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,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 DBA6DC433B4 for ; Thu, 8 Apr 2021 17:27:09 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 6CC3E610C8 for ; Thu, 8 Apr 2021 17:27:09 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6CC3E610C8 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 E00ED6B0036; Thu, 8 Apr 2021 13:27:08 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DB0F56B006E; Thu, 8 Apr 2021 13:27:08 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C03986B0071; Thu, 8 Apr 2021 13:27:08 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id A1CEF6B0036 for ; Thu, 8 Apr 2021 13:27:08 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 4C82783EB for ; Thu, 8 Apr 2021 17:27:08 +0000 (UTC) X-FDA: 78009880536.07.3E753D2 Received: from mail-ej1-f51.google.com (mail-ej1-f51.google.com [209.85.218.51]) by imf11.hostedemail.com (Postfix) with ESMTP id 3567F200026E for ; Thu, 8 Apr 2021 17:27:02 +0000 (UTC) Received: by mail-ej1-f51.google.com with SMTP id u17so4404548ejk.2 for ; Thu, 08 Apr 2021 10:27:07 -0700 (PDT) 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; bh=12TgNirSEgx1xK4iZu+oef0GZkuYbX4qCm+wQ6HbK54=; b=RHq1X5DervhotWlcoeZ+ZH0NkaVwhuBsZBjLjIk3P4d/VW+2KYPjT2TRuGnNVLqPIv w8J1sWpsS+vNl4kP2NeCRwwcP0CkpfCvRZUb/DrE1qSOS4/CjxPzIEEblvdKOjd2tJE6 dkCOwIMaCuoEhfrVy+QZ5CiRlLB9oY1ZRuu69Ax7OQwCct7VLVFfAttmZQwOPLB2INbP iiqoZfjwRE4GiM+ptZOoSbGTMP7cbSXOS/yOpqwNiBDIcG3aOm+JUhJ18g0RHi6kg/5Q 0A2/Lpj7bIpeYZhvomPGZ7epL3F9dw9GOqrMdREjaUcSFIsfDDfAAw2BUtk606/HGkMT cq0w== 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; bh=12TgNirSEgx1xK4iZu+oef0GZkuYbX4qCm+wQ6HbK54=; b=mUzOLcFF1XXmHwZ5jUA89mdVqU1UokGpw1RTF73p/zPclDWZq3FZDFtAS8hCfsz7nP WK7k9mDyuqhRuLU6+LecM+7L/aV0pIPCENA3igzA1CPbiOyGEj+TSXZqUhEfs5wI/Mjk I8tViNy/iWyLp1vM6HjG+a9ZS3MQdFeuX4ZwNaJB3lxFW981oBVQwosqAXIduoMdosDM Y4xNIOdh2S41e/491ydriGntZxKV3C4Bp5IkcFdhyQaFMA9WVd0JMwzWSTG68UiPr7Pp Fb308x6Y9YbNW934fj10b/mjHp9Yk1uQQUC+o/FDGWhGtcXzGpPtPRRRF7u0hgvNmg+r 5r0Q== X-Gm-Message-State: AOAM533iALBMKYAAasuLyEPBFMzVzAlukWcEXmKd/9jB64P8a+D31O7M uRQZxkT3Agk6loe5kT95lqv5Y7TwjU6XERYVe7uCye41 X-Google-Smtp-Source: ABdhPJybMRj5YdSyDJty/Q4euXx9aSpxateuLXjVnfO7x/PLTche1UstCaIQ+5W7KpvIGxamLG3he5X8ZXJRZUbjrlo= X-Received: by 2002:a17:906:6dd9:: with SMTP id j25mr11666611ejt.507.1617902826704; Thu, 08 Apr 2021 10:27:06 -0700 (PDT) MIME-Version: 1.0 References: <20210401183216.443C4443@viggo.jf.intel.com> <20210401183223.80F1E291@viggo.jf.intel.com> In-Reply-To: From: Yang Shi Date: Thu, 8 Apr 2021 10:26:54 -0700 Message-ID: Subject: Re: [PATCH 04/10] mm/migrate: make migrate_pages() return nr_succeeded To: Oscar Salvador Cc: Dave Hansen , Linux MM , Linux Kernel Mailing List , Yang Shi , weixugc@google.com, Huang Ying , Dan Williams , David Hildenbrand Content-Type: text/plain; charset="UTF-8" X-Stat-Signature: s74krosof14gfcfusczezy8naknzp3pp X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 3567F200026E Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf11; identity=mailfrom; envelope-from=""; helo=mail-ej1-f51.google.com; client-ip=209.85.218.51 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1617902822-872369 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 Thu, Apr 8, 2021 at 3:14 AM Oscar Salvador wrote: > > On Thu, Apr 01, 2021 at 11:32:23AM -0700, Dave Hansen wrote: > > > > From: Yang Shi > > > > The migrate_pages() returns the number of pages that were not migrated, > > or an error code. When returning an error code, there is no way to know > > how many pages were migrated or not migrated. > > > > In the following patch, migrate_pages() is used to demote pages to PMEM > > node, we need account how many pages are reclaimed (demoted) since page > > reclaim behavior depends on this. Add *nr_succeeded parameter to make > > migrate_pages() return how many pages are demoted successfully for all > > cases. > > > > Signed-off-by: Yang Shi > > Signed-off-by: Dave Hansen > > Reviewed-by: Yang Shi > > Cc: Wei Xu > > Cc: Huang Ying > > Cc: Dan Williams > > Cc: David Hildenbrand > > Cc: osalvador > > > > ... > > int migrate_pages(struct list_head *from, new_page_t get_new_page, > > free_page_t put_new_page, unsigned long private, > > - enum migrate_mode mode, int reason) > > + enum migrate_mode mode, int reason, unsigned int *nr_succeeded) > > { > > int retry = 1; > > int thp_retry = 1; > > int nr_failed = 0; > > - int nr_succeeded = 0; > > int nr_thp_succeeded = 0; > > int nr_thp_failed = 0; > > int nr_thp_split = 0; > > @@ -1611,10 +1611,10 @@ retry: > > case MIGRATEPAGE_SUCCESS: > > if (is_thp) { > > nr_thp_succeeded++; > > - nr_succeeded += nr_subpages; > > + *nr_succeeded += nr_subpages; > > break; > > } > > - nr_succeeded++; > > + (*nr_succeeded)++; > > break; > > default: > > /* > > @@ -1643,12 +1643,12 @@ out: > > */ > > list_splice(&ret_pages, from); > > > > - count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded); > > + count_vm_events(PGMIGRATE_SUCCESS, *nr_succeeded); > > count_vm_events(PGMIGRATE_FAIL, nr_failed); > > count_vm_events(THP_MIGRATION_SUCCESS, nr_thp_succeeded); > > count_vm_events(THP_MIGRATION_FAIL, nr_thp_failed); > > count_vm_events(THP_MIGRATION_SPLIT, nr_thp_split); > > - trace_mm_migrate_pages(nr_succeeded, nr_failed, nr_thp_succeeded, > > + trace_mm_migrate_pages(*nr_succeeded, nr_failed, nr_thp_succeeded, > > nr_thp_failed, nr_thp_split, mode, reason); > > It seems that reclaiming is the only user who cared about how many pages > could we migrated, could not do the following instead: > > diff --git a/mm/migrate.c b/mm/migrate.c > index 695a594e5860..d4170b7ea2fe 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1503,7 +1503,7 @@ static inline int try_split_thp(struct page *page, struct page **page2, > */ > int migrate_pages(struct list_head *from, new_page_t get_new_page, > free_page_t put_new_page, unsigned long private, > - enum migrate_mode mode, int reason) > + enum migrate_mode mode, int reason, unsigned int *ret_succeeded) > { > int retry = 1; > int thp_retry = 1; > @@ -1654,6 +1654,9 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, > if (!swapwrite) > current->flags &= ~PF_SWAPWRITE; > > + if (ret_succedded) > + *ret_succedded = nr_succedded; > + > return rc; > } > > And pass only a valid pointer from demote_page_list() and NULL from all > the others? > I was just wondered after all those "unsigned int nr_succedded" in all > other functions. > This would also solve the "be careful to initialize nr_succedded" > problem? Thanks, Oscar. Yes, kind of. But we have to remember to initialize "nr_succedded" pointer properly for every migrate_pages() callsite, right? And it doesn't prevent from returning wrong value if migrate_pages() is called multiple times by one caller although there might be not such case (calls migrate_pages() multiple times and care about nr_succeded) for now. So IMHO I do prefer Wei's suggestion to have migrate_pages() initialize nr_succeeded. This seems simpler. > > > -- > Oscar Salvador > SUSE L3