[PATCH v2 0/4] Make MIGRATE_ISOLATE a standalone bit

[PATCH v2 0/4] Make MIGRATE_ISOLATE a standalone bit

[Zi Yan]

Hi all,

This patchset moves MIGRATE_ISOLATE to a standalone bit to avoid
being overwritten during pageblock isolation process. Currently,
MIGRATE_ISOLATE is part of enum migratetype (in include/linux/mmzone.h),
thus, setting a pageblock to MIGRATE_ISOLATE overwrites its original
migratetype. This causes pageblock migratetype loss during
alloc_contig_range() and memory offline, especially when the process
fails due to a failed pageblock isolation and the code tries to undo the
finished pageblock isolations.

It is on top of mm-everything-2025-02-13-23-42.

In terms of performance for changing pageblock types, no performance
change is observed:

1. I used perf to collect stats of offlining and onlining all memory of a
200GB VM 10 times and see that set_pfnblock_flags_mask() takes about
0.13% of the whole process with and without this patchset across 3 runs.

2. I used perf to collect stats of dd from /dev/zero to a 200GB tmpfs file
and find get_pfnblock_flags_mask() takes about 0.30% of the process with and
without this patchset across 3 runs.


Design
===

Pageblock flags are read in words to achieve good performance and existing
pageblock flags take 4 bits per pageblock. To avoid a substantial change
to the pageblock flag code, pageblock flag bits are expanded to use 8
and MIGRATE_ISOLATE is moved to use the last bit (bit 7).

It might look like the pageblock flags have doubled the overhead, but in
reality, the overhead is only 1 byte per 2MB/4MB (based on pageblock config),
or 0.0000476 %.


Any comment and/or suggestion is welcome. Thanks.

Zi Yan (4):
  mm/page_isolation: make page isolation a standalone bit.
  mm/page_isolation: remove migratetype from
    move_freepages_block_isolate()
  mm/page_isolation: remove migratetype from undo_isolate_page_range()
  mm/page_isolation: remove migratetype parameter from more functions.

 drivers/virtio/virtio_mem.c     |  3 +-
 include/linux/gfp.h             |  6 ++-
 include/linux/mmzone.h          | 18 +++++--
 include/linux/page-isolation.h  | 24 ++++++---
 include/linux/pageblock-flags.h | 33 +++++++++++-
 include/trace/events/kmem.h     | 14 ++---
 mm/cma.c                        |  2 +-
 mm/memory_hotplug.c             |  5 +-
 mm/page_alloc.c                 | 96 ++++++++++++++++++++++++---------
 mm/page_isolation.c             | 66 +++++++++--------------
 10 files changed, 175 insertions(+), 92 deletions(-)

-- 
2.47.2

[PATCH v2 1/4] mm/page_isolation: make page isolation a standalone bit.

[Zi Yan]

During page isolation, the original migratetype is overwritten, since
MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like
PB_migrate_skip. pageblock bits needs to be word aligned, so expand
the number of pageblock bits from 4 to 8 and make migrate isolate bit 7.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/mmzone.h          | 18 +++++++++++++-----
 include/linux/page-isolation.h  |  2 +-
 include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++-
 mm/page_alloc.c                 | 21 +++++++++++++++++++--
 4 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8aecbbb0b685..3c7d3f22ccb2 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt)
 
 extern int page_group_by_mobility_disabled;
 
-#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1)
+#ifdef CONFIG_MEMORY_ISOLATION
+#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
+#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit)
+#else
+#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
+#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
+#endif
 
-#define get_pageblock_migratetype(page)					\
-	get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
+#define get_pageblock_migratetype(page) \
+		get_pfnblock_flags_mask(page, page_to_pfn(page), \
+			MIGRATETYPE_MASK)
 
-#define folio_migratetype(folio)				\
-	get_pfnblock_flags_mask(&folio->page, folio_pfn(folio),		\
+#define folio_migratetype(folio) \
+		get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
 			MIGRATETYPE_MASK)
