All of lore.kernel.org
 help / color / mirror / Atom feed
From: Khalid Aziz <khalid.aziz@oracle.com>
To: akpm@linux-foundation.org
Cc: Khalid Aziz <khalid.aziz@oracle.com>,
	willy@infradead.org, steven.sistare@oracle.com,
	ying.huang@intel.com, mgorman@techsingularity.net,
	khalid@kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v3] mm, compaction: Skip all non-migratable pages during scan
Date: Wed, 17 May 2023 10:15:54 -0600	[thread overview]
Message-ID: <20230517161555.84776-1-khalid.aziz@oracle.com> (raw)

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 <khalid.aziz@oracle.com>
Suggested-by: Steve Sistare <steven.sistare@oracle.com>
---
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 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()
+ */
+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;
 
 		/*
-- 
2.37.2


             reply	other threads:[~2023-05-17 16:17 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 16:15 Khalid Aziz [this message]
2023-05-17 18:32 ` [PATCH v3] mm, compaction: Skip all non-migratable pages during scan David Hildenbrand
2023-05-17 22:33   ` Khalid Aziz
2023-05-18  1:09   ` Huang, Ying
2023-05-19  9:51     ` David Hildenbrand
2023-05-22  5:55       ` Huang, Ying
2023-05-22 15:12         ` Khalid Aziz
2023-05-23  1:23           ` Huang, Ying
2023-05-18  1:21 ` Huang, Ying
2023-05-18 15:07   ` Khalid Aziz
2023-05-19  0:19     ` Huang, Ying
2023-05-23  3:42 ` Baolin Wang
2023-05-23 20:54   ` Khalid Aziz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230517161555.84776-1-khalid.aziz@oracle.com \
    --to=khalid.aziz@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=khalid@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=steven.sistare@oracle.com \
    --cc=willy@infradead.org \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.