linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit
@ 2025-05-23 19:12 Zi Yan
  2025-05-23 19:12 ` [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

Hi David,

I tried to get rid of pfnblock mask by replacing
{get,set}_pfnblock_flags_mask() with {get,set}_pfnblock_migratetype(),
but get_pfnblock_migratetype() will need to do both test_bit() for
MIGRATE_ISOLATE and try_cmpxchg() for other migratetype, which doubles
its cost. So I made {get,set}_pfnblock_flags_mask() internal and use
them in {get,set}_pfnblock_migratetype(). Let me know your thoughts.

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-05-22-02-14.

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
40GB VM 10 times and see that get_pfnblock_flags_mask() and
set_pfnblock_flags_mask() take about 0.12% and 0.02% of the whole process
respectively with and without this patchset across 3 runs.

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


Changelog
===
From v4[3]:
1. cleaned up existing pageblock flag functions:
   a. added {get,set}_{pfnblock,pageblock}_migratetype() to change
      pageblock migratetype
   b. added {get,set,clear}_pfnblock_bit() to change pageblock
      standalone bit, i.e., PB_migrate_skip and PB_migrate_isolate (added
      in this series).
   c. removed {get,set}_pfnblock_flags_mask().
2. added __NR_PAGEBLOCK_BITS to present the number of pageblock flag bits and
   used roundup_pow_of_two(__NR_PAGEBLOCK_BITS) as NR_PAGEBLOCK_BITS.
3. moved {get,set,clear}_pageblock_isolate() to linux/page-isolation.h.
4. added init_pageblock_migratetype() to initialize a pageblock with a
   migratetype and isolated. It is used by memmap_init_range(), which is
   called by move_pfn_range_to_zone() in online_pages() from
   mm/memory_hotplug.c. Other set_pageblock_migratetype() users are
   changed too except the ones in mm/page_alloc.c.
5. toggle_pageblock_isolate() is reimplemented using __change_bit().
6. set_pageblock_migratetype() gives a warning if a pageblock is changed
   from MIGRATE_ISOLATE to other migratetype.
7. added pb_isolate_mode: MEMORY_OFFLINE, CMA_ALLOCATION, ISOLATE_MODE_OTHERS
   to replace isolate flags.
8. REPORT_FAILURE is removed, since it is only used by MEMORY_OFFLINE.

From v3[2]:
1. kept the original is_migrate_isolate_page()
2. moved {get,set,clear}_pageblock_isolate() to mm/page_isolation.c
3. used a single version for get_pageblock_migratetype() and
   get_pfnblock_migratetype().
4. replace get_pageblock_isolate() with
   get_pageblock_migratetype() == MIGRATE_ISOLATE, a
   get_pageblock_isolate() becomes private in mm/page_isolation.c
5. made set_pageblock_migratetype() not accept MIGRATE_ISOLATE, so that
   people need to use the dedicate {get,set,clear}_pageblock_isolate() APIs.
6. changed online_page() from mm/memory_hotplug.c to first set pageblock
   migratetype to MIGRATE_MOVABLE, then isolate pageblocks.
7. added __maybe_unused to get_pageblock_isolate(), since it is only
   used in VM_BUG_ON(), which could be not present when MM debug is off.
   It is reported by kernel test robot.
7. fixed test_pages_isolated() type issues reported by kernel test
   robot.

From v2[1]:
1. Moved MIGRATETYPE_NO_ISO_MASK to Patch 2, where it is used.
2. Removed spurious changes in Patch 1.
3. Refactored code so that migratetype mask is passed properly for all
callers to {get,set}_pfnblock_flags_mask().
4. Added toggle_pageblock_isolate() for setting and clearing
MIGRATE_ISOLATE.
5. Changed get_pageblock_migratetype() when CONFIG_MEMORY_ISOLATION to
handle MIGRATE_ISOLATE case. It acts like a parsing layer for
get_pfnblock_flags_mask().


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, 8 pageblock flag bits are used.

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.

[1] https://lore.kernel.org/linux-mm/20250214154215.717537-1-ziy@nvidia.com/
[2] https://lore.kernel.org/linux-mm/20250507211059.2211628-2-ziy@nvidia.com/
[3] https://lore.kernel.org/linux-mm/20250509200111.3372279-1-ziy@nvidia.com/

Zi Yan (6):
  mm/page_alloc: pageblock flags functions clean up.
  mm/page_isolation: make page isolation a standalone bit.
  mm/page_alloc: add support for initializing pageblock as isolated.
  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/memory_hotplug.h  |   3 +-
 include/linux/mmzone.h          |  18 +-
 include/linux/page-isolation.h  |  46 ++++-
 include/linux/pageblock-flags.h |  45 +++--
 include/trace/events/kmem.h     |  14 +-
 mm/cma.c                        |   2 +-
 mm/hugetlb.c                    |   4 +-
 mm/internal.h                   |   3 +-
 mm/memory_hotplug.c             |  22 +-
 mm/memremap.c                   |   2 +-
 mm/mm_init.c                    |  24 ++-
 mm/page_alloc.c                 | 342 +++++++++++++++++++++++++-------
 mm/page_isolation.c             |  97 ++++-----
 15 files changed, 441 insertions(+), 190 deletions(-)

-- 
2.47.2



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up.
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-27  9:46   ` Vlastimil Babka
  2025-05-23 19:12 ` [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

No functional change is intended.

1. Add __NR_PAGEBLOCK_BITS for the number of pageblock flag bits and use
   roundup_pow_of_two(__NR_PAGEBLOCK_BITS) as NR_PAGEBLOCK_BITS to take
   right amount of bits for pageblock flags.
2. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
   like PB_migrate_skip.
3. Make {get,set}_pfnblock_flags_mask() internal functions and use
   {get,set}_pfnblock_migratetype() for pageblock migratetype operations.
4. Move pageblock flags common code to get_pfnblock_bitmap_bitidx().
3. Use MIGRATETYPE_MASK to get the migratetype of a pageblock from its
   flags.
4. Use PB_migrate_end in the definition of MIGRATETYPE_MASK instead of
   PB_migrate_bits.
5. Add a comment on is_migrate_cma_folio() to prevent one from changing it
   to use get_pageblock_migratetype() and causing issues.

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

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index b19a98c20de8..39540213d5b9 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -76,8 +76,12 @@ extern const char * const migratetype_names[MIGRATE_TYPES];
 #ifdef CONFIG_CMA
 #  define is_migrate_cma(migratetype) unlikely((migratetype) == MIGRATE_CMA)
 #  define is_migrate_cma_page(_page) (get_pageblock_migratetype(_page) == MIGRATE_CMA)
-#  define is_migrate_cma_folio(folio, pfn)	(MIGRATE_CMA ==		\
-	get_pfnblock_flags_mask(&folio->page, pfn, MIGRATETYPE_MASK))
+/*
+ * __dump_folio() in mm/debug.c passes a folio pointer to on-stack struct folio,
+ * so folio_pfn() cannot be used and pfn is needed.
+ */
+#  define is_migrate_cma_folio(folio, pfn) \
+	(get_pfnblock_migratetype(&folio->page, pfn) == MIGRATE_CMA)
 #else
 #  define is_migrate_cma(migratetype) false
 #  define is_migrate_cma_page(_page) false
@@ -106,14 +110,12 @@ static inline bool migratetype_is_mergeable(int mt)
 
 extern int page_group_by_mobility_disabled;
 
-#define MIGRATETYPE_MASK ((1UL << PB_migratetype_bits) - 1)
+#define get_pageblock_migratetype(page) \
+	get_pfnblock_migratetype(page, page_to_pfn(page))
 
-#define get_pageblock_migratetype(page)					\
-	get_pfnblock_flags_mask(page, page_to_pfn(page), MIGRATETYPE_MASK)
+#define folio_migratetype(folio) \
+	get_pageblock_migratetype(&folio->page)
 
-#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..277d8d92980c 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -25,7 +25,7 @@ static inline bool is_migrate_isolate(int migratetype)
 #define MEMORY_OFFLINE	0x1
 #define REPORT_FAILURE	0x2
 
-void set_pageblock_migratetype(struct page *page, int migratetype);
+void set_pageblock_migratetype(struct page *page, enum migratetype migratetype);
 
 bool move_freepages_block_isolate(struct zone *zone, struct page *page,
 				  int migratetype);
diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index fc6b9c87cb0a..3acbb271a29a 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -25,9 +25,13 @@ enum pageblock_bits {
 	 * Assume the bits will always align on a word. If this assumption
 	 * changes then get/set pageblock needs updating.
 	 */
-	NR_PAGEBLOCK_BITS
+	__NR_PAGEBLOCK_BITS
 };
 
+#define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
+
+#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
+
 #if defined(CONFIG_HUGETLB_PAGE)
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
@@ -65,27 +69,23 @@ extern unsigned int pageblock_order;
 /* Forward declaration */
 struct page;
 
-unsigned long get_pfnblock_flags_mask(const struct page *page,
-				unsigned long pfn,
-				unsigned long mask);
-
-void set_pfnblock_flags_mask(struct page *page,
-				unsigned long flags,
-				unsigned long pfn,
-				unsigned long mask);
+enum migratetype get_pfnblock_migratetype(const struct page *page,
+					  unsigned long pfn);
+bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
+		      enum pageblock_bits pb_bit);
+void set_pfnblock_bit(const struct page *page, unsigned long pfn,
+		      enum pageblock_bits pb_bit);
+void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
+			enum pageblock_bits pb_bit);
 
 /* Declarations for getting and setting flags. See mm/page_alloc.c */
 #ifdef CONFIG_COMPACTION
 #define get_pageblock_skip(page) \
-	get_pfnblock_flags_mask(page, page_to_pfn(page),	\
-			(1 << (PB_migrate_skip)))
+	get_pfnblock_bit(page, page_to_pfn(page), PB_migrate_skip)
 #define clear_pageblock_skip(page) \
-	set_pfnblock_flags_mask(page, 0, page_to_pfn(page),	\
-			(1 << PB_migrate_skip))
+	clear_pfnblock_bit(page, page_to_pfn(page), PB_migrate_skip)
 #define set_pageblock_skip(page) \
-	set_pfnblock_flags_mask(page, (1 << PB_migrate_skip),	\
-			page_to_pfn(page),			\
-			(1 << PB_migrate_skip))
+	set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_skip)
 #else
 static inline bool get_pageblock_skip(struct page *page)
 {
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index b1caedbade5b..4ce5210ea56e 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -797,7 +797,7 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 
 	/*
 	 * TODO now we have a visible range of pages which are not associated
-	 * with their zone properly. Not nice but set_pfnblock_flags_mask
+	 * with their zone properly. Not nice but set_pfnblock_migratetype()
 	 * expects the zone spans the pfn range. All the pages in the range
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 90b06f3d004c..0207164fcaf6 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -353,81 +353,172 @@ static inline int pfn_to_bitidx(const struct page *page, unsigned long pfn)
 	return (pfn >> pageblock_order) * NR_PAGEBLOCK_BITS;
 }
 
+static __always_inline void
+get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
+			   unsigned long **bitmap_word, unsigned long *bitidx)
+{
+	unsigned long *bitmap;
+	unsigned long word_bitidx;
+
+	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
+	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
+	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
+
+	bitmap = get_pageblock_bitmap(page, pfn);
+	*bitidx = pfn_to_bitidx(page, pfn);
+	word_bitidx = *bitidx / BITS_PER_LONG;
+	*bitidx &= (BITS_PER_LONG - 1);
+	*bitmap_word = &bitmap[word_bitidx];
+}
+
+
 /**
- * get_pfnblock_flags_mask - Return the requested group of flags for the pageblock_nr_pages block of pages
+ * __get_pfnblock_flags_mask - Return the requested group of flags for
+ * a pageblock_nr_pages block of pages
  * @page: The page within the block of interest
  * @pfn: The target page frame number
  * @mask: mask of bits that the caller is interested in
  *
  * Return: pageblock_bits flags
  */
-unsigned long get_pfnblock_flags_mask(const struct page *page,
-					unsigned long pfn, unsigned long mask)
+static unsigned long __get_pfnblock_flags_mask(const struct page *page,
+					       unsigned long pfn,
+					       unsigned long mask)
 {
-	unsigned long *bitmap;
-	unsigned long bitidx, word_bitidx;
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
 	unsigned long word;
 
-	bitmap = get_pageblock_bitmap(page, pfn);
-	bitidx = pfn_to_bitidx(page, pfn);
-	word_bitidx = bitidx / BITS_PER_LONG;
-	bitidx &= (BITS_PER_LONG-1);
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
 	/*
-	 * This races, without locks, with set_pfnblock_flags_mask(). Ensure
+	 * This races, without locks, with set_pfnblock_migratetype(). Ensure
 	 * a consistent read of the memory array, so that results, even though
 	 * racy, are not corrupted.
 	 */
-	word = READ_ONCE(bitmap[word_bitidx]);
+	word = READ_ONCE(*bitmap_word);
 	return (word >> bitidx) & mask;
 }
 
-static __always_inline int get_pfnblock_migratetype(const struct page *page,
-					unsigned long pfn)
+/**
+ * get_pfnblock_bit - Check if a standalone bit of a pageblock is set
+ * @page: The page within the block of interest
+ * @pfn: The target page frame number
+ * @pb_bit: pageblock bit to check
+ *
+ * Return: true if the bit is set, otherwise false
+ */
+bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
+		      enum pageblock_bits pb_bit)
 {
-	return get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
+
+	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
+			 pb_bit >= __NR_PAGEBLOCK_BITS))
+		return false;
+
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+	return test_bit(bitidx + pb_bit, bitmap_word);
 }
 
 /**
- * set_pfnblock_flags_mask - Set the requested group of flags for a pageblock_nr_pages block of pages
+ * get_pfnblock_migratetype - Return the migratetype of a pageblock
+ * @page: The page within the block of interest
+ * @pfn: The target page frame number
+ *
+ * Return: The migratetype of the pageblock
+ *
+ * Use get_pfnblock_migratetype() if caller already has both @page and @pfn
+ * to save a call to page_to_pfn().
+ */
+__always_inline enum migratetype
+get_pfnblock_migratetype(const struct page *page, unsigned long pfn)
+{
+	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
+}
+
+/**
+ * __set_pfnblock_flags_mask - Set the requested group of flags for
+ * a pageblock_nr_pages block of pages
  * @page: The page within the block of interest
- * @flags: The flags to set
  * @pfn: The target page frame number
+ * @flags: The flags to set
  * @mask: mask of bits that the caller is interested in
  */
-void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
-					unsigned long pfn,
-					unsigned long mask)
+static void __set_pfnblock_flags_mask(struct page *page, unsigned long pfn,
+				      unsigned long flags, unsigned long mask)
 {
-	unsigned long *bitmap;
-	unsigned long bitidx, word_bitidx;
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
 	unsigned long word;
 
-	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
-	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
-
-	bitmap = get_pageblock_bitmap(page, pfn);
-	bitidx = pfn_to_bitidx(page, pfn);
-	word_bitidx = bitidx / BITS_PER_LONG;
-	bitidx &= (BITS_PER_LONG-1);
-
-	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
 
 	mask <<= bitidx;
 	flags <<= bitidx;
 
-	word = READ_ONCE(bitmap[word_bitidx]);
+	word = READ_ONCE(*bitmap_word);
 	do {
-	} while (!try_cmpxchg(&bitmap[word_bitidx], &word, (word & ~mask) | flags));
+	} while (!try_cmpxchg(bitmap_word, &word, (word & ~mask) | flags));
+}
+
+/**
+ * set_pfnblock_bit - Set a standalone bit of a pageblock
+ * @page: The page within the block of interest
+ * @pfn: The target page frame number
+ * @pb_bit: pageblock bit to set
+ */
+void set_pfnblock_bit(const struct page *page, unsigned long pfn,
+		      enum pageblock_bits pb_bit)
+{
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
+
+	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
+			 pb_bit >= __NR_PAGEBLOCK_BITS))
+		return;
+
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+	__set_bit(bitidx + pb_bit, bitmap_word);
+}
+
+/**
+ * clear_pfnblock_bit - Clear a standalone bit of a pageblock
+ * @page: The page within the block of interest
+ * @pfn: The target page frame number
+ * @pb_bit: pageblock bit to clear
+ */
+void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
+			enum pageblock_bits pb_bit)
+{
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
+
+	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
+			 pb_bit >= __NR_PAGEBLOCK_BITS))
+		return;
+
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+	__clear_bit(bitidx + pb_bit, bitmap_word);
 }
 
-void set_pageblock_migratetype(struct page *page, int migratetype)
+/**
+ * set_pageblock_migratetype - Set the migratetype of a pageblock
+ * @page: The page within the block of interest
+ * @migratetype: migratetype to set
+ */
+__always_inline void set_pageblock_migratetype(struct page *page,
+					       enum migratetype migratetype)
 {
 	if (unlikely(page_group_by_mobility_disabled &&
 		     migratetype < MIGRATE_PCPTYPES))
 		migratetype = MIGRATE_UNMOVABLE;
 
-	set_pfnblock_flags_mask(page, (unsigned long)migratetype,
-				page_to_pfn(page), MIGRATETYPE_MASK);
+	__set_pfnblock_flags_mask(page, page_to_pfn(page),
+				  (unsigned long)migratetype, MIGRATETYPE_MASK);
 }
 
 #ifdef CONFIG_DEBUG_VM
@@ -667,7 +758,7 @@ static inline void __add_to_free_list(struct page *page, struct zone *zone,
 	int nr_pages = 1 << order;
 
 	VM_WARN_ONCE(get_pageblock_migratetype(page) != migratetype,
-		     "page type is %lu, passed migratetype is %d (nr=%d)\n",
+		     "page type is %d, passed migratetype is %d (nr=%d)\n",
 		     get_pageblock_migratetype(page), migratetype, nr_pages);
 
 	if (tail)
@@ -693,7 +784,7 @@ static inline void move_to_free_list(struct page *page, struct zone *zone,
 
 	/* Free page moving can fail, so it happens before the type update */
 	VM_WARN_ONCE(get_pageblock_migratetype(page) != old_mt,
-		     "page type is %lu, passed migratetype is %d (nr=%d)\n",
+		     "page type is %d, passed migratetype is %d (nr=%d)\n",
 		     get_pageblock_migratetype(page), old_mt, nr_pages);
 
 	list_move_tail(&page->buddy_list, &area->free_list[new_mt]);
@@ -715,7 +806,7 @@ static inline void __del_page_from_free_list(struct page *page, struct zone *zon
 	int nr_pages = 1 << order;
 
         VM_WARN_ONCE(get_pageblock_migratetype(page) != migratetype,
-		     "page type is %lu, passed migratetype is %d (nr=%d)\n",
+		     "page type is %d, passed migratetype is %d (nr=%d)\n",
 		     get_pageblock_migratetype(page), migratetype, nr_pages);
 
 	/* clear reported state and update reported page count */
@@ -3127,7 +3218,7 @@ static struct page *rmqueue_pcplist(struct zone *preferred_zone,
 
 /*
  * Do not instrument rmqueue() with KMSAN. This function may call
- * __msan_poison_alloca() through a call to set_pfnblock_flags_mask().
+ * __msan_poison_alloca() through a call to set_pfnblock_migratetype().
  * If __msan_poison_alloca() attempts to allocate pages for the stack depot, it
  * may call rmqueue() again, which will result in a deadlock.
  */
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit.
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
  2025-05-23 19:12 ` [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-27 10:11   ` Vlastimil Babka
  2025-05-23 19:12 ` [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

During page isolation, the original migratetype is overwritten, since
MIGRATE_* are enums and stored in pageblock bitmaps. Change
MIGRATE_ISOLATE to be stored a standalone bit, PB_migrate_isolate, like
PB_migrate_skip, so that migratetype is not lost during pageblock
isolation.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/page-isolation.h  | 16 ++++++++++++++++
 include/linux/pageblock-flags.h | 13 ++++++++++++-
 mm/page_alloc.c                 | 33 +++++++++++++++++++++++++++++++--
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 277d8d92980c..fc021d3f95ca 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -11,6 +11,12 @@ static inline bool is_migrate_isolate(int migratetype)
 {
 	return migratetype == MIGRATE_ISOLATE;
 }
+#define get_pageblock_isolate(page) \
+	get_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate)
+#define clear_pageblock_isolate(page) \
+	clear_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate)
+#define set_pageblock_isolate(page) \
+	set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate)
 #else
 static inline bool is_migrate_isolate_page(struct page *page)
 {
@@ -20,6 +26,16 @@ static inline bool is_migrate_isolate(int migratetype)
 {
 	return false;
 }
+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
 
 #define MEMORY_OFFLINE	0x1
diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index 3acbb271a29a..f2f8540b95ca 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -20,7 +20,13 @@ 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
+	/*
+	 * Pageblock isolation is represented with a separate bit, so that
+	 * the migratetype of a block is not overwritten by isolation.
+	 */
+	PB_migrate_isolate, /* If set the block is isolated */
+#endif
 	/*
 	 * Assume the bits will always align on a word. If this assumption
 	 * changes then get/set pageblock needs updating.
@@ -32,6 +38,11 @@ enum pageblock_bits {
 
 #define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
 
+#ifdef CONFIG_MEMORY_ISOLATION
+#define MIGRATETYPE_AND_ISO_MASK \
+	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
+#endif
+
 #if defined(CONFIG_HUGETLB_PAGE)
 
 #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0207164fcaf6..b2c623699461 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -360,8 +360,14 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
 	unsigned long *bitmap;
 	unsigned long word_bitidx;
 
+#ifdef CONFIG_MEMORY_ISOLATION
+	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
+	/* extra one for MIGRATE_ISOLATE */
+	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1);
+#else
 	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
 	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