+
 struct free_area {
 	struct list_head	free_list[MIGRATE_TYPES];
 	unsigned long		nr_free;
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 898bb788243b..51797dc39cbc 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -5,7 +5,7 @@
 #ifdef CONFIG_MEMORY_ISOLATION
 static inline bool is_migrate_isolate_page(struct page *page)
 {
-	return get_pageblock_migratetype(page) == MIGRATE_ISOLATE;
+	return get_pageblock_isolate(page);
 }
 static inline bool is_migrate_isolate(int migratetype)
 {
diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index fc6b9c87cb0a..d6fe17bf0c9b 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -20,7 +20,10 @@ enum pageblock_bits {
 	PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
 			/* 3 bits required for migrate types */
 	PB_migrate_skip,/* If set the block is skipped by compaction */
-
+#ifdef CONFIG_MEMORY_ISOLATION
+	PB_migrate_isolate = 7, /* If set the block is isolated */
+			/* set it to 7 to make pageblock bit word aligned */
+#endif
 	/*
 	 * Assume the bits will always align on a word. If this assumption
 	 * changes then get/set pageblock needs updating.
@@ -28,6 +31,10 @@ enum pageblock_bits {
 	NR_PAGEBLOCK_BITS
 };
 
+#ifdef CONFIG_MEMORY_ISOLATION
+#define PB_migrate_isolate_bit BIT(PB_migrate_isolate)
+#endif
+
 #if defined(CONFIG_HUGETLB_PAGE)
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
@@ -99,4 +106,28 @@ static inline void set_pageblock_skip(struct page *page)
 }
 #endif /* CONFIG_COMPACTION */
 
+#ifdef CONFIG_MEMORY_ISOLATION
+#define get_pageblock_isolate(page) \
+	get_pfnblock_flags_mask(page, page_to_pfn(page),	\
+			(1 << (PB_migrate_isolate)))
+#define clear_pageblock_isolate(page) \
+	set_pfnblock_flags_mask(page, 0, page_to_pfn(page),	\
+			(1 << PB_migrate_isolate))
+#define set_pageblock_isolate(page) \
+	set_pfnblock_flags_mask(page, (1 << PB_migrate_isolate),	\
+			page_to_pfn(page),			\
+			(1 << PB_migrate_isolate))
+#else
+static inline bool get_pageblock_isolate(struct page *page)
+{
+	return false;
+}
+static inline void clear_pageblock_isolate(struct page *page)
+{
+}
+static inline void set_pageblock_isolate(struct page *page)
+{
+}
+#endif /* CONFIG_MEMORY_ISOLATION */
+
 #endif	/* PAGEBLOCK_FLAGS_H */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 16dfcf7ade74..f17f4acc38c6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -362,6 +362,7 @@ unsigned long get_pfnblock_flags_mask(const struct page *page,
 	unsigned long *bitmap;
 	unsigned long bitidx, word_bitidx;
 	unsigned long word;
+	unsigned long flags;
 
 	bitmap = get_pageblock_bitmap(page, pfn);
 	bitidx = pfn_to_bitidx(page, pfn);
@@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page,
 	 * racy, are not corrupted.
 	 */
 	word = READ_ONCE(bitmap[word_bitidx]);
-	return (word >> bitidx) & mask;
+	flags = (word >> bitidx) & mask;
+
+#ifdef CONFIG_MEMORY_ISOLATION
+	if (flags & PB_migrate_isolate_bit)
+		return MIGRATE_ISOLATE;
+#endif
+	return flags;
 }
 
 static __always_inline int get_pfnblock_migratetype(const struct page *page,
@@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
 	unsigned long bitidx, word_bitidx;
 	unsigned long word;
 
+#ifdef CONFIG_MEMORY_ISOLATION
+	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
+	/* keep other migratetype bits if MIGRATE_ISOLATE is set */
+	if (flags == MIGRATE_ISOLATE) {
+		mask &= ~((1UL << PB_migratetype_bits) - 1);
+		flags = PB_migrate_isolate_bit;
+	}
+#else
 	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
-	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
+#endif
+	/* extra one for MIGRATE_ISOLATE */
+	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1);
 
 	bitmap = get_pageblock_bitmap(page, pfn);
 	bitidx = pfn_to_bitidx(page, pfn);
-- 
2.47.2

[PATCH v2 2/4] mm/page_isolation: remove migratetype from move_freepages_block_isolate()

[Zi Yan]

Since migratetype is no longer overwritten during pageblock isolation,
moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.

Rename move_freepages_block_isolate() to share common code and add
pageblock_isolate_and_move_free_pages() and
pageblock_unisolate_and_move_free_pages() to be explicit about the page
isolation operations.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h |  4 +--
 mm/page_alloc.c                | 48 +++++++++++++++++++++++++++-------
 mm/page_isolation.c            | 21 +++++++--------
 3 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 51797dc39cbc..28c56f423e34 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -27,8 +27,8 @@ static inline bool is_migrate_isolate(int migratetype)
 
 void set_pageblock_migratetype(struct page *page, int migratetype);
 
-bool move_freepages_block_isolate(struct zone *zone, struct page *page,
-				  int migratetype);
+bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page);
+bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page);
 
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 			     int migratetype, int flags);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index f17f4acc38c6..9bba5b1c4f1d 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1848,10 +1848,10 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
 }
 
 /**
- * move_freepages_block_isolate - move free pages in block for page isolation
+ * __move_freepages_for_page_isolation - move free pages in block for page isolation
  * @zone: the zone
  * @page: the pageblock page
- * @migratetype: migratetype to set on the pageblock
+ * @isolate_pageblock to isolate the given pageblock or unisolate it
  *
  * This is similar to move_freepages_block(), but handles the special
  * case encountered in page isolation, where the block of interest
@@ -1866,10 +1866,15 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
  *
  * Returns %true if pages could be moved, %false otherwise.
  */
-bool move_freepages_block_isolate(struct zone *zone, struct page *page,
-				  int migratetype)
+static bool __move_freepages_for_page_isolation(struct zone *zone,
+		struct page *page, bool isolate_pageblock)
 {
 	unsigned long start_pfn, pfn;
+	int from_mt;
+	int to_mt;
+
+	if (isolate_pageblock == get_pageblock_isolate(page))
+		return false;
 
 	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
 		return false;
@@ -1886,7 +1891,10 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 
 		del_page_from_free_list(buddy, zone, order,
 					get_pfnblock_migratetype(buddy, pfn));
-		set_pageblock_migratetype(page, migratetype);
+		if (isolate_pageblock)
+			set_pageblock_isolate(page);
+		else
+			clear_pageblock_isolate(page);
 		split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
 		return true;
 	}
@@ -1897,16 +1905,38 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 
 		del_page_from_free_list(page, zone, order,
 					get_pfnblock_migratetype(page, pfn));
-		set_pageblock_migratetype(page, migratetype);
+		if (isolate_pageblock)
+			set_pageblock_isolate(page);
+		else
+			clear_pageblock_isolate(page);
 		split_large_buddy(zone, page, pfn, order, FPI_NONE);
 		return true;
 	}
 move:
-	__move_freepages_block(zone, start_pfn,
-			       get_pfnblock_migratetype(page, start_pfn),
-			       migratetype);
+	if (isolate_pageblock) {
+		from_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
+				MIGRATETYPE_MASK);
+		to_mt = MIGRATE_ISOLATE;
+	} else {
+		from_mt = MIGRATE_ISOLATE;
+		to_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
+				MIGRATETYPE_MASK);
+	}
+
+	__move_freepages_block(zone, start_pfn, from_mt, to_mt);
 	return true;
 }
