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 181E6C3DA7D for ; Thu, 5 Jan 2023 05:54:11 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C86D8E0002; Thu, 5 Jan 2023 00:54:11 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 876E18E0001; Thu, 5 Jan 2023 00:54:11 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6F2E68E0002; Thu, 5 Jan 2023 00:54:11 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 5A5DD8E0001 for ; Thu, 5 Jan 2023 00:54:11 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 23E67160D5E for ; Thu, 5 Jan 2023 05:54:11 +0000 (UTC) X-FDA: 80319679902.12.0845FCB Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by imf02.hostedemail.com (Postfix) with ESMTP id 2CDA08000B for ; Thu, 5 Jan 2023 05:54:08 +0000 (UTC) Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=cid1nxKc; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.88 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1672898049; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=F/ZDmbHGsqoW/ty4ZaUVS8c/qkFNxYP9Jo/o6Z29EzQ=; b=WPIEJEI/iFl94MWn/iB29AyPg1GYpifgbaUEwChJqMLb+LR0wTsFvXLjSYKydFySleZZ43 VzV1Oa9bFOekZyBqZdYCbGrv6S8T6kU2PlVx4fPpeChYtmwfYPheYqj7CYQQDXfST0tppj SPRQgdV7lu2wt0kXxIBp8EPWeycGPic= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=cid1nxKc; spf=pass (imf02.hostedemail.com: domain of ying.huang@intel.com designates 192.55.52.88 as permitted sender) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1672898049; a=rsa-sha256; cv=none; b=rV002zySswUgF4iBKXl0YYK7zLLuc/HNQx6Fjt0Iwaoe/PdCikurRlJHycPq3gwewfp6KM tn0Li/K4+o6gYXJ2QKji+CLoKxBd5MajJa3RigfIwrk5Gqvkqe2anluoaL2gbWQ6MzE+CC mNISo3+pE9XYWEzAAp86MaXPJlGaDGw= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1672898049; x=1704434049; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=VmEZjSfJDXSrUyUEeB9nOlbhCCZ3NoNLGnwj9XiAsqY=; b=cid1nxKce6APyHMRHdhJ7jZnT1DBqCHLi3e4sJqtJrAPLRM0c8RyHmPi MiS12yWaGC2ZBlwzfFt/WEbltZR/rnC7xV8SCCvM9qZiNtOU5mexG6EEj CD4EM0DJuUB4SrUYeyOz87fDCL3/GXe1H1AlSKM5PORSy4JJQrgnvcd2X HL6ReX6EuGH5xwHlxP8VTSB0Cqpc0wkCfc7S7H3nokaNfrN2lyPqDNeJ7 4V36q0AbNe8Wh/TlXIMG9DxOSntQAHxaPQESUzYZCiDsF/StBaLtTqg1M KJytVsyEv2lJmYmlF034gmoSDIYa8XPmBFmLKwEayc23NRfOy1XchFqeA g==; X-IronPort-AV: E=McAfee;i="6500,9779,10580"; a="349342773" X-IronPort-AV: E=Sophos;i="5.96,302,1665471600"; d="scan'208";a="349342773" Received: from fmsmga007.fm.intel.com ([10.253.24.52]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jan 2023 21:54:07 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10580"; a="657381594" X-IronPort-AV: E=Sophos;i="5.96,302,1665471600"; d="scan'208";a="657381594" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by fmsmga007-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jan 2023 21:54:04 -0800 From: "Huang, Ying" To: Alistair Popple Cc: Andrew Morton , , , Zi Yan , Yang Shi , Baolin Wang , "Oscar Salvador" , Matthew Wilcox , "Bharata B Rao" , haoxin Subject: Re: [PATCH 1/8] migrate_pages: organize stats with struct migrate_pages_stats References: <20221227002859.27740-1-ying.huang@intel.com> <20221227002859.27740-2-ying.huang@intel.com> <87y1qhu0to.fsf@nvidia.com> Date: Thu, 05 Jan 2023 13:53:11 +0800 In-Reply-To: <87y1qhu0to.fsf@nvidia.com> (Alistair Popple's message of "Thu, 05 Jan 2023 14:02:12 +1100") Message-ID: <87lemheddk.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Stat-Signature: assnmsw85cx3cdk95zb3u8xgwy5oiwxb X-Rspam-User: X-Rspamd-Queue-Id: 2CDA08000B X-Rspamd-Server: rspam06 X-HE-Tag: 1672898048-510333 X-HE-Meta: U2FsdGVkX1+Me1Pk8SIQtEYLRiLGGnnMKLi87oDkW0EmnREat+GFSZcHv7C/bs1hcTm4Bi+3k9ZSZPArmvMehKPw68rXsuqi0kTRpKE7ysIVS1i6mhjFF8CSEx4r+/3bhKXifSOGjjL1QEKbESZxfnLNrlYkhcP0CjLt+yz80T3qAJZl/qjvEwi5Euoj0sWZM8aJra2zm3evhozdm/6w8DDb9KqcGmRrN4i2xmIuKukUfVsGN0qTSIqDa3NWyBYF0sEx1NMTmafokI/yuQMJFeTGHNOAp7UFnH8QyXGKszsaaD1myJ27/CLcmJMl+iKXP3RUwNSg4oGHWXwbnTbjWVKjuNCO0yx+R5TEe+B8zjaczoQ7LJWFwFbnxPopLRCLE5ybLOPjkjD+hM5ofmrPvD011cLYwi05a84ilGPRe/p8xd8anuKBH7oL4pmIo4mfJtNjMFNR73UXGp4G4O5MlMpgf4mtDAPwuqnAS5cKS1n7zfssGGbQ/zW7O5PXz7TOz1VhrBdmJXrIiDOl0cIdXWGPeE2up0YgPzeqWce/g9YTvyueIHRtkT9aEFsbjyJDtVfgHgzZvuPTKFI0hsBYQ2vgEup5cdqZUlUCG9k/xyEitN3FwFoc7CCz32aTHUpTsZx+7VoloPAeOBnSF2A6QyKh3YS0LYCC4XApi6InfZMw0qNqrn3moG2QiIgH4XpJ8gr5l+XfyvoZHA4Q6SOPCP6V2h2pB2z64o83n+oNsMShHNw/MYvZCdjyaZqdGPAg/6SAKtoZeVFZpsPo/QqStODkyXQ9f4I/vyR4B/+mVEI2dBR2kQIrps+g7bMbZgV8iGZVqhkiOzq2cpnaJGvGWrMafObDWYfhoPkkGDVZwGwSXppnAbpGstBxoGDm0Bfw0hdfGgWAsgC1D+LdkC0fpIxubGHLdB35 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: Alistair Popple writes: > Huang Ying writes: > >> Define struct migrate_pages_stats to organize the various statistics >> in migrate_pages(). This makes it easier to collect and consume the >> statistics in multiple functions. This will be needed in the >> following patches in the series. >> >> Signed-off-by: "Huang, Ying" >> Cc: Zi Yan >> Cc: Yang Shi >> Cc: Baolin Wang >> Cc: Oscar Salvador >> Cc: Matthew Wilcox >> Cc: Bharata B Rao >> Cc: Alistair Popple >> Cc: haoxin >> --- >> mm/migrate.c | 58 +++++++++++++++++++++++++++++----------------------- >> 1 file changed, 32 insertions(+), 26 deletions(-) >> >> diff --git a/mm/migrate.c b/mm/migrate.c >> index a4d3fc65085f..ec9263a33d38 100644 >> --- a/mm/migrate.c >> +++ b/mm/migrate.c >> @@ -1396,6 +1396,14 @@ static inline int try_split_folio(struct folio *folio, struct list_head *split_f >> return rc; >> } >> >> +struct migrate_pages_stats { >> + int nr_succeeded; >> + int nr_failed_pages; >> + int nr_thp_succeeded; >> + int nr_thp_failed; >> + int nr_thp_split; > > I think some brief comments in the code for what each stat is tracking > and their relationship to each other would be helpful (ie. does > nr_succeeded include thp subpages, etc). Or at least a reference to > where this is documented (ie. page_migration.rst) as I recall there has > been some confusion in the past that has lead to bugs. OK, will do that in the next version. > Otherwise the patch looks good so: > > Reviewed-by: Alistair Popple Thanks! Best Regards, Huang, Ying >> +}; >> + >> /* >> * migrate_pages - migrate the folios specified in a list, to the free folios >> * supplied as the target for the page migration >> @@ -1430,13 +1438,8 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> int large_retry = 1; >> int thp_retry = 1; >> int nr_failed = 0; >> - int nr_failed_pages = 0; >> int nr_retry_pages = 0; >> - int nr_succeeded = 0; >> - int nr_thp_succeeded = 0; >> int nr_large_failed = 0; >> - int nr_thp_failed = 0; >> - int nr_thp_split = 0; >> int pass = 0; >> bool is_large = false; >> bool is_thp = false; >> @@ -1446,9 +1449,11 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> LIST_HEAD(split_folios); >> bool nosplit = (reason == MR_NUMA_MISPLACED); >> bool no_split_folio_counting = false; >> + struct migrate_pages_stats stats; >> >> trace_mm_migrate_pages_start(mode, reason); >> >> + memset(&stats, 0, sizeof(stats)); >> split_folio_migration: >> for (pass = 0; pass < 10 && (retry || large_retry); pass++) { >> retry = 0; >> @@ -1502,9 +1507,9 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> /* Large folio migration is unsupported */ >> if (is_large) { >> nr_large_failed++; >> - nr_thp_failed += is_thp; >> + stats.nr_thp_failed += is_thp; >> if (!try_split_folio(folio, &split_folios)) { >> - nr_thp_split += is_thp; >> + stats.nr_thp_split += is_thp; >> break; >> } >> /* Hugetlb migration is unsupported */ >> @@ -1512,7 +1517,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> nr_failed++; >> } >> >> - nr_failed_pages += nr_pages; >> + stats.nr_failed_pages += nr_pages; >> list_move_tail(&folio->lru, &ret_folios); >> break; >> case -ENOMEM: >> @@ -1522,13 +1527,13 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> */ >> if (is_large) { >> nr_large_failed++; >> - nr_thp_failed += is_thp; >> + stats.nr_thp_failed += is_thp; >> /* Large folio NUMA faulting doesn't split to retry. */ >> if (!nosplit) { >> int ret = try_split_folio(folio, &split_folios); >> >> if (!ret) { >> - nr_thp_split += is_thp; >> + stats.nr_thp_split += is_thp; >> break; >> } else if (reason == MR_LONGTERM_PIN && >> ret == -EAGAIN) { >> @@ -1546,7 +1551,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> nr_failed++; >> } >> >> - nr_failed_pages += nr_pages + nr_retry_pages; >> + stats.nr_failed_pages += nr_pages + nr_retry_pages; >> /* >> * There might be some split folios of fail-to-migrate large >> * folios left in split_folios list. Move them back to migration >> @@ -1556,7 +1561,7 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> list_splice_init(&split_folios, from); >> /* nr_failed isn't updated for not used */ >> nr_large_failed += large_retry; >> - nr_thp_failed += thp_retry; >> + stats.nr_thp_failed += thp_retry; >> goto out; >> case -EAGAIN: >> if (is_large) { >> @@ -1568,8 +1573,8 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> nr_retry_pages += nr_pages; >> break; >> case MIGRATEPAGE_SUCCESS: >> - nr_succeeded += nr_pages; >> - nr_thp_succeeded += is_thp; >> + stats.nr_succeeded += nr_pages; >> + stats.nr_thp_succeeded += is_thp; >> break; >> default: >> /* >> @@ -1580,20 +1585,20 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> */ >> if (is_large) { >> nr_large_failed++; >> - nr_thp_failed += is_thp; >> + stats.nr_thp_failed += is_thp; >> } else if (!no_split_folio_counting) { >> nr_failed++; >> } >> >> - nr_failed_pages += nr_pages; >> + stats.nr_failed_pages += nr_pages; >> break; >> } >> } >> } >> nr_failed += retry; >> nr_large_failed += large_retry; >> - nr_thp_failed += thp_retry; >> - nr_failed_pages += nr_retry_pages; >> + stats.nr_thp_failed += thp_retry; >> + stats.nr_failed_pages += nr_retry_pages; >> /* >> * Try to migrate split folios of fail-to-migrate large folios, no >> * nr_failed counting in this round, since all split folios of a >> @@ -1626,16 +1631,17 @@ int migrate_pages(struct list_head *from, new_page_t get_new_page, >> if (list_empty(from)) >> rc = 0; >> >> - count_vm_events(PGMIGRATE_SUCCESS, nr_succeeded); >> - count_vm_events(PGMIGRATE_FAIL, nr_failed_pages); >> - 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_pages, nr_thp_succeeded, >> - nr_thp_failed, nr_thp_split, mode, reason); >> + count_vm_events(PGMIGRATE_SUCCESS, stats.nr_succeeded); >> + count_vm_events(PGMIGRATE_FAIL, stats.nr_failed_pages); >> + count_vm_events(THP_MIGRATION_SUCCESS, stats.nr_thp_succeeded); >> + count_vm_events(THP_MIGRATION_FAIL, stats.nr_thp_failed); >> + count_vm_events(THP_MIGRATION_SPLIT, stats.nr_thp_split); >> + trace_mm_migrate_pages(stats.nr_succeeded, stats.nr_failed_pages, >> + stats.nr_thp_succeeded, stats.nr_thp_failed, >> + stats.nr_thp_split, mode, reason); >> >> if (ret_succeeded) >> - *ret_succeeded = nr_succeeded; >> + *ret_succeeded = stats.nr_succeeded; >> >> return rc; >> }