+#endif
 	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
 
 	bitmap = get_pageblock_bitmap(page, pfn);
@@ -435,7 +441,20 @@ bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
 __always_inline enum migratetype
 get_pfnblock_migratetype(const struct page *page, unsigned long pfn)
 {
-	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
+	unsigned long mask = MIGRATETYPE_MASK;
+	unsigned long flags;
+
+#ifdef CONFIG_MEMORY_ISOLATION
+	mask = MIGRATETYPE_AND_ISO_MASK;
+#endif
+
+	flags = __get_pfnblock_flags_mask(page, pfn, mask);
+
+#ifdef CONFIG_MEMORY_ISOLATION
+	if (flags & BIT(PB_migrate_isolate))
+		return MIGRATE_ISOLATE;
+#endif
+	return flags & MIGRATETYPE_MASK;
 }
 
 /**
@@ -513,12 +532,22 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
 __always_inline void set_pageblock_migratetype(struct page *page,
 					       enum migratetype migratetype)
 {
+	unsigned long mask = MIGRATETYPE_MASK;
+
 	if (unlikely(page_group_by_mobility_disabled &&
 		     migratetype < MIGRATE_PCPTYPES))
 		migratetype = MIGRATE_UNMOVABLE;
 
+#ifdef CONFIG_MEMORY_ISOLATION
+	if (migratetype == MIGRATE_ISOLATE) {
+		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
+		return;
+	}
+	/* change mask to clear PB_migrate_isolate if it is set */
+	mask = MIGRATETYPE_AND_ISO_MASK;
+#endif
 	__set_pfnblock_flags_mask(page, page_to_pfn(page),
-				  (unsigned long)migratetype, MIGRATETYPE_MASK);
+				  (unsigned long)migratetype, mask);
 }
 
 #ifdef CONFIG_DEBUG_VM
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
  2025-05-23 19:12 ` [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
  2025-05-23 19:12 ` [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-27 10:31   ` Vlastimil Babka
  2025-05-23 19:12 ` [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

MIGRATE_ISOLATE is a standalone bit, so a pageblock cannot be initialized
to just MIGRATE_ISOLATE. Add init_pageblock_migratetype() to enable
initialize a pageblock with a migratetype and isolated.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 include/linux/memory_hotplug.h |  3 ++-
 include/linux/page-isolation.h |  3 +++
 mm/hugetlb.c                   |  4 ++--
 mm/internal.h                  |  3 ++-
 mm/memory_hotplug.c            | 12 ++++++++----
 mm/memremap.c                  |  2 +-
 mm/mm_init.c                   | 24 +++++++++++++++---------
 mm/page_alloc.c                | 26 ++++++++++++++++++++++++++
 8 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index eaac5ae8c05c..23f038a16231 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -314,7 +314,8 @@ extern int add_memory_driver_managed(int nid, u64 start, u64 size,
 				     mhp_t mhp_flags);
 extern void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 				   unsigned long nr_pages,
-				   struct vmem_altmap *altmap, int migratetype);
+				   struct vmem_altmap *altmap, int migratetype,
+				   bool isolate_pageblock);
 extern void remove_pfn_range_from_zone(struct zone *zone,
 				       unsigned long start_pfn,
 				       unsigned long nr_pages);
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index fc021d3f95ca..14c6a5f691c2 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -41,6 +41,9 @@ static inline void set_pageblock_isolate(struct page *page)
 #define MEMORY_OFFLINE	0x1
 #define REPORT_FAILURE	0x2
 