+
+bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page)
+{
+	return __move_freepages_for_page_isolation(zone, page, true);
+}
+
+bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page)
+{
+	return __move_freepages_for_page_isolation(zone, page, false);
+}
+
 #endif /* CONFIG_MEMORY_ISOLATION */
 
 static void change_pageblock_range(struct page *pageblock_page,
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 8ed53ee00f2a..01d9a4eace7a 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -188,7 +188,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	unmovable = has_unmovable_pages(check_unmovable_start, check_unmovable_end,
 			migratetype, isol_flags);
 	if (!unmovable) {
-		if (!move_freepages_block_isolate(zone, page, MIGRATE_ISOLATE)) {
+		if (!pageblock_isolate_and_move_free_pages(zone, page)) {
 			spin_unlock_irqrestore(&zone->lock, flags);
 			return -EBUSY;
 		}
@@ -209,7 +209,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	return -EBUSY;
 }
 
-static void unset_migratetype_isolate(struct page *page, int migratetype)
+static void unset_migratetype_isolate(struct page *page)
 {
 	struct zone *zone;
 	unsigned long flags;
@@ -262,10 +262,10 @@ static void unset_migratetype_isolate(struct page *page, int migratetype)
 		 * Isolating this block already succeeded, so this
 		 * should not fail on zone boundaries.
 		 */
-		WARN_ON_ONCE(!move_freepages_block_isolate(zone, page, migratetype));
+		WARN_ON_ONCE(!pageblock_unisolate_and_move_free_pages(zone, page));
 	} else {
-		set_pageblock_migratetype(page, migratetype);
-		__putback_isolated_page(page, order, migratetype);
+		clear_pageblock_isolate(page);
+		__putback_isolated_page(page, order, get_pageblock_migratetype(page));
 	}
 	zone->nr_isolate_pageblock--;
 out:
@@ -383,7 +383,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
 		if (PageBuddy(page)) {
 			int order = buddy_order(page);
 
-			/* move_freepages_block_isolate() handled this */
+			/* pageblock_isolate_and_move_free_pages() handled this */
 			VM_WARN_ON_ONCE(pfn + (1 << order) > boundary_pfn);
 
 			pfn += 1UL << order;
@@ -433,7 +433,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
 failed:
 	/* restore the original migratetype */
 	if (!skip_isolation)
-		unset_migratetype_isolate(pfn_to_page(isolate_pageblock), migratetype);
+		unset_migratetype_isolate(pfn_to_page(isolate_pageblock));
 	return -EBUSY;
 }
 
@@ -504,7 +504,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	ret = isolate_single_pageblock(isolate_end, flags, true,
 			skip_isolation, migratetype);
 	if (ret) {
-		unset_migratetype_isolate(pfn_to_page(isolate_start), migratetype);
+		unset_migratetype_isolate(pfn_to_page(isolate_start));
 		return ret;
 	}
 
@@ -517,8 +517,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 					start_pfn, end_pfn)) {
 			undo_isolate_page_range(isolate_start, pfn, migratetype);
 			unset_migratetype_isolate(
-				pfn_to_page(isolate_end - pageblock_nr_pages),
-				migratetype);
+				pfn_to_page(isolate_end - pageblock_nr_pages));
 			return -EBUSY;
 		}
 	}
@@ -548,7 +547,7 @@ void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 		page = __first_valid_page(pfn, pageblock_nr_pages);
 		if (!page || !is_migrate_isolate_page(page))
 			continue;
-		unset_migratetype_isolate(page, migratetype);
+		unset_migratetype_isolate(page);
 	}
 }
 /*
-- 
2.47.2

[PATCH v2 3/4] mm/page_isolation: remove migratetype from undo_isolate_page_range()

[Zi Yan]

Since migratetype is no longer overwritten during pageblock isolation,
undoing pageblock isolation no longer needs which migratetype to restore.

Signed-off-by: Zi Yan <ziy@nvidia.com>
Acked-by: David Hildenbrand <david@redhat.com>
---
 include/linux/page-isolation.h | 3 +--
 mm/memory_hotplug.c            | 4 ++--
 mm/page_alloc.c                | 2 +-
 mm/page_isolation.c            | 9 +++------
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 28c56f423e34..b8b44d3aacd4 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -33,8 +33,7 @@ bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *pag
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 			     int migratetype, int flags);
 
-void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			     int migratetype);
+void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn);
 
 int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 			int isol_flags);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index e3655f07dd6e..fb2216f267d8 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1229,7 +1229,7 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
 		build_all_zonelists(NULL);
 
 	/* Basic onlining is complete, allow allocation of onlined pages. */
-	undo_isolate_page_range(pfn, pfn + nr_pages, MIGRATE_MOVABLE);
+	undo_isolate_page_range(pfn, pfn + nr_pages);
 
 	/*
 	 * Freshly onlined pages aren't shuffled (e.g., all pages are placed to
@@ -2128,7 +2128,7 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 
 failed_removal_isolated:
 	/* pushback to free area */
-	undo_isolate_page_range(start_pfn, end_pfn, MIGRATE_MOVABLE);
+	undo_isolate_page_range(start_pfn, end_pfn);
 	memory_notify(MEM_CANCEL_OFFLINE, &arg);
 failed_removal_pcplists_disabled:
 	lru_cache_enable();
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9bba5b1c4f1d..4b251aa35b73 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6673,7 +6673,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 		     start, end, outer_start, outer_end);
 	}
 done:
-	undo_isolate_page_range(start, end, migratetype);
+	undo_isolate_page_range(start, end);
 	return ret;
 }
 EXPORT_SYMBOL(alloc_contig_range_noprof);
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 01d9a4eace7a..095cb2152fae 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -515,7 +515,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 		page = __first_valid_page(pfn, pageblock_nr_pages);
 		if (page && set_migratetype_isolate(page, migratetype, flags,
 					start_pfn, end_pfn)) {
-			undo_isolate_page_range(isolate_start, pfn, migratetype);
+			undo_isolate_page_range(isolate_start, pfn);
 			unset_migratetype_isolate(
 				pfn_to_page(isolate_end - pageblock_nr_pages));
 			return -EBUSY;
@@ -528,13 +528,10 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
  * undo_isolate_page_range - undo effects of start_isolate_page_range()
  * @start_pfn:		The first PFN of the isolated range
  * @end_pfn:		The last PFN of the isolated range
- * @migratetype:	New migrate type to set on the range
  *
- * This finds every MIGRATE_ISOLATE page block in the given range
- * and switches it to @migratetype.
+ * This finds and unsets every MIGRATE_ISOLATE page block in the given range
  */
-void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			    int migratetype)
+void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn)
 {
 	unsigned long pfn;
 	struct page *page;
-- 
2.47.2

[PATCH v2 4/4] mm/page_isolation: remove migratetype parameter from more functions.

[Zi Yan]

migratetype is no longer overwritten during pageblock isolation,
start_isolate_page_range(), has_unmovable_pages(), and
set_migratetype_isolate() no longer need which migratetype to restore
during isolation failure.

For has_unmoable_pages(), it needs to know if the isolation is for CMA
allocation, so adding CMA_ALLOCATION to isolation flags to provide the
information.

alloc_contig_range() no longer needs migratetype. Replace it with
a newly defined acr_flags_t to tell if an allocation is for CMA. So does
__alloc_contig_migrate_range().

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 drivers/virtio/virtio_mem.c    |  3 +--
 include/linux/gfp.h            |  6 +++++-
 include/linux/page-isolation.h | 15 +++++++++++---
 include/trace/events/kmem.h    | 14 +++++++------
 mm/cma.c                       |  2 +-
 mm/memory_hotplug.c            |  1 -
 mm/page_alloc.c                | 33 +++++++++++++++----------------
 mm/page_isolation.c            | 36 ++++++++++++----------------------
 8 files changed, 56 insertions(+), 54 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 8a294b9cbcf6..160c681028d2 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1243,8 +1243,7 @@ static int virtio_mem_fake_offline(struct virtio_mem *vm, unsigned long pfn,
 		if (atomic_read(&vm->config_changed))
 			return -EAGAIN;
 
-		rc = alloc_contig_range(pfn, pfn + nr_pages, MIGRATE_MOVABLE,
-					GFP_KERNEL);
+		rc = alloc_contig_range(pfn, pfn + nr_pages, 0, GFP_KERNEL);
 		if (rc == -ENOMEM)
 			/* whoops, out of memory */
 			return rc;
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 6bb1a5a7a4ae..ff1965561d48 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -400,9 +400,13 @@ static inline bool gfp_compaction_allowed(gfp_t gfp_mask)
 extern gfp_t vma_thp_gfp_mask(struct vm_area_struct *vma);
 
 #ifdef CONFIG_CONTIG_ALLOC
+
+typedef unsigned int __bitwise acr_flags_t;
+#define ACR_CMA		((__force acr_flags_t)BIT(0))	// allocate for CMA
+
 /* The below functions must be run on a range from a single zone. */
 extern int alloc_contig_range_noprof(unsigned long start, unsigned long end,
-			      unsigned migratetype, gfp_t gfp_mask);
+			      acr_flags_t alloc_flags, gfp_t gfp_mask);
 #define alloc_contig_range(...)			alloc_hooks(alloc_contig_range_noprof(__VA_ARGS__))
 
 extern struct page *alloc_contig_pages_noprof(unsigned long nr_pages, gfp_t gfp_mask,
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index b8b44d3aacd4..709a807202e9 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -22,8 +22,17 @@ static inline bool is_migrate_isolate(int migratetype)
 }
 #endif
 
-#define MEMORY_OFFLINE	0x1
-#define REPORT_FAILURE	0x2
+/*
+ * Isolation flags:
+ * MEMORY_OFFLINE - isolate to offline (!allocate) memory e.g., skip over
+ *		    PageHWPoison() pages and PageOffline() pages.
+ * REPORT_FAILURE - report details about the failure to isolate the range
+ * CMA_ALLOCATION - isolate for CMA allocations
+ */
+typedef unsigned int __bitwise isol_flags_t;
+#define MEMORY_OFFLINE		((__force isol_flags_t)BIT(0))
+#define REPORT_FAILURE		((__force isol_flags_t)BIT(1))
+#define CMA_ALLOCATION		((__force isol_flags_t)BIT(2))
 
 void set_pageblock_migratetype(struct page *page, int migratetype);
 
@@ -31,7 +40,7 @@ bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page)
 bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page);
 
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			     int migratetype, int flags);
+			     isol_flags_t flags);
 
 void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn);
 
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index b37eb0a7060f..a29471d33566 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -304,6 +304,7 @@ TRACE_EVENT(mm_page_alloc_extfrag,
 		__entry->change_ownership)
 );
 
+#ifdef CONFIG_CONTIG_ALLOC
 TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 
 	TP_PROTO(unsigned long start,
@@ -311,9 +312,9 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		 unsigned long nr_migrated,
 		 unsigned long nr_reclaimed,
 		 unsigned long nr_mapped,
-		 int migratetype),
+		 acr_flags_t alloc_flags),
 
-	TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, migratetype),
+	TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_flags),
 
 	TP_STRUCT__entry(
 		__field(unsigned long, start)
@@ -321,7 +322,7 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		__field(unsigned long, nr_migrated)
 		__field(unsigned long, nr_reclaimed)
 		__field(unsigned long, nr_mapped)
-		__field(int, migratetype)
+		__field(acr_flags_t, alloc_flags)
 	),
 
 	TP_fast_assign(
@@ -330,17 +331,18 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
 		__entry->nr_migrated = nr_migrated;
 		__entry->nr_reclaimed = nr_reclaimed;
 		__entry->nr_mapped = nr_mapped;
-		__entry->migratetype = migratetype;
+		__entry->alloc_flags = alloc_flags;
 	),
 
-	TP_printk("start=0x%lx end=0x%lx migratetype=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
+	TP_printk("start=0x%lx end=0x%lx alloc_flags=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
 		  __entry->start,
 		  __entry->end,
-		  __entry->migratetype,
+		  __entry->alloc_flags,
 		  __entry->nr_migrated,
 		  __entry->nr_reclaimed,
 		  __entry->nr_mapped)
 );
+#endif
 
 /*
  * Required for uniquely and securely identifying mm in rss_stat tracepoint.
diff --git a/mm/cma.c b/mm/cma.c
index de5bc0c81fc2..e4ad8a5f8bde 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -461,7 +461,7 @@ static struct page *__cma_alloc(struct cma *cma, unsigned long count,
 
 		pfn = cma->base_pfn + (bitmap_no << cma->order_per_bit);
 		mutex_lock(&cma_mutex);
-		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA, gfp);
+		ret = alloc_contig_range(pfn, pfn + count, ACR_CMA, gfp);
 		mutex_unlock(&cma_mutex);
 		if (ret == 0) {
 			page = pfn_to_page(pfn);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index fb2216f267d8..b69fb43c5ac7 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2018,7 +2018,6 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 
 	/* set above range as isolated */
 	ret = start_isolate_page_range(start_pfn, end_pfn,
-				       MIGRATE_MOVABLE,
 				       MEMORY_OFFLINE | REPORT_FAILURE);
 	if (ret) {
 		reason = "failure to isolate range";
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4b251aa35b73..3a9328f7b2e7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1913,14 +1913,15 @@ static bool __move_freepages_for_page_isolation(struct zone *zone,
 		return true;
 	}
 move:
+	/* use MIGRATETYPE_NO_ISO_MASK to get the non-isolate migratetype */
 	if (isolate_pageblock) {
-		from_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
-				MIGRATETYPE_MASK);
+		from_mt = get_pfnblock_flags_mask(page, page_to_pfn(page),
+				MIGRATETYPE_NO_ISO_MASK);
 		to_mt = MIGRATE_ISOLATE;
 	} else {
 		from_mt = MIGRATE_ISOLATE;
-		to_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
-				MIGRATETYPE_MASK);
+		to_mt = get_pfnblock_flags_mask(page, page_to_pfn(page),
+				MIGRATETYPE_NO_ISO_MASK);
 	}
 
 	__move_freepages_block(zone, start_pfn, from_mt, to_mt);
