Linux-mm Archive on lore.kernel.org
 help / color / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: linux-mm@kvack.org, Mel Gorman <mgorman@techsingularity.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org,
	Andrea Arcangeli <aarcange@redhat.com>,
	David Hildenbrand <david@redhat.com>,
	Michal Hocko <mhocko@kernel.org>, Mike Rapoport <rppt@kernel.org>,
	Vlastimil Babka <vbabka@suse.cz>,
	stable@vger.kernel.org
Subject: [PATCH] mm, compaction: make fast_isolate_freepages() stay within zone
Date: Wed, 17 Feb 2021 18:33:00 +0100
Message-ID: <20210217173300.6394-1-vbabka@suse.cz> (raw)

Compaction always operates on pages from a single given zone when isolating
both pages to migrate and freepages. Pageblock boundaries are intersected with
zone boundaries to be safe in case zone starts or ends in the middle of
pageblock. The use of pageblock_pfn_to_page() protects against non-contiguous
pageblocks.

The functions fast_isolate_freepages() and fast_isolate_around() don't
currently protect the fast freepage isolation thoroughly enough against these
corner cases, and can result in freepage isolation operate outside of zone
boundaries:

- in fast_isolate_freepages() if we get a pfn from the first pageblock of a
  zone that starts in the middle of that pageblock, 'highest' can be a pfn
  outside of the zone. If we fail to isolate anything in this function, we
  may then call fast_isolate_around() on a pfn outside of the zone and there
  effectively do a set_pageblock_skip(page_to_pfn(highest)) which may currently
  hit a VM_BUG_ON() in some configurations
- fast_isolate_around() checks only the zone end boundary and not beginning,
  nor that the pageblock is contiguous (with pageblock_pfn_to_page()) so it's
  possible that we end up calling isolate_freepages_block() on a range of pfn's
  from two different zones and end up e.g. isolating freepages under the wrong
  zone's lock.

This patch should fix the above issues.

Fixes: 5a811889de10 ("mm, compaction: use free lists to quickly locate a migration target")
Cc: <stable@vger.kernel.org>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 mm/compaction.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Hi, as promised here's a fix for issues that I think exist regardless of the
memblock stuff, but were partially exposed by that. I will see if I can manage
to test that it does prevent the known symptoms (it should if I didn't miss
anything).

diff --git a/mm/compaction.c b/mm/compaction.c
index 190ccdaa6c19..22a35521e358 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1288,7 +1288,7 @@ static void
 fast_isolate_around(struct compact_control *cc, unsigned long pfn, unsigned long nr_isolated)
 {
 	unsigned long start_pfn, end_pfn;
-	struct page *page = pfn_to_page(pfn);
+	struct page *page;
 
 	/* Do not search around if there are enough pages already */
 	if (cc->nr_freepages >= cc->nr_migratepages)
@@ -1299,8 +1299,12 @@ fast_isolate_around(struct compact_control *cc, unsigned long pfn, unsigned long
 		return;
 
 	/* Pageblock boundaries */
-	start_pfn = pageblock_start_pfn(pfn);
-	end_pfn = min(pageblock_end_pfn(pfn), zone_end_pfn(cc->zone)) - 1;
+	start_pfn = max(pageblock_start_pfn(pfn), cc->zone->zone_start_pfn);
+	end_pfn = min(pageblock_end_pfn(pfn), zone_end_pfn(cc->zone));
+
+	page = pageblock_pfn_to_page(start_pfn, end_pfn, cc->zone);
+	if (!page)
+		return;
 
 	/* Scan before */
 	if (start_pfn != pfn) {
@@ -1402,7 +1406,8 @@ fast_isolate_freepages(struct compact_control *cc)
 			pfn = page_to_pfn(freepage);
 
 			if (pfn >= highest)
-				highest = pageblock_start_pfn(pfn);
+				highest = max(pageblock_start_pfn(pfn),
+					      cc->zone->zone_start_pfn);
 
 			if (pfn >= low_pfn) {
 				cc->fast_search_fail = 0;
@@ -1472,7 +1477,8 @@ fast_isolate_freepages(struct compact_control *cc)
 			} else {
 				if (cc->direct_compaction && pfn_valid(min_pfn)) {
 					page = pageblock_pfn_to_page(min_pfn,
-						pageblock_end_pfn(min_pfn),
+						min(pageblock_end_pfn(min_pfn),
+						    zone_end_pfn(cc->zone)),
 						cc->zone);
 					cc->free_pfn = min_pfn;
 				}
-- 
2.30.0



             reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-17 17:33 Vlastimil Babka [this message]
2021-02-17 18:16 ` David Rientjes
2021-02-17 18:50 ` Mel Gorman
2021-02-18 17:42 ` Vlastimil Babka

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=20210217173300.6394-1-vbabka@suse.cz \
    --to=vbabka@suse.cz \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@techsingularity.net \
    --cc=mhocko@kernel.org \
    --cc=rppt@kernel.org \
    --cc=stable@vger.kernel.org \
    /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

Linux-mm Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-mm/0 linux-mm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-mm linux-mm/ https://lore.kernel.org/linux-mm \
		linux-mm@kvack.org
	public-inbox-index linux-mm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kvack.linux-mm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git