+void __meminit init_pageblock_migratetype(struct page *page,
+					  enum migratetype migratetype,
+					  bool isolate);
 void set_pageblock_migratetype(struct page *page, enum migratetype migratetype);
 
 bool move_freepages_block_isolate(struct zone *zone, struct page *page,
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 5a0bf1ea48f9..001c8ad48819 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3311,8 +3311,8 @@ static void __init hugetlb_bootmem_init_migratetype(struct folio *folio,
 		if (folio_test_hugetlb_cma(folio))
 			init_cma_pageblock(folio_page(folio, i));
 		else
-			set_pageblock_migratetype(folio_page(folio, i),
-					  MIGRATE_MOVABLE);
+			init_pageblock_migratetype(folio_page(folio, i),
+					  MIGRATE_MOVABLE, false);
 	}
 }
 
diff --git a/mm/internal.h b/mm/internal.h
index 6b8ed2017743..c43180bea6b4 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -821,7 +821,8 @@ extern void *memmap_alloc(phys_addr_t size, phys_addr_t align,
 			  int nid, bool exact_nid);
 
 void memmap_init_range(unsigned long, int, unsigned long, unsigned long,
-		unsigned long, enum meminit_context, struct vmem_altmap *, int);
+		unsigned long, enum meminit_context, struct vmem_altmap *, int,
+		bool);
 
 #if defined CONFIG_COMPACTION || defined CONFIG_CMA
 
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 4ce5210ea56e..43ac34ee8d2e 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -770,7 +770,8 @@ static inline void section_taint_zone_device(unsigned long pfn)
  */
 void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 				  unsigned long nr_pages,
-				  struct vmem_altmap *altmap, int migratetype)
+				  struct vmem_altmap *altmap, int migratetype,
+				  bool isolate_pageblock)
 {
 	struct pglist_data *pgdat = zone->zone_pgdat;
 	int nid = pgdat->node_id;
@@ -802,7 +803,8 @@ void move_pfn_range_to_zone(struct zone *zone, unsigned long start_pfn,
 	 * are reserved so nobody should be touching them so we should be safe
 	 */
 	memmap_init_range(nr_pages, nid, zone_idx(zone), start_pfn, 0,
-			 MEMINIT_HOTPLUG, altmap, migratetype);
+			 MEMINIT_HOTPLUG, altmap, migratetype,
+			 isolate_pageblock);
 
 	set_zone_contiguous(zone);
 }
@@ -1127,7 +1129,8 @@ int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
 	if (mhp_off_inaccessible)
 		page_init_poison(pfn_to_page(pfn), sizeof(struct page) * nr_pages);
 
-	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_UNMOVABLE);
+	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_UNMOVABLE,
+			false);
 
 	for (i = 0; i < nr_pages; i++) {
 		struct page *page = pfn_to_page(pfn + i);
@@ -1192,7 +1195,8 @@ int online_pages(unsigned long pfn, unsigned long nr_pages,
 
 
 	/* associate pfn range with the zone */
-	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_ISOLATE);
+	move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_MOVABLE,
+			       true);
 
 	arg.start_pfn = pfn;
 	arg.nr_pages = nr_pages;
diff --git a/mm/memremap.c b/mm/memremap.c
index c417c843e9b1..3319e7cc2898 100644
--- a/mm/memremap.c
+++ b/mm/memremap.c
@@ -254,7 +254,7 @@ static int pagemap_range(struct dev_pagemap *pgmap, struct mhp_params *params,
 		zone = &NODE_DATA(nid)->node_zones[ZONE_DEVICE];
 		move_pfn_range_to_zone(zone, PHYS_PFN(range->start),
 				PHYS_PFN(range_len(range)), params->altmap,
-				MIGRATE_MOVABLE);
+				MIGRATE_MOVABLE, false);
 	}
 
 	mem_hotplug_done();
diff --git a/mm/mm_init.c b/mm/mm_init.c
index 1c5444e188f8..a3d9ea02e07b 100644
--- a/mm/mm_init.c
+++ b/mm/mm_init.c
@@ -685,7 +685,8 @@ void __meminit __init_page_from_nid(unsigned long pfn, int nid)
 	__init_single_page(pfn_to_page(pfn), pfn, zid, nid);
 
 	if (pageblock_aligned(pfn))
-		set_pageblock_migratetype(pfn_to_page(pfn), MIGRATE_MOVABLE);
+		init_pageblock_migratetype(pfn_to_page(pfn), MIGRATE_MOVABLE,
+				false);
 }
 
 #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