@@ -6382,11 +6383,12 @@ static void alloc_contig_dump_pages(struct list_head *page_list)
 
 /*
  * [start, end) must belong to a single zone.
- * @migratetype: using migratetype to filter the type of migration in
+ * @alloc_flags: using acr_flags_t to filter the type of migration in
  *		trace_mm_alloc_contig_migrate_range_info.
  */
-static int __alloc_contig_migrate_range(struct compact_control *cc,
-		unsigned long start, unsigned long end, int migratetype)
+int __alloc_contig_migrate_range(struct compact_control *cc,
+					unsigned long start, unsigned long end,
+					acr_flags_t alloc_flags)
 {
 	/* This function is based on compact_zone() from compaction.c. */
 	unsigned int nr_reclaimed;
@@ -6458,7 +6460,7 @@ static int __alloc_contig_migrate_range(struct compact_control *cc,
 		putback_movable_pages(&cc->migratepages);
 	}
 
-	trace_mm_alloc_contig_migrate_range_info(start, end, migratetype,
+	trace_mm_alloc_contig_migrate_range_info(start, end, alloc_flags,
 						 total_migrated,
 						 total_reclaimed,
 						 total_mapped);
@@ -6529,10 +6531,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  * alloc_contig_range() -- tries to allocate given range of pages
  * @start:	start PFN to allocate
  * @end:	one-past-the-last PFN to allocate
- * @migratetype:	migratetype of the underlying pageblocks (either
- *			#MIGRATE_MOVABLE or #MIGRATE_CMA).  All pageblocks
- *			in range must have the same migratetype and it must
- *			be either of the two.
+ * @alloc_flags:	allocation information
  * @gfp_mask:	GFP mask. Node/zone/placement hints are ignored; only some
  *		action and reclaim modifiers are supported. Reclaim modifiers
  *		control allocation behavior during compaction/migration/reclaim.
@@ -6549,7 +6548,7 @@ static int __alloc_contig_verify_gfp_mask(gfp_t gfp_mask, gfp_t *gfp_cc_mask)
  * need to be freed with free_contig_range().
  */
 int alloc_contig_range_noprof(unsigned long start, unsigned long end,
-		       unsigned migratetype, gfp_t gfp_mask)
+			acr_flags_t alloc_flags, gfp_t gfp_mask)
 {
 	unsigned long outer_start, outer_end;
 	int ret = 0;
@@ -6590,7 +6589,8 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	 * put back to page allocator so that buddy can use them.
 	 */
 
-	ret = start_isolate_page_range(start, end, migratetype, 0);
+	ret = start_isolate_page_range(start, end,
+			(alloc_flags & ACR_CMA) ? CMA_ALLOCATION : 0);
 	if (ret)
 		goto done;
 
@@ -6606,7 +6606,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	 * allocated.  So, if we fall through be sure to clear ret so that
 	 * -EBUSY is not accidentally used or returned to caller.
 	 */
-	ret = __alloc_contig_migrate_range(&cc, start, end, migratetype);
+	ret = __alloc_contig_migrate_range(&cc, start, end, alloc_flags);
 	if (ret && ret != -EBUSY)
 		goto done;
 
@@ -6683,8 +6683,7 @@ static int __alloc_contig_pages(unsigned long start_pfn,
 {
 	unsigned long end_pfn = start_pfn + nr_pages;
 
-	return alloc_contig_range_noprof(start_pfn, end_pfn, MIGRATE_MOVABLE,
-				   gfp_mask);
+	return alloc_contig_range_noprof(start_pfn, end_pfn, 0, gfp_mask);
 }
 
 static bool pfn_range_valid_contig(struct zone *z, unsigned long start_pfn,
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 095cb2152fae..c39b51e96186 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -31,7 +31,7 @@
  *
  */
 static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long end_pfn,
-				int migratetype, int flags)
+				isol_flags_t flags)
 {
 	struct page *page = pfn_to_page(start_pfn);
 	struct zone *zone = page_zone(page);
@@ -46,7 +46,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
 		 * isolate CMA pageblocks even when they are not movable in fact
 		 * so consider them movable here.
 		 */
-		if (is_migrate_cma(migratetype))
+		if (flags & CMA_ALLOCATION)
 			return NULL;
 
 		return page;
@@ -151,7 +151,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
  * present in [start_pfn, end_pfn). The pageblock must intersect with
  * [start_pfn, end_pfn).
  */
-static int set_migratetype_isolate(struct page *page, int migratetype, int isol_flags,
+static int set_migratetype_isolate(struct page *page, isol_flags_t isol_flags,
 			unsigned long start_pfn, unsigned long end_pfn)
 {
 	struct zone *zone = page_zone(page);
@@ -186,7 +186,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 				  end_pfn);
 
 	unmovable = has_unmovable_pages(check_unmovable_start, check_unmovable_end,
-			migratetype, isol_flags);
+			isol_flags);
 	if (!unmovable) {
 		if (!pageblock_isolate_and_move_free_pages(zone, page)) {
 			spin_unlock_irqrestore(&zone->lock, flags);
@@ -296,7 +296,6 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * @isolate_before:	isolate the pageblock before the boundary_pfn
  * @skip_isolation:	the flag to skip the pageblock isolation in second
  *			isolate_single_pageblock()
- * @migratetype:	migrate type to set in error recovery.
  *
  * Free and in-use pages can be as big as MAX_PAGE_ORDER and contain more than one
  * pageblock. When not all pageblocks within a page are isolated at the same
@@ -311,8 +310,8 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * either. The function handles this by splitting the free page or migrating
  * the in-use page then splitting the free page.
  */
-static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
-		bool isolate_before, bool skip_isolation, int migratetype)
+static int isolate_single_pageblock(unsigned long boundary_pfn, isol_flags_t flags,
+			bool isolate_before, bool skip_isolation)
 {
 	unsigned long start_pfn;
 	unsigned long isolate_pageblock;
@@ -338,11 +337,9 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
 				      zone->zone_start_pfn);
 
 	if (skip_isolation) {
-		int mt __maybe_unused = get_pageblock_migratetype(pfn_to_page(isolate_pageblock));
-
-		VM_BUG_ON(!is_migrate_isolate(mt));
+		VM_BUG_ON(!get_pageblock_isolate(pfn_to_page(isolate_pageblock)));
 	} else {
-		ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock), migratetype,
+		ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock),
 				flags, isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
 
 		if (ret)
@@ -441,14 +438,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
  * start_isolate_page_range() - mark page range MIGRATE_ISOLATE
  * @start_pfn:		The first PFN of the range to be isolated.
  * @end_pfn:		The last PFN of the range to be isolated.
- * @migratetype:	Migrate type to set in error recovery.
- * @flags:		The following flags are allowed (they can be combined in
- *			a bit mask)
- *			MEMORY_OFFLINE - isolate to offline (!allocate) memory
- *					 e.g., skip over PageHWPoison() pages
- *					 and PageOffline() pages.
- *			REPORT_FAILURE - report details about the failure to
- *			isolate the range
+ * @flags:		isolation flags
  *
  * Making page-allocation-type to be MIGRATE_ISOLATE means free pages in
  * the range will never be allocated. Any free pages and pages freed in the
@@ -481,7 +471,7 @@ static int isolate_single_pageblock(unsigned long boundary_pfn, int flags,
  * Return: 0 on success and -EBUSY if any part of range cannot be isolated.
  */
 int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
-			     int migratetype, int flags)
+			     isol_flags_t flags)
 {
 	unsigned long pfn;
 	struct page *page;
@@ -493,7 +483,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 
 	/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
 	ret = isolate_single_pageblock(isolate_start, flags, false,
-			skip_isolation, migratetype);
+			skip_isolation);
 	if (ret)
 		return ret;
 
@@ -502,7 +492,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 
 	/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
 	ret = isolate_single_pageblock(isolate_end, flags, true,
-			skip_isolation, migratetype);
+			skip_isolation);
 	if (ret) {
 		unset_migratetype_isolate(pfn_to_page(isolate_start));
 		return ret;
@@ -513,7 +503,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	     pfn < isolate_end - pageblock_nr_pages;
 	     pfn += pageblock_nr_pages) {
 		page = __first_valid_page(pfn, pageblock_nr_pages);
-		if (page && set_migratetype_isolate(page, migratetype, flags,
+		if (page && set_migratetype_isolate(page, flags,
 					start_pfn, end_pfn)) {
 			undo_isolate_page_range(isolate_start, pfn);
 			unset_migratetype_isolate(
-- 
2.47.2

Re: [PATCH v2 1/4] mm/page_isolation: make page isolation a standalone bit.

[Johannes Weiner]

On Fri, Feb 14, 2025 at 10:42:12AM -0500, Zi Yan wrote:
> During page isolation, the original migratetype is overwritten, since
> MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like
> PB_migrate_skip. pageblock bits needs to be word aligned, so expand
> the number of pageblock bits from 4 to 8 and make migrate isolate bit 7.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>  include/linux/mmzone.h          | 18 +++++++++++++-----
>  include/linux/page-isolation.h  |  2 +-
>  include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++-
>  mm/page_alloc.c                 | 21 +++++++++++++++++++--
>  4 files changed, 65 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 8aecbbb0b685..3c7d3f22ccb2 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt)
>  
>  extern int page_group_by_mobility_disabled;
>  
> -#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1)
> +#ifdef CONFIG_MEMORY_ISOLATION
> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
> +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit)
> +#else
> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
> +#endif

There is no user of the NO_ISO_MASK until the last patch. Can you
please defer introduction until then?

> -#define get_pageblock_migratetype(page)					\
> -	get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
> +#define get_pageblock_migratetype(page) \
> +		get_pfnblock_flags_mask(page, page_to_pfn(page), \
> +			MIGRATETYPE_MASK)
>  
> -#define folio_migratetype(folio)				\
> -	get_pfnblock_flags_mask(&folio->page, folio_pfn(folio),		\
> +#define folio_migratetype(folio) \
> +		get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
>  			MIGRATETYPE_MASK)

That's a spurious change currently. I assume you tweaked the
MIGRATETYPE_MASK parameter during development, but I can't see a
functional difference now.

> @@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page,
>  	 * racy, are not corrupted.
>  	 */
>  	word = READ_ONCE(bitmap[word_bitidx]);
> -	return (word >> bitidx) & mask;
> +	flags = (word >> bitidx) & mask;
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (flags & PB_migrate_isolate_bit)
> +		return MIGRATE_ISOLATE;
> +#endif
> +	return flags;
>  }
>  
>  static __always_inline int get_pfnblock_migratetype(const struct page *page,
> @@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
>  	unsigned long bitidx, word_bitidx;
>  	unsigned long word;
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
> +	/* keep other migratetype bits if MIGRATE_ISOLATE is set */
> +	if (flags == MIGRATE_ISOLATE) {
> +		mask &= ~((1UL << PB_migratetype_bits) - 1);
> +		flags = PB_migrate_isolate_bit;
> +	}

Please change the callers in both cases to pass the appropriate masks
of interest instead.

That's likely a bit of churn in the allocator code, but adding caller
specifics to this function violates abstraction layering rules.

Re: [PATCH v2 2/4] mm/page_isolation: remove migratetype from move_freepages_block_isolate()

[Johannes Weiner]

On Fri, Feb 14, 2025 at 10:42:13AM -0500, Zi Yan wrote:
> Since migratetype is no longer overwritten during pageblock isolation,
> moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.
> 
> Rename move_freepages_block_isolate() to share common code and add
> pageblock_isolate_and_move_free_pages() and
> pageblock_unisolate_and_move_free_pages() to be explicit about the page
> isolation operations.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>  include/linux/page-isolation.h |  4 +--
>  mm/page_alloc.c                | 48 +++++++++++++++++++++++++++-------
>  mm/page_isolation.c            | 21 +++++++--------
>  3 files changed, 51 insertions(+), 22 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 51797dc39cbc..28c56f423e34 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -27,8 +27,8 @@ static inline bool is_migrate_isolate(int migratetype)
>  
>  void set_pageblock_migratetype(struct page *page, int migratetype);
>  
> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
> -				  int migratetype);
> +bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page);
> +bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page);
>  
>  int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>  			     int migratetype, int flags);
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index f17f4acc38c6..9bba5b1c4f1d 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1848,10 +1848,10 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>  }
>  
>  /**
> - * move_freepages_block_isolate - move free pages in block for page isolation
> + * __move_freepages_for_page_isolation - move free pages in block for page isolation
>   * @zone: the zone
>   * @page: the pageblock page
> - * @migratetype: migratetype to set on the pageblock
> + * @isolate_pageblock to isolate the given pageblock or unisolate it
>   *
>   * This is similar to move_freepages_block(), but handles the special
>   * case encountered in page isolation, where the block of interest
> @@ -1866,10 +1866,15 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>   *
>   * Returns %true if pages could be moved, %false otherwise.
>   */
> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
> -				  int migratetype)
> +static bool __move_freepages_for_page_isolation(struct zone *zone,
> +		struct page *page, bool isolate_pageblock)

