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 2A55FC77B7A for ; Fri, 19 May 2023 00:20:45 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7447F900004; Thu, 18 May 2023 20:20:44 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6F4A2900003; Thu, 18 May 2023 20:20:44 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 5E2E7900004; Thu, 18 May 2023 20:20:44 -0400 (EDT) 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 4F0BD900003 for ; Thu, 18 May 2023 20:20:44 -0400 (EDT) Received: from smtpin20.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 17BFD14023F for ; Fri, 19 May 2023 00:20:44 +0000 (UTC) X-FDA: 80805098808.20.8412CBB Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by imf10.hostedemail.com (Postfix) with ESMTP id 88559C0012 for ; Fri, 19 May 2023 00:20:41 +0000 (UTC) Authentication-Results: imf10.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=F9HTzBRb; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf10.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.126 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684455642; 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=RSt/JNvviQnbmT4RkKXU4bAofLsLxSHIns1kLmgdI7E=; b=FbHBur8vgc4WnuEzlTxjY+u8TQ0JZy6VQVCweUx2RJOswfaC7WdxwRn1UJh70eHub0PH3p gQdNf7yN8XVLQmh1gkPA6PGNihXB8CBG4ChYIwP6qbTtwtoBbNKos0H4z1161pyfJ5DgaT NckOiPxDBKaDJODQEOO4awVkgrJTM9w= ARC-Authentication-Results: i=1; imf10.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=F9HTzBRb; dmarc=pass (policy=none) header.from=intel.com; spf=pass (imf10.hostedemail.com: domain of ying.huang@intel.com designates 134.134.136.126 as permitted sender) smtp.mailfrom=ying.huang@intel.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684455642; a=rsa-sha256; cv=none; b=AC3Qocvnmt+2HanYf5Ju/uQD9cUakNBCusKO85cHUyBnav3Fc7haufvCxYjfHC2k0l+F0E b5t+yBJxA3Gr3/5Ne/CryNfPcVvr5HofTayrPwvJouMrDD0NgKDStg5ZMdkp3b8dZO9POt jZFfoGy4u5+jglUhSXkdTT6HpPMJwgs= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1684455641; x=1715991641; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=WTuK+vaLLwzgrdOsRK3ZECGUwV3wsnYB4PehJqDqJis=; b=F9HTzBRbhzwIkvPgT172OKLz8bm0UhjWtX3ZXLxefxOiwaAR2gIvuLrh CkIovqTEOrUl+4s3xLO0oRP0vHi+m4G/J6QpvYUdZfXGVWomd5xom2Kq2 gi7a1ptMA8Jw5FTZy1AftZrbu/sLYIt93CWBYuUEc3kO5hPXwiCdL3ZI7 /XivLYOuQk71OkmtjJ4PXjHJgvHkpkTYjXQK8kIKkRDIq+JGVBMNWnk3s WyxzhhXSAYZkMkloLzQeXI5ujLGBZqQGvKGmXd38Uo16An5bJjCdpPTdb nLVjXlv/EpdqQ8S8EPaFv+vIP605uRnDBzqEaClofkp4dEW/njEIPh+ih w==; X-IronPort-AV: E=McAfee;i="6600,9927,10714"; a="336819405" X-IronPort-AV: E=Sophos;i="6.00,175,1681196400"; d="scan'208";a="336819405" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2023 17:20:39 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10714"; a="733084244" X-IronPort-AV: E=Sophos;i="6.00,175,1681196400"; d="scan'208";a="733084244" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.238.208.55]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 May 2023 17:20:37 -0700 From: "Huang, Ying" To: Khalid Aziz Cc: akpm@linux-foundation.org, willy@infradead.org, steven.sistare@oracle.com, mgorman@techsingularity.net, khalid@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3] mm, compaction: Skip all non-migratable pages during scan References: <20230517161555.84776-1-khalid.aziz@oracle.com> <87o7mibfj0.fsf@yhuang6-desk2.ccr.corp.intel.com> <7616301d-d7a7-bfb4-234b-eb83fb248cac@oracle.com> Date: Fri, 19 May 2023 08:19:28 +0800 In-Reply-To: <7616301d-d7a7-bfb4-234b-eb83fb248cac@oracle.com> (Khalid Aziz's message of "Thu, 18 May 2023 09:07:46 -0600") Message-ID: <87pm6x6ulr.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-Rspam-User: X-Stat-Signature: gyyqo91zpd3odw958jg7jc84mkeazwur X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 88559C0012 X-HE-Tag: 1684455641-822496 X-HE-Meta: U2FsdGVkX1974Z+vaS8NoIjPmnYchVusDYx6vlK81f0yYXwWU9WR201WivwiAWWsaqqy3+3naAsMo/zAbqBfMifilWyme55l23PzPdJ6wXzCfHxid1Hu9/WEiUV1YvZzTe/FsWHV8C19sub1XR25V9b7GgZd/djFhotyuIamoezz6FhYROfKeWNtsYQPZtJ19S1WxBRAF8KuQAxe0xKwqPzqYI6xqWzK1cShP13PenS7y+FavS+IMn0Fk5OTGMa/nUnRTOwNgqnh+U6jsHMseFLX6I9hMst0YUMqrTtf4GLkcTKQkABBP7BcbPOphPgsBTH9jO09CzhhKrRQUT2824ENJuRPEZdzsnXfx0txQshq3TEEXG4yuIV6uE3ftdNY0iHtoKk5xY/DkF7SW6mrKBijUctLQQIZa3m2IMKulWtG6rOI8ElIu21tVvq+SJ5ynOgqPwb47hWzWGNX+O4XVaxRqwAjFQQvEkRUWw4X3udEpANHFP0tNodNrxHeC4FMKeMEEgeaI/5xZq4Ozf5GHUk5i98hZK0pgHc80EI1zONLlJiabx+X73eAUwzSVZmlDRRQULljukMNtwLU+ceUgaj4pQeqneWLC6rH4NDqrmGTK5QsZ170LHnyuhDGcgluJoh6slfCMSQ4/QECHJMx4766qOTUqcE13xHGz1sgEK4EztOMXmJ93XNeJ3rrCbUB4pdLTbb3eOjAoaP40IHWIF0J/MZW+irk160MkjJhfJVFzlEyDImMqCkcIaxkh4AcfHbvyM8VNeQ91WULZmWQVigAYBZ0spE1FLVsbjEp0YR+4NmVybzVejALQFcDTPXOUslH1FXKLLF+wcwmbJAs0nRju0C5iqAZs6S5eUi54998cdvfuco9QDAjamMD00/koWItPOrgGJbo3+lybglmBuK2hLjWMEGeR5iotTZHz4YJk358PNm0Ur56UWKk+GzVG96Qt6WEzv8sf3t1BLE 5diNRUCi /3woGmTJ+Ro4c9f9k1wtDAH6P1+obK33aLC3OoWSj8MbUHQ5JZhyPtfjImDI5RHDHg/jeV0+X278ks1PAoPj2TGl4VQmOgypAu6+CiiR/nFxXMCl7AxVB73Zu3+2N4iDHs4Gg7fASB8Dedxni6p6dLMioga5H3o2gNeV02Bj1K6exTSc= 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: Khalid Aziz writes: > On 5/17/23 19:21, Huang, Ying wrote: >> Khalid Aziz writes: >> >>> Pages pinned in memory through extra refcounts can not be migrated. >>> Currently as isolate_migratepages_block() scans pages for >>> compaction, it skips any pinned anonymous pages. All non-migratable >>> pages should be skipped and not just the anonymous pinned pages. >>> This patch adds a check for extra refcounts on a page to determine >>> if the page can be migrated. This was seen as a real issue on a >>> customer workload where a large number of pages were pinned by vfio >>> on the host and any attempts to allocate hugepages resulted in >>> significant amount of cpu time spent in either direct compaction or >>> in kcompactd scanning vfio pinned pages over and over again that can >>> not be migrated. >>> >>> Signed-off-by: Khalid Aziz >>> Suggested-by: Steve Sistare >>> --- >>> v3: >>> - Account for extra ref added by get_page_unless_zero() earlier >>> in isolate_migratepages_block() (Suggested by Huang, Ying) >>> - Clean up computation of extra refs to be consistent >>> (Suggested by Huang, Ying) >>> >>> v2: >>> - Update comments in the code (Suggested by Andrew) >>> - Use PagePrivate() instead of page_has_private() (Suggested >>> by Matthew) >>> - Pass mapping to page_has_extrarefs() (Suggested by Matthew) >>> - Use page_ref_count() (Suggested by Matthew) >>> - Rename is_pinned_page() to reflect its function more >>> accurately (Suggested by Matthew) >>> >>> mm/compaction.c | 36 ++++++++++++++++++++++++++++++++---- >>> 1 file changed, 32 insertions(+), 4 deletions(-) >>> >>> diff --git a/mm/compaction.c b/mm/compaction.c >>> index 5a9501e0ae01..f04c00981172 100644 >>> --- a/mm/compaction.c >>> +++ b/mm/compaction.c >>> @@ -764,6 +764,34 @@ static bool too_many_isolated(pg_data_t *pgdat) >>> return too_many; >>> } >>> +/* >>> + * Check if this base page should be skipped from isolation because >>> + * it has extra refcounts that will prevent it from being migrated. >> This appears duplicated with the comments in caller. It's OK to >> keep >> one only? > > I don't see any harm in explaining what the function is doing and then > why this function is being called. This allows one to understand code > in either place without having to refer to the function and function > call both to understand what the code is doing. Duplicated comments will waste reader's time. >> >>> + * This function is called for regular pages only, and not >>> + * for THP or hugetlbfs pages. This code is inspired by similar code >>> + * in migrate_vma_check_page(), can_split_folio() and >>> + * folio_migrate_mapping() >> It's not good to duplicate code. Why not just use >> folio_expected_refs()? > > compaction.c has not been converted over to folios, so I want to not > do piecemeal conversion. When compaction code is converted over, it > would make sense to see if this could also use the folio function. > folio_expected_refs() currently is a static function in mm/migrate.c. page_folio() is handy. Best Regards, Huang, Ying >> >>> + */ >>> +static inline bool page_has_extra_refs(struct page *page) >>> +{ >>> + /* caller holds a ref already from get_page_unless_zero() */ >>> + unsigned long extra_refs = 1; >>> + >>> + /* anonymous page can have extra ref from swap cache */ >>> + if (PageAnon(page)) >>> + extra_refs += PageSwapCache(page) ? 1 : 0; >>> + else >>> + extra_refs += 1 + PagePrivate(page); >>> + >>> + /* >>> + * This is an admittedly racy check but good enough to determine >>> + * if a page is pinned and can not be migrated >>> + */ >>> + if ((page_ref_count(page) - extra_refs) > page_mapcount(page)) >>> + return true; >>> + return false; >>> +} >>> + >>> /** >>> * isolate_migratepages_block() - isolate all migrate-able pages within >>> * a single pageblock >>> @@ -992,12 +1020,12 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn, >>> goto isolate_fail; >>> /* >>> - * Migration will fail if an anonymous page is pinned in memory, >>> - * so avoid taking lru_lock and isolating it unnecessarily in an >>> - * admittedly racy check. >>> + * Migration will fail if a page has extra refcounts >>> + * preventing it from migrating, so avoid taking >>> + * lru_lock and isolating it unnecessarily >>> */ >>> mapping = page_mapping(page); >>> - if (!mapping && (page_count(page) - 1) > total_mapcount(page)) >>> + if (page_has_extra_refs(page)) >>> goto isolate_fail_put; >>> /*