@@ -874,7 +875,8 @@ static void __init init_unavailable_range(unsigned long spfn,
 void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone,
 		unsigned long start_pfn, unsigned long zone_end_pfn,
 		enum meminit_context context,
-		struct vmem_altmap *altmap, int migratetype)
+		struct vmem_altmap *altmap, int migratetype,
+		bool isolate_pageblock)
 {
 	unsigned long pfn, end_pfn = start_pfn + size;
 	struct page *page;
@@ -931,7 +933,8 @@ void __meminit memmap_init_range(unsigned long size, int nid, unsigned long zone
 		 * over the place during system boot.
 		 */
 		if (pageblock_aligned(pfn)) {
-			set_pageblock_migratetype(page, migratetype);
+			init_pageblock_migratetype(page, migratetype,
+					isolate_pageblock);
 			cond_resched();
 		}
 		pfn++;
@@ -954,7 +957,8 @@ static void __init memmap_init_zone_range(struct zone *zone,
 		return;
 
 	memmap_init_range(end_pfn - start_pfn, nid, zone_id, start_pfn,
-			  zone_end_pfn, MEMINIT_EARLY, NULL, MIGRATE_MOVABLE);
+			  zone_end_pfn, MEMINIT_EARLY, NULL, MIGRATE_MOVABLE,
+			  false);
 
 	if (*hole_pfn < start_pfn)
 		init_unavailable_range(*hole_pfn, start_pfn, zone_id, nid);
@@ -1035,7 +1039,7 @@ static void __ref __init_zone_device_page(struct page *page, unsigned long pfn,
 	 * because this is done early in section_activate()
 	 */
 	if (pageblock_aligned(pfn)) {
-		set_pageblock_migratetype(page, MIGRATE_MOVABLE);
+		init_pageblock_migratetype(page, MIGRATE_MOVABLE, false);
 		cond_resched();
 	}
 
@@ -1996,7 +2000,8 @@ static void __init deferred_free_pages(unsigned long pfn,
 	/* Free a large naturally-aligned chunk if possible */
 	if (nr_pages == MAX_ORDER_NR_PAGES && IS_MAX_ORDER_ALIGNED(pfn)) {
 		for (i = 0; i < nr_pages; i += pageblock_nr_pages)
-			set_pageblock_migratetype(page + i, MIGRATE_MOVABLE);
+			init_pageblock_migratetype(page + i, MIGRATE_MOVABLE,
+					false);
 		__free_pages_core(page, MAX_PAGE_ORDER, MEMINIT_EARLY);
 		return;
 	}
@@ -2006,7 +2011,8 @@ static void __init deferred_free_pages(unsigned long pfn,
 
 	for (i = 0; i < nr_pages; i++, page++, pfn++) {
 		if (pageblock_aligned(pfn))
-			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
+			init_pageblock_migratetype(page, MIGRATE_MOVABLE,
+					false);
 		__free_pages_core(page, 0, MEMINIT_EARLY);
 	}
 }
@@ -2305,7 +2311,7 @@ void __init init_cma_reserved_pageblock(struct page *page)
 		set_page_count(p, 0);
 	} while (++p, --i);
 
-	set_pageblock_migratetype(page, MIGRATE_CMA);
+	init_pageblock_migratetype(page, MIGRATE_CMA, false);
 	set_page_refcounted(page);
 	/* pages were reserved and not allocated */
 	clear_page_tag_ref(page);
@@ -2319,7 +2325,7 @@ void __init init_cma_reserved_pageblock(struct page *page)
  */
 void __init init_cma_pageblock(struct page *page)
 {
-	set_pageblock_migratetype(page, MIGRATE_CMA);
+	init_pageblock_migratetype(page, MIGRATE_CMA, false);
 	adjust_managed_page_count(page, pageblock_nr_pages);
 	page_zone(page)->cma_pages += pageblock_nr_pages;
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index b2c623699461..8fcbd7fa13c2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -550,6 +550,32 @@ __always_inline void set_pageblock_migratetype(struct page *page,
 				  (unsigned long)migratetype, mask);
 }
 
+void __meminit init_pageblock_migratetype(struct page *page,
+					  enum migratetype migratetype,
+					  bool isolate)
+{
+	unsigned long mask = MIGRATETYPE_MASK;
+	unsigned long flags = migratetype;
+
+	if (unlikely(page_group_by_mobility_disabled &&
+		     migratetype < MIGRATE_PCPTYPES))
+		migratetype = MIGRATE_UNMOVABLE;
+
+#ifdef CONFIG_MEMORY_ISOLATION
+	if (migratetype == MIGRATE_ISOLATE) {
+		VM_WARN_ONCE(
+			1,
+			"Set isolate=true to isolate pageblock with a migratetype");
+		return;
+	}
+	if (isolate) {
+		mask = MIGRATETYPE_AND_ISO_MASK;
+		flags |= BIT(PB_migrate_isolate);
+	}
+#endif
+	__set_pfnblock_flags_mask(page, page_to_pfn(page), flags, mask);
+}
+
 #ifdef CONFIG_DEBUG_VM
 static int page_outside_zone_boundaries(struct zone *zone, struct page *page)
 {
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
                   ` (2 preceding siblings ...)
  2025-05-23 19:12 ` [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-27 10:50   ` Vlastimil Babka
  2025-05-23 19:12 ` [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
  2025-05-23 19:12 ` [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
  5 siblings, 1 reply; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

Since migratetype is no longer overwritten during pageblock isolation,
moving a pageblock out of MIGRATE_ISOLATE no longer needs a new
migratetype.

Add pageblock_isolate_and_move_free_pages() and
pageblock_unisolate_and_move_free_pages() to be explicit about the page
isolation operations. Both share the common code in
__move_freepages_block_isolate(), which is renamed from
move_freepages_block_isolate().

Add toggle_pageblock_isolate() to flip pageblock isolation bit in
__move_freepages_block_isolate().

Make set_pageblock_migratetype() only accept non MIGRATE_ISOLATE types,
so that one should use set_pageblock_isolate() to isolate pageblocks.
As a result, move pageblock migratetype code out of
__move_freepages_block().

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

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 14c6a5f691c2..7241a6719618 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -44,10 +44,9 @@ static inline void set_pageblock_isolate(struct page *page)
 void __meminit init_pageblock_migratetype(struct page *page,
 					  enum migratetype migratetype,
 					  bool isolate);
-void set_pageblock_migratetype(struct page *page, enum migratetype 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 8fcbd7fa13c2..44a08b1a9de4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -524,13 +524,36 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
 	__clear_bit(bitidx + pb_bit, bitmap_word);
 }
 
+#ifdef CONFIG_MEMORY_ISOLATION
+/**
+ * toggle_pfnblock_bit - Toggle a standalone bit of a pageblock
+ * @page: The page within the block of interest
+ * @pfn: The target page frame number
+ * @pb_bit: pageblock bit to toggle
+ */
+static void toggle_pfnblock_bit(const struct page *page, unsigned long pfn,
+		      enum pageblock_bits pb_bit)
+{
+	unsigned long *bitmap_word;
+	unsigned long bitidx;
+
+	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
+			 pb_bit >= __NR_PAGEBLOCK_BITS))
+		return;
+
+	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+	__change_bit(bitidx + pb_bit, bitmap_word);
+}
+#endif
+
 /**
  * set_pageblock_migratetype - Set the migratetype of a pageblock
  * @page: The page within the block of interest
  * @migratetype: migratetype to set
  */
-__always_inline void set_pageblock_migratetype(struct page *page,
-					       enum migratetype migratetype)
+static void set_pageblock_migratetype(struct page *page,
+				      enum migratetype migratetype)
 {
 	unsigned long mask = MIGRATETYPE_MASK;
 
@@ -540,11 +563,15 @@ __always_inline void set_pageblock_migratetype(struct page *page,
 
 #ifdef CONFIG_MEMORY_ISOLATION
 	if (migratetype == MIGRATE_ISOLATE) {
-		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
+		VM_WARN_ONCE(1,
+			"Use set_pageblock_isolate() for pageblock isolation");
 		return;
 	}
 	/* change mask to clear PB_migrate_isolate if it is set */
 	mask = MIGRATETYPE_AND_ISO_MASK;
+	VM_WARN_ONCE(get_pfnblock_bit(page, page_to_pfn(page),
+				      PB_migrate_isolate),
+		     "Use clear_pageblock_isolate() to unisolate pageblock");
 #endif
 	__set_pfnblock_flags_mask(page, page_to_pfn(page),
 				  (unsigned long)migratetype, mask);
@@ -1931,8 +1958,8 @@ static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
 #endif
 
 /*
- * Change the type of a block and move all its free pages to that
- * type's freelist.
+ * Move all free pages of a block to new type's freelist. Caller needs to
+ * change the block type.
  */
 static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
 				  int old_mt, int new_mt)
@@ -1964,8 +1991,6 @@ static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
 		pages_moved += 1 << order;
 	}
 
-	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
-
 	return pages_moved;
 }
 
@@ -2023,11 +2048,16 @@ static int move_freepages_block(struct zone *zone, struct page *page,
 				int old_mt, int new_mt)
 {
 	unsigned long start_pfn;
+	int res;
 
 	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
 		return -1;
 
-	return __move_freepages_block(zone, start_pfn, old_mt, new_mt);
+	res = __move_freepages_block(zone, start_pfn, old_mt, new_mt);
+	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
+
+	return res;
+
 }
 
 #ifdef CONFIG_MEMORY_ISOLATION
@@ -2055,11 +2085,16 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
 	return start_pfn;
 }
 
+static inline void toggle_pageblock_isolate(struct page *page)
+{
+	toggle_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
+}
+
 /**
- * move_freepages_block_isolate - move free pages in block for page isolation
+ * __move_freepages_block_isolate - move free pages in block for page isolation
  * @zone: the zone
  * @page: the pageblock page
- * @migratetype: migratetype to set on the pageblock
+ * @isolate: 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
@@ -2074,10 +2109,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_block_isolate(struct zone *zone,
+		struct page *page, bool isolate)
 {
 	unsigned long start_pfn, pfn;
+	int from_mt;
+	int to_mt;
+
+	if (isolate == get_pageblock_isolate(page))
+		return false;
 
 	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
 		return false;
@@ -2094,7 +2134,7 @@ 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);
+		toggle_pageblock_isolate(page);
 		split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
 		return true;
 	}
@@ -2105,16 +2145,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);
+		toggle_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);
+	/* Use MIGRATETYPE_MASK to get non-isolate migratetype */
+	if (isolate) {
+		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);
+	toggle_pageblock_isolate(pfn_to_page(start_pfn));
+
 	return true;
 }
+
+bool pageblock_isolate_and_move_free_pages(struct zone *zone, struct page *page)
+{
+	return __move_freepages_block_isolate(zone, page, true);
+}
+
+bool pageblock_unisolate_and_move_free_pages(struct zone *zone, struct page *page)
+{
+	return __move_freepages_block_isolate(zone, page, false);
+}
+
 #endif /* CONFIG_MEMORY_ISOLATION */
 
 static void change_pageblock_range(struct page *pageblock_page,
@@ -2306,6 +2368,7 @@ try_to_claim_block(struct zone *zone, struct page *page,
 	if (free_pages + alike_pages >= (1 << (pageblock_order-1)) ||
 			page_group_by_mobility_disabled) {
 		__move_freepages_block(zone, start_pfn, block_type, start_type);
+		set_pageblock_migratetype(pfn_to_page(start_pfn), start_type);
 		return __rmqueue_smallest(zone, order, start_type);
 	}
 
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index b2fc5266e3d2..08f627a5032f 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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range()
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
                   ` (3 preceding siblings ...)
  2025-05-23 19:12 ` [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-27 10:56   ` Vlastimil Babka
  2025-05-23 19:12 ` [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
  5 siblings, 1 reply; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, 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 7241a6719618..7a681a49e73c 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -51,8 +51,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 43ac34ee8d2e..ab66acd3e6b3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1233,7 +1233,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
@@ -2119,7 +2119,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 44a08b1a9de4..499c3f40851f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7027,7 +7027,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 08f627a5032f..1edfef408faf 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



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions.
  2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
                   ` (4 preceding siblings ...)
  2025-05-23 19:12 ` [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
@ 2025-05-23 19:12 ` Zi Yan
  2025-05-26  1:33   ` Zi Yan
  2025-05-27 12:55   ` Vlastimil Babka
  5 siblings, 2 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-23 19:12 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, 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 provide the information. At the
same time change isolation flags to enum pb_isolate_mode (MEMORY_OFFLINE,
CMA_ALLOCATION, and ISOLATE_MODE_OTHERS). Remove REPORT_FAILURE and check
MEMORY_OFFLINE instead, since only MEMORY_OFFLINE reports isolation
failures.

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 | 19 ++++++++--
 include/trace/events/kmem.h    | 14 ++++---
 mm/cma.c                       |  2 +-
 mm/memory_hotplug.c            |  4 +-
 mm/page_alloc.c                | 25 ++++++-------
 mm/page_isolation.c            | 67 +++++++++++++++-------------------
 8 files changed, 72 insertions(+), 68 deletions(-)

diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 56d0dbe62163..8accc0f255a8 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 be160e8d8bcb..dea27ed24f8e 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -423,9 +423,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 7a681a49e73c..b61bca909ddc 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -38,8 +38,19 @@ static inline void set_pageblock_isolate(struct page *page)
 }
 #endif
 
-#define MEMORY_OFFLINE	0x1
-#define REPORT_FAILURE	0x2
+/*
+ * Pageblock isolation modes:
+ * MEMORY_OFFLINE      - isolate to offline (!allocate) memory e.g., skip over
+ *		         PageHWPoison() pages and PageOffline() pages.
+ *		         Unmovable pages will be reported in this mode.
+ * CMA_ALLOCATION      - isolate for CMA allocations
+ * ISOLATE_MODE_OTHERS - isolate for other purposes
+ */
+enum pb_isolate_mode {
+	MEMORY_OFFLINE,
+	CMA_ALLOCATION,
+	ISOLATE_MODE_OTHERS,
+};
 
 void __meminit init_pageblock_migratetype(struct page *page,
 					  enum migratetype migratetype,
@@ -49,10 +60,10 @@ 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);
+			     enum pb_isolate_mode mode);
 
 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);
+			enum pb_isolate_mode mode);
 #endif
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index f74925a6cf69..efffcf578217 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
 
 TRACE_EVENT(mm_setup_per_zone_wmarks,
 
diff --git a/mm/cma.c b/mm/cma.c
index 397567883a10..9ee8fad797bc 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -822,7 +822,7 @@ static int cma_range_alloc(struct cma *cma, struct cma_memrange *cmr,
 
 		pfn = cmr->base_pfn + (bitmap_no << cma->order_per_bit);
 		mutex_lock(&cma->alloc_mutex);
-		ret = alloc_contig_range(pfn, pfn + count, MIGRATE_CMA, gfp);
+		ret = alloc_contig_range(pfn, pfn + count, ACR_CMA, gfp);
 		mutex_unlock(&cma->alloc_mutex);
 		if (ret == 0) {
 			page = pfn_to_page(pfn);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index ab66acd3e6b3..44f140dee0ce 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2008,9 +2008,7 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
 	lru_cache_disable();
 
 	/* set above range as isolated */
-	ret = start_isolate_page_range(start_pfn, end_pfn,
-				       MIGRATE_MOVABLE,
-				       MEMORY_OFFLINE | REPORT_FAILURE);
+	ret = start_isolate_page_range(start_pfn, end_pfn, MEMORY_OFFLINE);
 	if (ret) {
 		reason = "failure to isolate range";
 		goto failed_removal_pcplists_disabled;
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 499c3f40851f..48f458f7143a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6720,11 +6720,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)
+					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;
@@ -6796,7 +6797,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);
@@ -6867,10 +6868,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.
@@ -6887,7 +6885,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;
@@ -6903,6 +6901,8 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	};
 	INIT_LIST_HEAD(&cc.migratepages);
 	bool is_range_aligned;
+	isolate_mode_t mode = (alloc_flags & ACR_CMA) ? CMA_ALLOCATION :
+							ISOLATE_MODE_OTHERS;
 
 	gfp_mask = current_gfp_context(gfp_mask);
 	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
@@ -6929,7 +6929,7 @@ 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, mode);
 	if (ret)
 		goto done;
 
@@ -6945,7 +6945,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;
 
@@ -6979,7 +6979,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	outer_start = find_large_buddy(start);
 
 	/* Make sure the range is really isolated. */
-	if (test_pages_isolated(outer_start, end, 0)) {
+	if (test_pages_isolated(outer_start, end, mode)) {
 		ret = -EBUSY;
 		goto done;
 	}
@@ -7037,8 +7037,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 1edfef408faf..bcf6525ea238 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)
+				enum pb_isolate_mode mode)
 {
 	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 (mode == CMA_ALLOCATION)
 			return NULL;
 
 		return page;
@@ -117,7 +117,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
 		 * The HWPoisoned page may be not in buddy system, and
 		 * page_count() is not 0.
 		 */
-		if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
+		if ((mode == MEMORY_OFFLINE) && PageHWPoison(page))
 			continue;
 
 		/*
@@ -130,7 +130,7 @@ static struct page *has_unmovable_pages(unsigned long start_pfn, unsigned long e
 		 * move these pages that still have a reference count > 0.
 		 * (false negatives in this function only)
 		 */
-		if ((flags & MEMORY_OFFLINE) && PageOffline(page))
+		if ((mode == MEMORY_OFFLINE) && PageOffline(page))
 			continue;
 
 		if (__PageMovable(page) || PageLRU(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, enum pb_isolate_mode mode,
 			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);
+			mode);
 	if (!unmovable) {
 		if (!pageblock_isolate_and_move_free_pages(zone, page)) {
 			spin_unlock_irqrestore(&zone->lock, flags);
@@ -198,7 +198,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	}
 
 	spin_unlock_irqrestore(&zone->lock, flags);
-	if (isol_flags & REPORT_FAILURE) {
+	if (mode == MEMORY_OFFLINE) {
 		/*
 		 * printk() with zone->lock held will likely trigger a
 		 * lockdep splat, so defer it here.
@@ -292,11 +292,10 @@ __first_valid_page(unsigned long pfn, unsigned long nr_pages)
  * isolate_single_pageblock() -- tries to isolate a pageblock that might be
  * within a free or in-use page.
  * @boundary_pfn:		pageblock-aligned pfn that a page might cross
- * @flags:			isolation flags
+ * @mode:			isolation mode
  * @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,9 @@ __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,
+			enum pb_isolate_mode mode, bool isolate_before,
+			bool skip_isolation)
 {
 	unsigned long start_pfn;
 	unsigned long isolate_pageblock;
@@ -338,12 +338,11 @@ 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,
-				flags, isolate_pageblock, isolate_pageblock + pageblock_nr_pages);
+		ret = set_migratetype_isolate(pfn_to_page(isolate_pageblock),
+				mode, isolate_pageblock,
+				isolate_pageblock + pageblock_nr_pages);
 
 		if (ret)
 			return ret;
@@ -441,14 +440,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
+ * @mode:		isolation mode
  *
  * 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 +473,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)
+			     enum pb_isolate_mode mode)
 {
 	unsigned long pfn;
 	struct page *page;
@@ -492,8 +484,8 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 	bool skip_isolation = false;
 
 	/* isolate [isolate_start, isolate_start + pageblock_nr_pages) pageblock */
-	ret = isolate_single_pageblock(isolate_start, flags, false,
-			skip_isolation, migratetype);
+	ret = isolate_single_pageblock(isolate_start, mode, false,
+			skip_isolation);
 	if (ret)
 		return ret;
 
@@ -501,8 +493,7 @@ int start_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn,
 		skip_isolation = true;
 
 	/* isolate [isolate_end - pageblock_nr_pages, isolate_end) pageblock */
-	ret = isolate_single_pageblock(isolate_end, flags, true,
-			skip_isolation, migratetype);
+	ret = isolate_single_pageblock(isolate_end, mode, true, skip_isolation);
 	if (ret) {
 		unset_migratetype_isolate(pfn_to_page(isolate_start));
 		return ret;
@@ -513,8 +504,8 @@ 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,
-					start_pfn, end_pfn)) {
+		if (page && set_migratetype_isolate(page, mode, start_pfn,
+					end_pfn)) {
 			undo_isolate_page_range(isolate_start, pfn);
 			unset_migratetype_isolate(
 				pfn_to_page(isolate_end - pageblock_nr_pages));
@@ -556,7 +547,7 @@ void undo_isolate_page_range(unsigned long start_pfn, unsigned long end_pfn)
  */
 static unsigned long
 __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
-				  int flags)
+				  enum pb_isolate_mode mode)
 {
 	struct page *page;
 
@@ -569,10 +560,10 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
 			 * simple way to verify that as VM_BUG_ON(), though.
 			 */
 			pfn += 1 << buddy_order(page);
-		else if ((flags & MEMORY_OFFLINE) && PageHWPoison(page))
+		else if ((mode == MEMORY_OFFLINE) && PageHWPoison(page))
 			/* A HWPoisoned page cannot be also PageBuddy */
 			pfn++;
-		else if ((flags & MEMORY_OFFLINE) && PageOffline(page) &&
+		else if ((mode == MEMORY_OFFLINE) && PageOffline(page) &&
 			 !page_count(page))
 			/*
 			 * The responsible driver agreed to skip PageOffline()
@@ -591,11 +582,11 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
  * test_pages_isolated - check if pageblocks in range are isolated
  * @start_pfn:		The first PFN of the isolated range
  * @end_pfn:		The first PFN *after* the isolated range
- * @isol_flags:		Testing mode flags
+ * @mode:		Testing mode
  *
  * This tests if all in the specified range are free.
  *
- * If %MEMORY_OFFLINE is specified in @flags, it will consider
+ * If %MEMORY_OFFLINE is specified in @mode, it will consider
  * poisoned and offlined pages free as well.
  *
  * Caller must ensure the requested range doesn't span zones.
@@ -603,7 +594,7 @@ __test_page_isolated_in_pageblock(unsigned long pfn, unsigned long end_pfn,
  * Returns 0 if true, -EBUSY if one or more pages are in use.
  */
 int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
-			int isol_flags)
+			enum pb_isolate_mode mode)
 {
 	unsigned long pfn, flags;
 	struct page *page;
@@ -639,7 +630,7 @@ int test_pages_isolated(unsigned long start_pfn, unsigned long end_pfn,
 	/* Check all pages are free or marked as ISOLATED */
 	zone = page_zone(page);
 	spin_lock_irqsave(&zone->lock, flags);
-	pfn = __test_page_isolated_in_pageblock(start_pfn, end_pfn, isol_flags);
+	pfn = __test_page_isolated_in_pageblock(start_pfn, end_pfn, mode);
 	spin_unlock_irqrestore(&zone->lock, flags);
 
 	ret = pfn < end_pfn ? -EBUSY : 0;
-- 
2.47.2



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions.
  2025-05-23 19:12 ` [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
@ 2025-05-26  1:33   ` Zi Yan
  2025-05-27 12:55   ` Vlastimil Babka
  1 sibling, 0 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-26  1:33 UTC (permalink / raw)
  To: David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Vlastimil Babka, Baolin Wang,
	Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Richard Chang, linux-kernel, Zi Yan

On 23 May 2025, at 15:12, Zi Yan wrote:

> 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 provide the information. At the
> same time change isolation flags to enum pb_isolate_mode (MEMORY_OFFLINE,
> CMA_ALLOCATION, and ISOLATE_MODE_OTHERS). Remove REPORT_FAILURE and check
> MEMORY_OFFLINE instead, since only MEMORY_OFFLINE reports isolation
> failures.
>
> 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 | 19 ++++++++--
>  include/trace/events/kmem.h    | 14 ++++---
>  mm/cma.c                       |  2 +-
>  mm/memory_hotplug.c            |  4 +-
>  mm/page_alloc.c                | 25 ++++++-------
>  mm/page_isolation.c            | 67 +++++++++++++++-------------------
>  8 files changed, 72 insertions(+), 68 deletions(-)

This fixes mode's wrong type.

From 0c9792cd7ad1d1cd16b276b70d972e5b871ff653 Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Sun, 25 May 2025 21:29:06 -0400
Subject: [PATCH] fix mode's type.

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/page_alloc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 48f458f7143a..3096304d1b58 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6901,8 +6901,8 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
 	};
 	INIT_LIST_HEAD(&cc.migratepages);
 	bool is_range_aligned;
-	isolate_mode_t mode = (alloc_flags & ACR_CMA) ? CMA_ALLOCATION :
-							ISOLATE_MODE_OTHERS;
+	enum pb_isolate_mode mode =
+		(alloc_flags & ACR_CMA) ? CMA_ALLOCATION : ISOLATE_MODE_OTHERS;

 	gfp_mask = current_gfp_context(gfp_mask);
 	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
-- 
2.47.2



--
Best Regards,
Yan, Zi


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up.
  2025-05-23 19:12 ` [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
@ 2025-05-27  9:46   ` Vlastimil Babka
  2025-05-27 14:47     ` Zi Yan
  0 siblings, 1 reply; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27  9:46 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> No functional change is intended.
> 
> 1. Add __NR_PAGEBLOCK_BITS for the number of pageblock flag bits and use
>    roundup_pow_of_two(__NR_PAGEBLOCK_BITS) as NR_PAGEBLOCK_BITS to take
>    right amount of bits for pageblock flags.
> 2. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
>    like PB_migrate_skip.
> 3. Make {get,set}_pfnblock_flags_mask() internal functions and use
>    {get,set}_pfnblock_migratetype() for pageblock migratetype operations.
> 4. Move pageblock flags common code to get_pfnblock_bitmap_bitidx().
> 3. Use MIGRATETYPE_MASK to get the migratetype of a pageblock from its
>    flags.
> 4. Use PB_migrate_end in the definition of MIGRATETYPE_MASK instead of
>    PB_migrate_bits.
> 5. Add a comment on is_migrate_cma_folio() to prevent one from changing it
>    to use get_pageblock_migratetype() and causing issues.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>

<snip>

> +/**
> + * __set_pfnblock_flags_mask - Set the requested group of flags for
> + * a pageblock_nr_pages block of pages
>   * @page: The page within the block of interest
> - * @flags: The flags to set
>   * @pfn: The target page frame number
> + * @flags: The flags to set
>   * @mask: mask of bits that the caller is interested in
>   */
> -void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
> -					unsigned long pfn,
> -					unsigned long mask)
> +static void __set_pfnblock_flags_mask(struct page *page, unsigned long pfn,
> +				      unsigned long flags, unsigned long mask)
>  {
> -	unsigned long *bitmap;
> -	unsigned long bitidx, word_bitidx;
> +	unsigned long *bitmap_word;
> +	unsigned long bitidx;
>  	unsigned long word;
>  
> -	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
> -	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
> -
> -	bitmap = get_pageblock_bitmap(page, pfn);
> -	bitidx = pfn_to_bitidx(page, pfn);
> -	word_bitidx = bitidx / BITS_PER_LONG;
> -	bitidx &= (BITS_PER_LONG-1);
> -
> -	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
>  
>  	mask <<= bitidx;
>  	flags <<= bitidx;
>  
> -	word = READ_ONCE(bitmap[word_bitidx]);
> +	word = READ_ONCE(*bitmap_word);
>  	do {
> -	} while (!try_cmpxchg(&bitmap[word_bitidx], &word, (word & ~mask) | flags));
> +	} while (!try_cmpxchg(bitmap_word, &word, (word & ~mask) | flags));
> +}
> +
> +/**
> + * set_pfnblock_bit - Set a standalone bit of a pageblock
> + * @page: The page within the block of interest
> + * @pfn: The target page frame number
> + * @pb_bit: pageblock bit to set
> + */
> +void set_pfnblock_bit(const struct page *page, unsigned long pfn,
> +		      enum pageblock_bits pb_bit)
> +{
> +	unsigned long *bitmap_word;
> +	unsigned long bitidx;
> +
> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
> +		return;

This check appears at 3 places, maybe worth wrapping it in a helper?

> +
> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
> +
> +	__set_bit(bitidx + pb_bit, bitmap_word);

I think it's wrong to use the __set_bit non-atomic variant because e.g.
compaction's PB_migrate_skip (actually a misnomer at this point I think,
e.g. PB_compact_skip would make more sense if you wanted to clean up things
some more) can be modified with no lock. It's why
__set_pfnblock_flags_mask() above uses try_cmpxchg() even though changes to
migratetype are normally done under zone lock.

> +}
> +
> +/**
> + * clear_pfnblock_bit - Clear a standalone bit of a pageblock
> + * @page: The page within the block of interest
> + * @pfn: The target page frame number
> + * @pb_bit: pageblock bit to clear
> + */
> +void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
> +			enum pageblock_bits pb_bit)
> +{
> +	unsigned long *bitmap_word;
> +	unsigned long bitidx;
> +
> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
> +		return;
> +
> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
> +
> +	__clear_bit(bitidx + pb_bit, bitmap_word);

Same here.

>  }


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit.
  2025-05-23 19:12 ` [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
@ 2025-05-27 10:11   ` Vlastimil Babka
  2025-05-27 14:56     ` Zi Yan
  0 siblings, 1 reply; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27 10:11 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> During page isolation, the original migratetype is overwritten, since
> MIGRATE_* are enums and stored in pageblock bitmaps. Change
> MIGRATE_ISOLATE to be stored a standalone bit, PB_migrate_isolate, like
> PB_migrate_skip, so that migratetype is not lost during pageblock
> isolation.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>

<snip>

>  #define MEMORY_OFFLINE	0x1
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index 3acbb271a29a..f2f8540b95ca 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -20,7 +20,13 @@ 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
> +	/*
> +	 * Pageblock isolation is represented with a separate bit, so that
> +	 * the migratetype of a block is not overwritten by isolation.
> +	 */
> +	PB_migrate_isolate, /* If set the block is isolated */
> +#endif
>  	/*
>  	 * Assume the bits will always align on a word. If this assumption
>  	 * changes then get/set pageblock needs updating.
> @@ -32,6 +38,11 @@ enum pageblock_bits {
>  
>  #define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +#define MIGRATETYPE_AND_ISO_MASK \
> +	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
> +#endif

I think if there was:

#else
#define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
#endif

you could avoid some #ifdef code later.

>  #if defined(CONFIG_HUGETLB_PAGE)
>  
>  #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 0207164fcaf6..b2c623699461 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -360,8 +360,14 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>  	unsigned long *bitmap;
>  	unsigned long word_bitidx;
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
> +	/* extra one for MIGRATE_ISOLATE */
> +	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1);

This implicitly assumes MIGRATE_ISOLATE is the last of migratetypes so we
can actually need less PB_migratetype_bits if we stop encoding it within
them anymore, but there's nothing enforcing that (not even as a comment)?

> +#else
>  	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>  	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
> +#endif
>  	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
>  
>  	bitmap = get_pageblock_bitmap(page, pfn);
> @@ -435,7 +441,20 @@ bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
>  __always_inline enum migratetype
>  get_pfnblock_migratetype(const struct page *page, unsigned long pfn)
>  {
> -	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
> +	unsigned long mask = MIGRATETYPE_MASK;

E.g. with my suggestion above you could use MIGRATETYPE_AND_ISO_MASK here.

> +	unsigned long flags;
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	mask = MIGRATETYPE_AND_ISO_MASK;
> +#endif

And drop this.

> +	flags = __get_pfnblock_flags_mask(page, pfn, mask);
> +
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (flags & BIT(PB_migrate_isolate))
> +		return MIGRATE_ISOLATE;
> +#endif
> +	return flags & MIGRATETYPE_MASK;
>  }
>  
>  /**
> @@ -513,12 +532,22 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>  __always_inline void set_pageblock_migratetype(struct page *page,
>  					       enum migratetype migratetype)
>  {
> +	unsigned long mask = MIGRATETYPE_MASK;
> +
>  	if (unlikely(page_group_by_mobility_disabled &&
>  		     migratetype < MIGRATE_PCPTYPES))
>  		migratetype = MIGRATE_UNMOVABLE;
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +	if (migratetype == MIGRATE_ISOLATE) {
> +		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
> +		return;
> +	}
> +	/* change mask to clear PB_migrate_isolate if it is set */
> +	mask = MIGRATETYPE_AND_ISO_MASK;
> +#endif
>  	__set_pfnblock_flags_mask(page, page_to_pfn(page),
> -				  (unsigned long)migratetype, MIGRATETYPE_MASK);
> +				  (unsigned long)migratetype, mask);

This could just pass MIGRATETYPE_AND_ISO_MASK here.

>  }
>  
>  #ifdef CONFIG_DEBUG_VM



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
  2025-05-23 19:12 ` [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
@ 2025-05-27 10:31   ` Vlastimil Babka
  0 siblings, 0 replies; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27 10:31 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> MIGRATE_ISOLATE is a standalone bit, so a pageblock cannot be initialized
> to just MIGRATE_ISOLATE. Add init_pageblock_migratetype() to enable
> initialize a pageblock with a migratetype and isolated.
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
  2025-05-23 19:12 ` [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
@ 2025-05-27 10:50   ` Vlastimil Babka
  2025-05-27 15:02     ` Zi Yan
  0 siblings, 1 reply; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27 10:50 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> Since migratetype is no longer overwritten during pageblock isolation,
> moving a pageblock out of MIGRATE_ISOLATE no longer needs a new
> migratetype.
> 
> Add pageblock_isolate_and_move_free_pages() and
> pageblock_unisolate_and_move_free_pages() to be explicit about the page
> isolation operations. Both share the common code in
> __move_freepages_block_isolate(), which is renamed from
> move_freepages_block_isolate().
> 
> Add toggle_pageblock_isolate() to flip pageblock isolation bit in
> __move_freepages_block_isolate().
> 
> Make set_pageblock_migratetype() only accept non MIGRATE_ISOLATE types,
> so that one should use set_pageblock_isolate() to isolate pageblocks.
> As a result, move pageblock migratetype code out of
> __move_freepages_block().
> 
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
>  include/linux/page-isolation.h |  5 +-
>  mm/page_alloc.c                | 97 ++++++++++++++++++++++++++++------
>  mm/page_isolation.c            | 21 ++++----
>  3 files changed, 92 insertions(+), 31 deletions(-)
> 
> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
> index 14c6a5f691c2..7241a6719618 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -44,10 +44,9 @@ static inline void set_pageblock_isolate(struct page *page)
>  void __meminit init_pageblock_migratetype(struct page *page,
>  					  enum migratetype migratetype,
>  					  bool isolate);
> -void set_pageblock_migratetype(struct page *page, enum migratetype 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 8fcbd7fa13c2..44a08b1a9de4 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -524,13 +524,36 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>  	__clear_bit(bitidx + pb_bit, bitmap_word);
>  }
>  
> +#ifdef CONFIG_MEMORY_ISOLATION
> +/**
> + * toggle_pfnblock_bit - Toggle a standalone bit of a pageblock
> + * @page: The page within the block of interest
> + * @pfn: The target page frame number
> + * @pb_bit: pageblock bit to toggle
> + */
> +static void toggle_pfnblock_bit(const struct page *page, unsigned long pfn,
> +		      enum pageblock_bits pb_bit)
> +{
> +	unsigned long *bitmap_word;
> +	unsigned long bitidx;
> +
> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
> +		return;
> +
> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
> +
> +	__change_bit(bitidx + pb_bit, bitmap_word);

Again the non-atomic variant, but actually below I suggest we drop this.

> +}
> +#endif
> +
>  /**
>   * set_pageblock_migratetype - Set the migratetype of a pageblock
>   * @page: The page within the block of interest
>   * @migratetype: migratetype to set
>   */
> -__always_inline void set_pageblock_migratetype(struct page *page,
> -					       enum migratetype migratetype)
> +static void set_pageblock_migratetype(struct page *page,
> +				      enum migratetype migratetype)
>  {
>  	unsigned long mask = MIGRATETYPE_MASK;
>  
> @@ -540,11 +563,15 @@ __always_inline void set_pageblock_migratetype(struct page *page,
>  
>  #ifdef CONFIG_MEMORY_ISOLATION
>  	if (migratetype == MIGRATE_ISOLATE) {
> -		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
> +		VM_WARN_ONCE(1,
> +			"Use set_pageblock_isolate() for pageblock isolation");
>  		return;
>  	}
>  	/* change mask to clear PB_migrate_isolate if it is set */
>  	mask = MIGRATETYPE_AND_ISO_MASK;
> +	VM_WARN_ONCE(get_pfnblock_bit(page, page_to_pfn(page),
> +				      PB_migrate_isolate),
> +		     "Use clear_pageblock_isolate() to unisolate pageblock");
>  #endif

We might be too paranoid with the warnings given these are all local
functions to this file so risk of misuse should be low. Maybe we could
remove later...

>  	__set_pfnblock_flags_mask(page, page_to_pfn(page),
>  				  (unsigned long)migratetype, mask);
> @@ -1931,8 +1958,8 @@ static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
>  #endif
>  
>  /*
> - * Change the type of a block and move all its free pages to that
> - * type's freelist.
> + * Move all free pages of a block to new type's freelist. Caller needs to
> + * change the block type.
>   */
>  static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
>  				  int old_mt, int new_mt)
> @@ -1964,8 +1991,6 @@ static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
>  		pages_moved += 1 << order;
>  	}
>  
> -	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
> -
>  	return pages_moved;
>  }
>  
> @@ -2023,11 +2048,16 @@ static int move_freepages_block(struct zone *zone, struct page *page,
>  				int old_mt, int new_mt)
>  {
>  	unsigned long start_pfn;
> +	int res;
>  
>  	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
>  		return -1;
>  
> -	return __move_freepages_block(zone, start_pfn, old_mt, new_mt);
> +	res = __move_freepages_block(zone, start_pfn, old_mt, new_mt);
> +	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
> +
> +	return res;
> +
>  }
>  
>  #ifdef CONFIG_MEMORY_ISOLATION
> @@ -2055,11 +2085,16 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>  	return start_pfn;
>  }
>  
> +static inline void toggle_pageblock_isolate(struct page *page)
> +{
> +	toggle_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
> +}

I'm wary about the togle action, as we should always know what action we
want to do anyway. So we could just add a "bool isolate" parameter and call
set or clear explicitly? Allows for some hypothetical DEBUG_VM checks too
(pageblock is not already in the state we want it to be).




^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range()
  2025-05-23 19:12 ` [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
@ 2025-05-27 10:56   ` Vlastimil Babka
  0 siblings, 0 replies; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27 10:56 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> 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>

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions.
  2025-05-23 19:12 ` [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
  2025-05-26  1:33   ` Zi Yan
@ 2025-05-27 12:55   ` Vlastimil Babka
  2025-05-27 15:04     ` Zi Yan
  1 sibling, 1 reply; 18+ messages in thread
From: Vlastimil Babka @ 2025-05-27 12:55 UTC (permalink / raw)
  To: Zi Yan, David Hildenbrand, Johannes Weiner, linux-mm
  Cc: Andrew Morton, Oscar Salvador, Baolin Wang, Kirill A . Shutemov,
	Mel Gorman, Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Richard Chang, linux-kernel

On 5/23/25 21:12, Zi Yan wrote:
> 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 provide the information. At the
> same time change isolation flags to enum pb_isolate_mode (MEMORY_OFFLINE,
> CMA_ALLOCATION, and ISOLATE_MODE_OTHERS). Remove REPORT_FAILURE and check
> MEMORY_OFFLINE instead, since only MEMORY_OFFLINE reports isolation
> failures.
> 
> 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 | 19 ++++++++--
>  include/trace/events/kmem.h    | 14 ++++---
>  mm/cma.c                       |  2 +-
>  mm/memory_hotplug.c            |  4 +-
>  mm/page_alloc.c                | 25 ++++++-------
>  mm/page_isolation.c            | 67 +++++++++++++++-------------------
>  8 files changed, 72 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 56d0dbe62163..8accc0f255a8 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 be160e8d8bcb..dea27ed24f8e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -423,9 +423,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

Would it make sense to define ACR_NONE as 0 so it's more descriptive than
seeing 0 somewhere?

> +
>  /* 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 7a681a49e73c..b61bca909ddc 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -38,8 +38,19 @@ static inline void set_pageblock_isolate(struct page *page)
>  }
>  #endif
>  
> -#define MEMORY_OFFLINE	0x1
> -#define REPORT_FAILURE	0x2
> +/*
> + * Pageblock isolation modes:
> + * MEMORY_OFFLINE      - isolate to offline (!allocate) memory e.g., skip over
> + *		         PageHWPoison() pages and PageOffline() pages.
> + *		         Unmovable pages will be reported in this mode.
> + * CMA_ALLOCATION      - isolate for CMA allocations
> + * ISOLATE_MODE_OTHERS - isolate for other purposes
> + */
> +enum pb_isolate_mode {
> +	MEMORY_OFFLINE,
> +	CMA_ALLOCATION,
> +	ISOLATE_MODE_OTHERS,

Since this is in a .h file, I'd prefer more consistent naming, and longer
names shouldn't hurt as there are not that many users.

Even something like:
PB_ISOLATE_MODE_OFFLINE
PB_ISOLATE_MODE_CMA
PB_ISOLATE_MODE_OTHER

?



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up.
  2025-05-27  9:46   ` Vlastimil Babka
@ 2025-05-27 14:47     ` Zi Yan
  0 siblings, 0 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-27 14:47 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: David Hildenbrand, Johannes Weiner, linux-mm, Andrew Morton,
	Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
	linux-kernel

On 27 May 2025, at 5:46, Vlastimil Babka wrote:

> On 5/23/25 21:12, Zi Yan wrote:
>> No functional change is intended.
>>
>> 1. Add __NR_PAGEBLOCK_BITS for the number of pageblock flag bits and use
>>    roundup_pow_of_two(__NR_PAGEBLOCK_BITS) as NR_PAGEBLOCK_BITS to take
>>    right amount of bits for pageblock flags.
>> 2. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
>>    like PB_migrate_skip.
>> 3. Make {get,set}_pfnblock_flags_mask() internal functions and use
>>    {get,set}_pfnblock_migratetype() for pageblock migratetype operations.
>> 4. Move pageblock flags common code to get_pfnblock_bitmap_bitidx().
>> 3. Use MIGRATETYPE_MASK to get the migratetype of a pageblock from its
>>    flags.
>> 4. Use PB_migrate_end in the definition of MIGRATETYPE_MASK instead of
>>    PB_migrate_bits.
>> 5. Add a comment on is_migrate_cma_folio() to prevent one from changing it
>>    to use get_pageblock_migratetype() and causing issues.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> <snip>
>
>> +/**
>> + * __set_pfnblock_flags_mask - Set the requested group of flags for
>> + * a pageblock_nr_pages block of pages
>>   * @page: The page within the block of interest
>> - * @flags: The flags to set
>>   * @pfn: The target page frame number
>> + * @flags: The flags to set
>>   * @mask: mask of bits that the caller is interested in
>>   */
>> -void set_pfnblock_flags_mask(struct page *page, unsigned long flags,
>> -					unsigned long pfn,
>> -					unsigned long mask)
>> +static void __set_pfnblock_flags_mask(struct page *page, unsigned long pfn,
>> +				      unsigned long flags, unsigned long mask)
>>  {
>> -	unsigned long *bitmap;
>> -	unsigned long bitidx, word_bitidx;
>> +	unsigned long *bitmap_word;
>> +	unsigned long bitidx;
>>  	unsigned long word;
>>
>> -	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>> -	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
>> -
>> -	bitmap = get_pageblock_bitmap(page, pfn);
>> -	bitidx = pfn_to_bitidx(page, pfn);
>> -	word_bitidx = bitidx / BITS_PER_LONG;
>> -	bitidx &= (BITS_PER_LONG-1);
>> -
>> -	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
>> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
>>
>>  	mask <<= bitidx;
>>  	flags <<= bitidx;
>>
>> -	word = READ_ONCE(bitmap[word_bitidx]);
>> +	word = READ_ONCE(*bitmap_word);
>>  	do {
>> -	} while (!try_cmpxchg(&bitmap[word_bitidx], &word, (word & ~mask) | flags));
>> +	} while (!try_cmpxchg(bitmap_word, &word, (word & ~mask) | flags));
>> +}
>> +
>> +/**
>> + * set_pfnblock_bit - Set a standalone bit of a pageblock
>> + * @page: The page within the block of interest
>> + * @pfn: The target page frame number
>> + * @pb_bit: pageblock bit to set
>> + */
>> +void set_pfnblock_bit(const struct page *page, unsigned long pfn,
>> +		      enum pageblock_bits pb_bit)
>> +{
>> +	unsigned long *bitmap_word;
>> +	unsigned long bitidx;
>> +
>> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
>> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
>> +		return;
>
> This check appears at 3 places, maybe worth wrapping it in a helper?

Sure.

>
>> +
>> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
>> +
>> +	__set_bit(bitidx + pb_bit, bitmap_word);
>
> I think it's wrong to use the __set_bit non-atomic variant because e.g.
> compaction's PB_migrate_skip (actually a misnomer at this point I think,
> e.g. PB_compact_skip would make more sense if you wanted to clean up things
Will rename it.

> some more) can be modified with no lock. It's why
> __set_pfnblock_flags_mask() above uses try_cmpxchg() even though changes to
> migratetype are normally done under zone lock.

Got it. Thank you for the explanation. Will fix all *_pfnblock_bit() functions
and add a comment about why atomic variants are used.

>
>> +}
>> +
>> +/**
>> + * clear_pfnblock_bit - Clear a standalone bit of a pageblock
>> + * @page: The page within the block of interest
>> + * @pfn: The target page frame number
>> + * @pb_bit: pageblock bit to clear
>> + */
>> +void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>> +			enum pageblock_bits pb_bit)
>> +{
>> +	unsigned long *bitmap_word;
>> +	unsigned long bitidx;
>> +
>> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
>> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
>> +		return;
>> +
>> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
>> +
>> +	__clear_bit(bitidx + pb_bit, bitmap_word);
>
> Same here.

Ack.

Best Regards,
Yan, Zi


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit.
  2025-05-27 10:11   ` Vlastimil Babka
@ 2025-05-27 14:56     ` Zi Yan
  0 siblings, 0 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-27 14:56 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: David Hildenbrand, Johannes Weiner, linux-mm, Andrew Morton,
	Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
	linux-kernel

On 27 May 2025, at 6:11, Vlastimil Babka wrote:

> On 5/23/25 21:12, Zi Yan wrote:
>> During page isolation, the original migratetype is overwritten, since
>> MIGRATE_* are enums and stored in pageblock bitmaps. Change
>> MIGRATE_ISOLATE to be stored a standalone bit, PB_migrate_isolate, like
>> PB_migrate_skip, so that migratetype is not lost during pageblock
>> isolation.
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>
> <snip>
>
>>  #define MEMORY_OFFLINE	0x1
>> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
>> index 3acbb271a29a..f2f8540b95ca 100644
>> --- a/include/linux/pageblock-flags.h
>> +++ b/include/linux/pageblock-flags.h
>> @@ -20,7 +20,13 @@ 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
>> +	/*
>> +	 * Pageblock isolation is represented with a separate bit, so that
>> +	 * the migratetype of a block is not overwritten by isolation.
>> +	 */
>> +	PB_migrate_isolate, /* If set the block is isolated */
>> +#endif
>>  	/*
>>  	 * Assume the bits will always align on a word. If this assumption
>>  	 * changes then get/set pageblock needs updating.
>> @@ -32,6 +38,11 @@ enum pageblock_bits {
>>
>>  #define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
>>
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +#define MIGRATETYPE_AND_ISO_MASK \
>> +	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
>> +#endif
>
> I think if there was:
>
> #else
> #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
> #endif
>
> you could avoid some #ifdef code later.

Sure. Will do.

>
>>  #if defined(CONFIG_HUGETLB_PAGE)
>>
>>  #ifdef CONFIG_HUGETLB_PAGE_SIZE_VARIABLE
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 0207164fcaf6..b2c623699461 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -360,8 +360,14 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
>>  	unsigned long *bitmap;
>>  	unsigned long word_bitidx;
>>
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 8);
>> +	/* extra one for MIGRATE_ISOLATE */
>> +	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits) + 1);
>
> This implicitly assumes MIGRATE_ISOLATE is the last of migratetypes so we
> can actually need less PB_migratetype_bits if we stop encoding it within
> them anymore, but there's nothing enforcing that (not even as a comment)?’