I'm biased, but I think the old name is fine.

bool isolate?

>  {
>  	unsigned long start_pfn, pfn;
> +	int from_mt;
> +	int to_mt;
> +
> +	if (isolate_pageblock == get_pageblock_isolate(page))
> +		return false;
>  
>  	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
>  		return false;
> @@ -1886,7 +1891,10 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>  
>  		del_page_from_free_list(buddy, zone, order,
>  					get_pfnblock_migratetype(buddy, pfn));
> -		set_pageblock_migratetype(page, migratetype);
> +		if (isolate_pageblock)
> +			set_pageblock_isolate(page);
> +		else
> +			clear_pageblock_isolate(page);

Since this pattern repeats, maybe create a toggle function that does this?

		set_pfnblock_flags_mask(page, (isolate << PB_migrate_isolate),
					page_to_pfn(page),
					(1 << PB_migrate_isolate));

>  		split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
>  		return true;
>  	}
> @@ -1897,16 +1905,38 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>  
>  		del_page_from_free_list(page, zone, order,
>  					get_pfnblock_migratetype(page, pfn));
> -		set_pageblock_migratetype(page, migratetype);
> +		if (isolate_pageblock)
> +			set_pageblock_isolate(page);
> +		else
> +			clear_pageblock_isolate(page);
>  		split_large_buddy(zone, page, pfn, order, FPI_NONE);
>  		return true;
>  	}
>  move:
> -	__move_freepages_block(zone, start_pfn,
> -			       get_pfnblock_migratetype(page, start_pfn),
> -			       migratetype);
> +	if (isolate_pageblock) {
> +		from_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
> +				MIGRATETYPE_MASK);
> +		to_mt = MIGRATE_ISOLATE;
> +	} else {
> +		from_mt = MIGRATE_ISOLATE;
> +		to_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
> +				MIGRATETYPE_MASK);
> +	}
> +
> +	__move_freepages_block(zone, start_pfn, from_mt, to_mt);
>  	return true;