You point is valid. How about adding __MIGRATE_TYPES right before
MIGRATE_ISOLATE in enum migratetype and using
BUILD_BUG_ON(__MIGRATE_TYPES > (1 << PB_migratetype_bits));
here outside of #ifdef.

The next step of cleanup, as discussed with David, is to remove
MIGRATE_ISOLATE, so that all MIGRATE_TYPES are stored in
PB_migratetype_bits. And {get,set}_pfnblock_migratetype()
gives

struct pageblock_info {
	enum migratetype migratetype;
	bool isolated;
};

>
>> +#else
>>  	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
>>  	BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
>> +#endif
>>  	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
>>
>>  	bitmap = get_pageblock_bitmap(page, pfn);
>> @@ -435,7 +441,20 @@ bool get_pfnblock_bit(const struct page *page, unsigned long pfn,
>>  __always_inline enum migratetype
>>  get_pfnblock_migratetype(const struct page *page, unsigned long pfn)
>>  {
>> -	return __get_pfnblock_flags_mask(page, pfn, MIGRATETYPE_MASK);
>> +	unsigned long mask = MIGRATETYPE_MASK;
>
> E.g. with my suggestion above you could use MIGRATETYPE_AND_ISO_MASK here.
>
>> +	unsigned long flags;
>> +
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	mask = MIGRATETYPE_AND_ISO_MASK;
>> +#endif
>
> And drop this.

Sure. Will do.

>
>> +	flags = __get_pfnblock_flags_mask(page, pfn, mask);
>> +
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	if (flags & BIT(PB_migrate_isolate))
>> +		return MIGRATE_ISOLATE;
>> +#endif
>> +	return flags & MIGRATETYPE_MASK;
>>  }
>>
>>  /**
>> @@ -513,12 +532,22 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>>  __always_inline void set_pageblock_migratetype(struct page *page,
>>  					       enum migratetype migratetype)
>>  {
>> +	unsigned long mask = MIGRATETYPE_MASK;
>> +
>>  	if (unlikely(page_group_by_mobility_disabled &&
>>  		     migratetype < MIGRATE_PCPTYPES))
>>  		migratetype = MIGRATE_UNMOVABLE;
>>
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +	if (migratetype == MIGRATE_ISOLATE) {
>> +		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
>> +		return;
>> +	}
>> +	/* change mask to clear PB_migrate_isolate if it is set */
>> +	mask = MIGRATETYPE_AND_ISO_MASK;
>> +#endif
>>  	__set_pfnblock_flags_mask(page, page_to_pfn(page),
>> -				  (unsigned long)migratetype, MIGRATETYPE_MASK);
>> +				  (unsigned long)migratetype, mask);
>
> This could just pass MIGRATETYPE_AND_ISO_MASK here.

Yep.

>
>>  }
>>
>>  #ifdef CONFIG_DEBUG_VM


Best Regards,
Yan, Zi


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
  2025-05-27 10:50   ` Vlastimil Babka
@ 2025-05-27 15:02     ` Zi Yan
  0 siblings, 0 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-27 15:02 UTC (permalink / raw)
  To: Vlastimil Babka, Johannes Weiner
  Cc: David Hildenbrand, linux-mm, Andrew Morton, Oscar Salvador,
	Baolin Wang, Kirill A . Shutemov, Mel Gorman, Suren Baghdasaryan,
	Michal Hocko, Brendan Jackman, Richard Chang, linux-kernel

On 27 May 2025, at 6:50, Vlastimil Babka wrote:

> On 5/23/25 21:12, Zi Yan wrote:
>> Since migratetype is no longer overwritten during pageblock isolation,
>> moving a pageblock out of MIGRATE_ISOLATE no longer needs a new
>> migratetype.
>>
>> Add pageblock_isolate_and_move_free_pages() and
>> pageblock_unisolate_and_move_free_pages() to be explicit about the page
>> isolation operations. Both share the common code in
>> __move_freepages_block_isolate(), which is renamed from
>> move_freepages_block_isolate().
>>
>> Add toggle_pageblock_isolate() to flip pageblock isolation bit in
>> __move_freepages_block_isolate().
>>
>> Make set_pageblock_migratetype() only accept non MIGRATE_ISOLATE types,
>> so that one should use set_pageblock_isolate() to isolate pageblocks.
>> As a result, move pageblock migratetype code out of
>> __move_freepages_block().
>>
>> Signed-off-by: Zi Yan <ziy@nvidia.com>
>> ---
>>  include/linux/page-isolation.h |  5 +-
>>  mm/page_alloc.c                | 97 ++++++++++++++++++++++++++++------
>>  mm/page_isolation.c            | 21 ++++----
>>  3 files changed, 92 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
>> index 14c6a5f691c2..7241a6719618 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -44,10 +44,9 @@ static inline void set_pageblock_isolate(struct page *page)
>>  void __meminit init_pageblock_migratetype(struct page *page,
>>  					  enum migratetype migratetype,
>>  					  bool isolate);
>> -void set_pageblock_migratetype(struct page *page, enum migratetype 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 8fcbd7fa13c2..44a08b1a9de4 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -524,13 +524,36 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
>>  	__clear_bit(bitidx + pb_bit, bitmap_word);
>>  }
>>
>> +#ifdef CONFIG_MEMORY_ISOLATION
>> +/**
>> + * toggle_pfnblock_bit - Toggle a standalone bit of a pageblock
>> + * @page: The page within the block of interest
>> + * @pfn: The target page frame number
>> + * @pb_bit: pageblock bit to toggle
>> + */
>> +static void toggle_pfnblock_bit(const struct page *page, unsigned long pfn,
>> +		      enum pageblock_bits pb_bit)
>> +{
>> +	unsigned long *bitmap_word;
>> +	unsigned long bitidx;
>> +
>> +	if (WARN_ON_ONCE(pb_bit <= PB_migrate_end ||
>> +			 pb_bit >= __NR_PAGEBLOCK_BITS))
>> +		return;
>> +
>> +	get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
>> +
>> +	__change_bit(bitidx + pb_bit, bitmap_word);
>
> Again the non-atomic variant, but actually below I suggest we drop this.

Yep.

>
>> +}
>> +#endif
>> +
>>  /**
>>   * set_pageblock_migratetype - Set the migratetype of a pageblock
>>   * @page: The page within the block of interest
>>   * @migratetype: migratetype to set
>>   */
>> -__always_inline void set_pageblock_migratetype(struct page *page,
>> -					       enum migratetype migratetype)
>> +static void set_pageblock_migratetype(struct page *page,
>> +				      enum migratetype migratetype)
>>  {
>>  	unsigned long mask = MIGRATETYPE_MASK;
>>
>> @@ -540,11 +563,15 @@ __always_inline void set_pageblock_migratetype(struct page *page,
>>
>>  #ifdef CONFIG_MEMORY_ISOLATION
>>  	if (migratetype == MIGRATE_ISOLATE) {
>> -		set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
>> +		VM_WARN_ONCE(1,
>> +			"Use set_pageblock_isolate() for pageblock isolation");
>>  		return;
>>  	}
>>  	/* change mask to clear PB_migrate_isolate if it is set */
>>  	mask = MIGRATETYPE_AND_ISO_MASK;
>> +	VM_WARN_ONCE(get_pfnblock_bit(page, page_to_pfn(page),
>> +				      PB_migrate_isolate),
>> +		     "Use clear_pageblock_isolate() to unisolate pageblock");
>>  #endif
>
> We might be too paranoid with the warnings given these are all local
> functions to this file so risk of misuse should be low. Maybe we could
> remove later...

Yeah. In the next step, when struct pageblock_info is used to change
a pageblock migratetype and isolation state, these warnings should
go away, since caller will need to be explicit about isolation operations.

>
>>  	__set_pfnblock_flags_mask(page, page_to_pfn(page),
>>  				  (unsigned long)migratetype, mask);
>> @@ -1931,8 +1958,8 @@ static inline struct page *__rmqueue_cma_fallback(struct zone *zone,
>>  #endif
>>
>>  /*
>> - * Change the type of a block and move all its free pages to that
>> - * type's freelist.
>> + * Move all free pages of a block to new type's freelist. Caller needs to
>> + * change the block type.
>>   */
>>  static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
>>  				  int old_mt, int new_mt)
>> @@ -1964,8 +1991,6 @@ static int __move_freepages_block(struct zone *zone, unsigned long start_pfn,
>>  		pages_moved += 1 << order;
>>  	}
>>
>> -	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
>> -
>>  	return pages_moved;
>>  }
>>
>> @@ -2023,11 +2048,16 @@ static int move_freepages_block(struct zone *zone, struct page *page,
>>  				int old_mt, int new_mt)
>>  {
>>  	unsigned long start_pfn;
>> +	int res;
>>
>>  	if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
>>  		return -1;
>>
>> -	return __move_freepages_block(zone, start_pfn, old_mt, new_mt);
>> +	res = __move_freepages_block(zone, start_pfn, old_mt, new_mt);
>> +	set_pageblock_migratetype(pfn_to_page(start_pfn), new_mt);
>> +
>> +	return res;
>> +
>>  }
>>
>>  #ifdef CONFIG_MEMORY_ISOLATION
>> @@ -2055,11 +2085,16 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
>>  	return start_pfn;
>>  }
>>
>> +static inline void toggle_pageblock_isolate(struct page *page)
>> +{
>> +	toggle_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
>> +}
>
> I'm wary about the togle action, as we should always know what action we
> want to do anyway. So we could just add a "bool isolate" parameter and call
> set or clear explicitly? Allows for some hypothetical DEBUG_VM checks too
> (pageblock is not already in the state we want it to be).