Keeping both MIGRATE_ISOLATE and PB_migrate_isolate encoding the same
state is fragile. At least in the pfnblock bitmask, there should only
be one bit encoding this.

Obviously, there are many places in the allocator that care about
freelist destination: they want MIGRATE_ISOLATE if the bit is set, and
the "actual" type otherwise.

I think what should work is decoupling enum migratetype from the
pageblock bits, and then have a parsing layer on top like this:

enum migratetype {
	MIGRATE_UNMOVABLE,
	...
	MIGRATE_TYPE_BITS,
	MIGRATE_ISOLATE = MIGRATE_TYPE_BITS,
	MIGRATE_TYPES
};

#define PB_migratetype_bits MIGRATE_TYPE_BITS

static enum migratetype get_pageblock_migratetype()
{
	flags = get_pfnblock_flags_mask(..., MIGRATETYPE_MASK | (1 << PB_migrate_isolate));
	if (flags & (1 << PB_migrate_isolate))
		return MIGRATE_ISOLATE;
	return flags;
}

Re: [PATCH v2 1/4] mm/page_isolation: make page isolation a standalone bit.

[Zi Yan]

On 14 Feb 2025, at 11:45, Johannes Weiner wrote:

> On Fri, Feb 14, 2025 at 10:42:12AM -0500, Zi Yan wrote:
>> During page isolation, the original migratetype is overwritten, since
>> MIGRATE_* are enums. Change MIGRATE_ISOLATE to be a standalone bit like
>> PB_migrate_skip. pageblock bits needs to be word aligned, so expand
>> the number of pageblock bits from 4 to 8 and make migrate isolate bit 7.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>  include/linux/mmzone.h          | 18 +++++++++++++-----
>>  include/linux/page-isolation.h  |  2 +-
>>  include/linux/pageblock-flags.h | 33 ++++++++++++++++++++++++++++++++-
>>  mm/page_alloc.c                 | 21 +++++++++++++++++++--
>>  4 files changed, 65 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 8aecbbb0b685..3c7d3f22ccb2 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -106,14 +106,22 @@ static inline bool migratetype_is_mergeable(int mt)
>>
>>  extern int page_group_by_mobility_disabled;
>>
>> -#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1)
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
>> +#define MIGRATETYPE_MASK (MIGRATETYPE_NO_ISO_MASK | PB_migrate_isolate_bit)
>> +#else
>> +#define MIGRATETYPE_NO_ISO_MASK (BIT(PB_migratetype_bits) - 1)
>> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
>> +#endif
>
> There is no user of the NO_ISO_MASK until the last patch. Can you
> please defer introduction until then?

Sure.