This function was added to follow Johannes’ suggestion of getting rid of
if statement. I can change it back, make it explicit, and add
an VM_WARN_ONCE.

Hi Johannes,

If you want the non if statement version to stay, please let me know.


Best Regards,
Yan, Zi


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions.
  2025-05-27 12:55   ` Vlastimil Babka
@ 2025-05-27 15:04     ` Zi Yan
  0 siblings, 0 replies; 18+ messages in thread
From: Zi Yan @ 2025-05-27 15:04 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: David Hildenbrand, Johannes Weiner, linux-mm, Andrew Morton,
	Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
	linux-kernel

On 27 May 2025, at 8:55, Vlastimil Babka wrote:

> On 5/23/25 21:12, Zi Yan wrote:
>> 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 provide the information. At the
>> same time change isolation flags to enum pb_isolate_mode (MEMORY_OFFLINE,
>> CMA_ALLOCATION, and ISOLATE_MODE_OTHERS). Remove REPORT_FAILURE and check
>> MEMORY_OFFLINE instead, since only MEMORY_OFFLINE reports isolation
>> failures.
>>
>> 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 | 19 ++++++++--
>>  include/trace/events/kmem.h    | 14 ++++---
>>  mm/cma.c                       |  2 +-
>>  mm/memory_hotplug.c            |  4 +-
>>  mm/page_alloc.c                | 25 ++++++-------
>>  mm/page_isolation.c            | 67 +++++++++++++++-------------------
>>  8 files changed, 72 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 56d0dbe62163..8accc0f255a8 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 be160e8d8bcb..dea27ed24f8e 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -423,9 +423,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
>
> Would it make sense to define ACR_NONE as 0 so it's more descriptive than
> seeing 0 somewhere?

Totally. Will add one.

>
>> +
>>  /* 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 7a681a49e73c..b61bca909ddc 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -38,8 +38,19 @@ static inline void set_pageblock_isolate(struct page *page)
>>  }
>>  #endif
>>
>> -#define MEMORY_OFFLINE	0x1
>> -#define REPORT_FAILURE	0x2
>> +/*
>> + * Pageblock isolation modes:
>> + * MEMORY_OFFLINE      - isolate to offline (!allocate) memory e.g., skip over
>> + *		         PageHWPoison() pages and PageOffline() pages.
>> + *		         Unmovable pages will be reported in this mode.
>> + * CMA_ALLOCATION      - isolate for CMA allocations
>> + * ISOLATE_MODE_OTHERS - isolate for other purposes
>> + */
>> +enum pb_isolate_mode {
>> +	MEMORY_OFFLINE,
>> +	CMA_ALLOCATION,
>> +	ISOLATE_MODE_OTHERS,
>
> Since this is in a .h file, I'd prefer more consistent naming, and longer
> names shouldn't hurt as there are not that many users.
>
> Even something like:
> PB_ISOLATE_MODE_OFFLINE
> PB_ISOLATE_MODE_CMA
> PB_ISOLATE_MODE_OTHER
>
> ?

OK. I am going to use more descriptive names below:

PB_ISOLATE_MODE_MEM_OFFLINE
PB_ISOLATE_MODE_CMA_ALLOC
PB_ISOLATE_MODE_OTHER

Thank you for all the suggestions and reviews. :)


Best Regards,
Yan, Zi


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2025-05-27 15:04 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-23 19:12 [PATCH v5 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
2025-05-23 19:12 ` [PATCH v5 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
2025-05-27  9:46   ` Vlastimil Babka
2025-05-27 14:47     ` Zi Yan
2025-05-23 19:12 ` [PATCH v5 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
2025-05-27 10:11   ` Vlastimil Babka
2025-05-27 14:56     ` Zi Yan
2025-05-23 19:12 ` [PATCH v5 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
2025-05-27 10:31   ` Vlastimil Babka
2025-05-23 19:12 ` [PATCH v5 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
2025-05-27 10:50   ` Vlastimil Babka
2025-05-27 15:02     ` Zi Yan
2025-05-23 19:12 ` [PATCH v5 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
2025-05-27 10:56   ` Vlastimil Babka
2025-05-23 19:12 ` [PATCH v5 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
2025-05-26  1:33   ` Zi Yan
2025-05-27 12:55   ` Vlastimil Babka
2025-05-27 15:04     ` Zi Yan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).