>
>> -#define get_pageblock_migratetype(page)					\
>> -	get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
>> +#define get_pageblock_migratetype(page) \
>> +		get_pfnblock_flags_mask(page, page_to_pfn(page), \
>> +			MIGRATETYPE_MASK)
>>
>> -#define folio_migratetype(folio)				\
>> -	get_pfnblock_flags_mask(&folio->page, folio_pfn(folio),		\
>> +#define folio_migratetype(folio) \
>> +		get_pfnblock_flags_mask(&folio->page, folio_pfn(folio), \
>>  			MIGRATETYPE_MASK)
>
> That's a spurious change currently. I assume you tweaked the
> MIGRATETYPE_MASK parameter during development, but I can't see a
> functional difference now.

Right, will remove it.
>
>> @@ -373,7 +374,13 @@ unsigned long get_pfnblock_flags_mask(const struct page *page,
>>  	 * racy, are not corrupted.
>>  	 */
>>  	word = READ_ONCE(bitmap[word_bitidx]);
>> -	return (word >> bitidx) & mask;
>> +	flags = (word >> bitidx) & mask;
>> +
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	if (flags & PB_migrate_isolate_bit)
>> +		return MIGRATE_ISOLATE;
>> +#endif
>> +	return flags;
>>  }
>>
>>  static __always_inline int get_pfnblock_migratetype(const struct page *page,
>> @@ -397,8 +404,18 @@ void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
>>  	unsigned long bitidx, word_bitidx;
>>  	unsigned long word;
>>
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
>> +	/* keep other migratetype bits if MIGRATE_ISOLATE is set */
>> +	if (flags == MIGRATE_ISOLATE) {
>> +		mask &= ~((1UL << PB_migratetype_bits) - 1);
>> +		flags = PB_migrate_isolate_bit;
>> +	}
>
> Please change the callers in both cases to pass the appropriate masks
> of interest instead.
>
> That's likely a bit of churn in the allocator code, but adding caller
> specifics to this function violates abstraction layering rules.

Basically, expose this to the caller. Namely, if it is setting
MIGRATE_ISOLATE, it should use the MIGRATETYPE_ISO_ONLY_MASK. Otherwise,
use MIGRATETYPE_MASK. Maybe adds two helper functions for
setting pageblock isolation and clearing pageblock isolation, like
{set,clear}_pageblock_skip()? I thought about it and did not do that
due to code churn, but now you ask for it explicitly, I am going to
do that.

Thank you for the review.

Best Regards,
Yan, Zi

Re: [PATCH v2 2/4] mm/page_isolation: remove migratetype from move_freepages_block_isolate()

[Zi Yan]

On 14 Feb 2025, at 12:20, Johannes Weiner wrote:

> On Fri, Feb 14, 2025 at 10:42:13AM -0500, Zi Yan wrote:
>> Since migratetype is no longer overwritten during pageblock isolation,
>> moving pageblocks to and from MIGRATE_ISOLATE do not need migratetype.
>>
>> Rename move_freepages_block_isolate() to share common code and add
>> pageblock_isolate_and_move_free_pages() and
>> pageblock_unisolate_and_move_free_pages() to be explicit about the page
>> isolation operations.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>  include/linux/page-isolation.h |  4 +--
>>  mm/page_alloc.c                | 48 +++++++++++++++++++++++++++-------
>>  mm/page_isolation.c            | 21 +++++++--------
>>  3 files changed, 51 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>> index 51797dc39cbc..28c56f423e34 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -27,8 +27,8 @@ static inline bool is_migrate_isolate(int migratetype)
>>
>>  void set_pageblock_migratetype(struct page *page, int migratetype);
>>
>> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>> -				  int migratetype);
>> +bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page);
>> +bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page);
>>
>>  int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
>>  			     int migratetype, int flags);
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index f17f4acc38c6..9bba5b1c4f1d 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -1848,10 +1848,10 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>>  }
>>
>>  /**
>> - * move_freepages_block_isolate - move free pages in block for page isolation
>> + * __move_freepages_for_page_isolation - move free pages in block for page isolation
>>   * @zone: the zone
>>   * @page: the pageblock page
>> - * @migratetype: migratetype to set on the pageblock
>> + * @isolate_pageblock to isolate the given pageblock or unisolate it
>>   *
>>   * This is similar to move_freepages_block(), but handles the special
>>   * case encountered in page isolation, where the block of interest
>> @@ -1866,10 +1866,15 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>>   *
>>   * Returns %true if pages could be moved, %false otherwise.
>>   */
>> -bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>> -				  int migratetype)
>> +static bool __move_freepages_for_page_isolation(struct zone *zone,
>> +		struct page *page, bool isolate_pageblock)
>
> I'm biased, but I think the old name is fine.
>
> bool isolate?

OK. Will use the old name and bool isolate.
>
>>  {
>>  	unsigned long start_pfn, pfn;
>> +	int from_mt;
>> +	int to_mt;
>> +
>> +	if (isolate_pageblock == get_pageblock_isolate(page))
>> +		return false;
>>
>>  	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
>>  		return false;
>> @@ -1886,7 +1891,10 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>>
>>  		del_page_from_free_list(buddy, zone, order,
>>  					get_pfnblock_migratetype(buddy, pfn));
>> -		set_pageblock_migratetype(page, migratetype);
>> +		if (isolate_pageblock)
>> +			set_pageblock_isolate(page);
>> +		else
>> +			clear_pageblock_isolate(page);
>
> Since this pattern repeats, maybe create a toggle function that does this?
>
> 		set_pfnblock_flags_mask(page, (isolate << PB_migrate_isolate),
> 					page_to_pfn(page),
> 					(1 << PB_migrate_isolate));

Sure.

>
>>  		split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
>>  		return true;
>>  	}
>> @@ -1897,16 +1905,38 @@ bool move_freepages_block_isolate(struct zone *zone, struct page *page,
>>
>>  		del_page_from_free_list(page, zone, order,
>>  					get_pfnblock_migratetype(page, pfn));
>> -		set_pageblock_migratetype(page, migratetype);
>> +		if (isolate_pageblock)
>> +			set_pageblock_isolate(page);
>> +		else
>> +			clear_pageblock_isolate(page);
>>  		split_large_buddy(zone, page, pfn, order, FPI_NONE);
>>  		return true;
>>  	}
>>  move:
>> -	__move_freepages_block(zone, start_pfn,
>> -			       get_pfnblock_migratetype(page, start_pfn),
>> -			       migratetype);
>> +	if (isolate_pageblock) {
>> +		from_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
>> +				MIGRATETYPE_MASK);
>> +		to_mt = MIGRATE_ISOLATE;
>> +	} else {
>> +		from_mt = MIGRATE_ISOLATE;
>> +		to_mt = __get_pfnblock_flags_mask(page, page_to_pfn(page),
>> +				MIGRATETYPE_MASK);
>> +	}
>> +
>> +	__move_freepages_block(zone, start_pfn, from_mt, to_mt);
>>  	return true;
>
> Keeping both MIGRATE_ISOLATE and PB_migrate_isolate encoding the same
> state is fragile. At least in the pfnblock bitmask, there should only
> be one bit encoding this.
>
> Obviously, there are many places in the allocator that care about
> freelist destination: they want MIGRATE_ISOLATE if the bit is set, and
> the "actual" type otherwise.
>
> I think what should work is decoupling enum migratetype from the
> pageblock bits, and then have a parsing layer on top like this:
>
> enum migratetype {
> 	MIGRATE_UNMOVABLE,
> 	...
> 	MIGRATE_TYPE_BITS,
> 	MIGRATE_ISOLATE = MIGRATE_TYPE_BITS,
> 	MIGRATE_TYPES
> };
>
> #define PB_migratetype_bits MIGRATE_TYPE_BITS
>
> static enum migratetype get_pageblock_migratetype()
> {
> 	flags = get_pfnblock_flags_mask(..., MIGRATETYPE_MASK | (1 << PB_migrate_isolate));
> 	if (flags & (1 << PB_migrate_isolate))
> 		return MIGRATE_ISOLATE;
> 	return flags;
> }
I had something similar in RFC and change to current implementation
to hide the details. But that is fragile like you said. I will try
your approach in the next version.

Thank you for the reviews. :)

Best Regards,
Yan, Zi