* [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up.
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 17:01 ` Vlastimil Babka
2025-05-30 19:39 ` David Hildenbrand
2025-05-30 16:22 ` [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
` (4 subsequent siblings)
5 siblings, 2 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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. Rename PB_migrate_skip to PB_compact_skip.
3. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
like PB_compact_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>
---
Documentation/mm/physical_memory.rst | 2 +-
include/linux/mmzone.h | 18 +--
include/linux/page-isolation.h | 2 +-
include/linux/pageblock-flags.h | 34 +++---
mm/memory_hotplug.c | 2 +-
mm/page_alloc.c | 171 +++++++++++++++++++++------
6 files changed, 162 insertions(+), 67 deletions(-)
diff --git a/Documentation/mm/physical_memory.rst b/Documentation/mm/physical_memory.rst
index d3ac106e6b14..9af11b5bd145 100644
--- a/Documentation/mm/physical_memory.rst
+++ b/Documentation/mm/physical_memory.rst
@@ -584,7 +584,7 @@ Compaction control
``compact_blockskip_flush``
Set to true when compaction migration scanner and free scanner meet, which
- means the ``PB_migrate_skip`` bits should be cleared.
+ means the ``PB_compact_skip`` bits should be cleared.
``contiguous``
Set to true when the zone is contiguous (in other words, no hole).
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 87a667533d6d..392a03e37610 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -92,8 +92,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
@@ -122,14 +126,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 e73a4292ef02..451b351c689e 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -19,15 +19,19 @@ enum pageblock_bits {
PB_migrate,
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 */
+ PB_compact_skip,/* If set the block is skipped by compaction */
/*
* 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_compact_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_compact_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_compact_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 cc9577a27ec4..74cb7696e527 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -353,81 +353,174 @@ static inline int pfn_to_bitidx(const struct page *page, unsigned long pfn)
return (pfn >> pageblock_order) * NR_PAGEBLOCK_BITS;
}
+static __always_inline bool is_standalone_pb_bit(enum pageblock_bits pb_bit)
+{
+ return pb_bit > PB_migrate_end && pb_bit < __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(!is_standalone_pb_bit(pb_bit)))
+ 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(!is_standalone_pb_bit(pb_bit)))
+ return;
+
+ get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+ set_bit(bitidx + pb_bit, bitmap_word);
}
-void set_pageblock_migratetype(struct page *page, int migratetype)
+/**
+ * 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(!is_standalone_pb_bit(pb_bit)))
+ return;
+
+ get_pfnblock_bitmap_bitidx(page, pfn, &bitmap_word, &bitidx);
+
+ clear_bit(bitidx + pb_bit, bitmap_word);
+}
+
+/**
+ * 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 +760,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 +786,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 +808,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 +3220,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] 31+ messages in thread
* Re: [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up.
2025-05-30 16:22 ` [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
@ 2025-05-30 17:01 ` Vlastimil Babka
2025-05-30 19:39 ` David Hildenbrand
1 sibling, 0 replies; 31+ messages in thread
From: Vlastimil Babka @ 2025-05-30 17:01 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/30/25 18:22, 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. Rename PB_migrate_skip to PB_compact_skip.
> 3. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
> like PB_compact_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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up.
2025-05-30 16:22 ` [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
2025-05-30 17:01 ` Vlastimil Babka
@ 2025-05-30 19:39 ` David Hildenbrand
1 sibling, 0 replies; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 19:39 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Vlastimil Babka, 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 30.05.25 18:22, 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. Rename PB_migrate_skip to PB_compact_skip.
> 3. Add {get,set,clear}_pfnblock_bit() to operate one a standalone bit,
> like PB_compact_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.
>
I would have further split this up. If your patch description is a list,
it's usually a good indication that each item should be a separate patch.
In any case, nothing jumped at me
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit.
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
2025-05-30 16:22 ` [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 17:04 ` Vlastimil Babka
2025-05-30 19:41 ` David Hildenbrand
2025-05-30 16:22 ` [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
` (3 subsequent siblings)
5 siblings, 2 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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_compact_skip, so that migratetype is not lost during pageblock
isolation.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
include/linux/mmzone.h | 3 +++
include/linux/page-isolation.h | 16 ++++++++++++++++
include/linux/pageblock-flags.h | 14 ++++++++++++++
mm/page_alloc.c | 27 ++++++++++++++++++++++++---
4 files changed, 57 insertions(+), 3 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 392a03e37610..0a5cdc52b405 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -79,6 +79,9 @@ enum migratetype {
* __free_pageblock_cma() function.
*/
MIGRATE_CMA,
+ __MIGRATE_TYPE_END = MIGRATE_CMA,
+#else
+ __MIGRATE_TYPE_END = MIGRATE_HIGHATOMIC,
#endif
#ifdef CONFIG_MEMORY_ISOLATION
MIGRATE_ISOLATE, /* can't allocate from here */
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 451b351c689e..1cf5f0fbd627 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -21,6 +21,13 @@ enum pageblock_bits {
/* 3 bits required for migrate types */
PB_compact_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 +39,13 @@ 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))
+#else
+#define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
+#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 74cb7696e527..5de23eba0db8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -365,8 +365,12 @@ 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);
+#else
BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
- BUILD_BUG_ON(MIGRATE_TYPES > (1 << PB_migratetype_bits));
+#endif
+ BUILD_BUG_ON(__MIGRATE_TYPE_END >= (1 << PB_migratetype_bits));
VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
bitmap = get_pageblock_bitmap(page, pfn);
@@ -439,7 +443,16 @@ 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_AND_ISO_MASK;
+ unsigned long flags;
+
+ 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;
}
/**
@@ -519,8 +532,16 @@ __always_inline void set_pageblock_migratetype(struct page *page,
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;
+ }
+ /* MIGRATETYPE_AND_ISO_MASK clears PB_migrate_isolate if it is set */
+#endif
__set_pfnblock_flags_mask(page, page_to_pfn(page),
- (unsigned long)migratetype, MIGRATETYPE_MASK);
+ (unsigned long)migratetype,
+ MIGRATETYPE_AND_ISO_MASK);
}
#ifdef CONFIG_DEBUG_VM
--
2.47.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit.
2025-05-30 16:22 ` [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
@ 2025-05-30 17:04 ` Vlastimil Babka
2025-05-30 19:41 ` David Hildenbrand
1 sibling, 0 replies; 31+ messages in thread
From: Vlastimil Babka @ 2025-05-30 17:04 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/30/25 18:22, 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_compact_skip, so that migratetype is not lost during pageblock
> isolation.
>
> Signed-off-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit.
2025-05-30 16:22 ` [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
2025-05-30 17:04 ` Vlastimil Babka
@ 2025-05-30 19:41 ` David Hildenbrand
1 sibling, 0 replies; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 19:41 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Vlastimil Babka, 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 30.05.25 18:22, 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_compact_skip, so that migratetype is not lost during pageblock
> isolation.
>
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
2025-05-30 16:22 ` [PATCH v6 1/6] mm/page_alloc: pageblock flags functions clean up Zi Yan
2025-05-30 16:22 ` [PATCH v6 2/6] mm/page_isolation: make page isolation a standalone bit Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 17:06 ` Vlastimil Babka
2025-05-30 19:44 ` David Hildenbrand
2025-05-30 16:22 ` [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
` (2 subsequent siblings)
5 siblings, 2 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
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 8746ed2fec13..afeae59b29e1 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3319,8 +3319,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 8684fa851b84..6e753ca2c338 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 5de23eba0db8..0284d74b6d8e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -544,6 +544,32 @@ __always_inline void set_pageblock_migratetype(struct page *page,
MIGRATETYPE_AND_ISO_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] 31+ messages in thread
* Re: [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 16:22 ` [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
@ 2025-05-30 17:06 ` Vlastimil Babka
2025-05-30 17:10 ` Zi Yan
2025-05-30 17:59 ` Zi Yan
2025-05-30 19:44 ` David Hildenbrand
1 sibling, 2 replies; 31+ messages in thread
From: Vlastimil Babka @ 2025-05-30 17:06 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/30/25 18:22, 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>
> +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);
Nit: I think this could have also used MIGRATETYPE_AND_ISO_MASK unconditionally?
> +}
> +
> #ifdef CONFIG_DEBUG_VM
> static int page_outside_zone_boundaries(struct zone *zone, struct page *page)
> {
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 17:06 ` Vlastimil Babka
@ 2025-05-30 17:10 ` Zi Yan
2025-05-30 17:59 ` Zi Yan
1 sibling, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 17:10 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 30 May 2025, at 13:06, Vlastimil Babka wrote:
> On 5/30/25 18:22, 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>
>
>> +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);
>
> Nit: I think this could have also used MIGRATETYPE_AND_ISO_MASK unconditionally?
Yes, let me send a fixup patch. Thanks for spotting it.
>> +}
>> +
>> #ifdef CONFIG_DEBUG_VM
>> static int page_outside_zone_boundaries(struct zone *zone, struct page *page)
>> {
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 17:06 ` Vlastimil Babka
2025-05-30 17:10 ` Zi Yan
@ 2025-05-30 17:59 ` Zi Yan
1 sibling, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 17:59 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 30 May 2025, at 13:06, Vlastimil Babka wrote:
> On 5/30/25 18:22, 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>
>
>> +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);
>
> Nit: I think this could have also used MIGRATETYPE_AND_ISO_MASK unconditionally?
This is the fixup:
From a701327561cc5d28ffccc66b7bfdabf87f32aa23 Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 30 May 2025 13:22:34 -0400
Subject: [PATCH] init_pageblock_migratetype() fixup.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
mm/page_alloc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 0284d74b6d8e..80bbfc47c9e9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -548,7 +548,6 @@ 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 &&
@@ -563,11 +562,11 @@ void __meminit init_pageblock_migratetype(struct page *page,
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);
+ __set_pfnblock_flags_mask(page, page_to_pfn(page), flags,
+ MIGRATETYPE_AND_ISO_MASK);
}
#ifdef CONFIG_DEBUG_VM
--
2.47.2
Best Regards,
Yan, Zi
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 16:22 ` [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
2025-05-30 17:06 ` Vlastimil Babka
@ 2025-05-30 19:44 ` David Hildenbrand
2025-05-30 19:48 ` Zi Yan
1 sibling, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 19:44 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Vlastimil Babka, 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 30.05.25 18:22, 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>
> ---
[...]
> 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);
Nit: Likely indentation suboptimal?
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated.
2025-05-30 19:44 ` David Hildenbrand
@ 2025-05-30 19:48 ` Zi Yan
0 siblings, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 19:48 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30 May 2025, at 15:44, David Hildenbrand wrote:
> On 30.05.25 18:22, 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>
>> ---
>
> [...]
>
>> 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);
>
> Nit: Likely indentation suboptimal?
Yep.
>
> Acked-by: David Hildenbrand <david@redhat.com>
Thanks.
Here is the fixup:
From 83e9b8e749481b9397b170075cb5280d0640b16d Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 30 May 2025 15:47:06 -0400
Subject: [PATCH] indentation fixup.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
mm/memory_hotplug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 43ac34ee8d2e..16e3ad874144 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1130,7 +1130,7 @@ int mhp_init_memmap_on_memory(unsigned long pfn, unsigned long nr_pages,
page_init_poison(pfn_to_page(pfn), sizeof(struct page) * nr_pages);
move_pfn_range_to_zone(zone, pfn, nr_pages, NULL, MIGRATE_UNMOVABLE,
- false);
+ false);
for (i = 0; i < nr_pages; i++) {
struct page *page = pfn_to_page(pfn + i);
--
2.47.2
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
` (2 preceding siblings ...)
2025-05-30 16:22 ` [PATCH v6 3/6] mm/page_alloc: add support for initializing pageblock as isolated Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 17:10 ` Vlastimil Babka
2025-05-30 19:52 ` David Hildenbrand
2025-05-30 16:22 ` [PATCH v6 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
2025-05-30 16:22 ` [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
5 siblings, 2 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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 | 80 ++++++++++++++++++++++++++--------
mm/page_isolation.c | 21 +++++----
3 files changed, 75 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 0284d74b6d8e..bab114022829 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -525,8 +525,8 @@ void clear_pfnblock_bit(const struct page *page, unsigned long pfn,
* @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)
{
if (unlikely(page_group_by_mobility_disabled &&
migratetype < MIGRATE_PCPTYPES))
@@ -534,9 +534,13 @@ __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;
}
+ VM_WARN_ONCE(get_pfnblock_bit(page, page_to_pfn(page),
+ PB_migrate_isolate),
+ "Use clear_pageblock_isolate() to unisolate pageblock");
/* MIGRATETYPE_AND_ISO_MASK clears PB_migrate_isolate if it is set */
#endif
__set_pfnblock_flags_mask(page, page_to_pfn(page),
@@ -1925,8 +1929,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)
@@ -1958,8 +1962,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;
}
@@ -2017,11 +2019,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
@@ -2049,11 +2056,19 @@ static unsigned long find_large_buddy(unsigned long start_pfn)
return start_pfn;
}
+static inline void toggle_pageblock_isolate(struct page *page, bool isolate)
+{
+ if (isolate)
+ set_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
+ else
+ clear_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
@@ -2068,10 +2083,18 @@ 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)) {
+ VM_WARN_ONCE(1, "%s a pageblock that is already in that state",
+ isolate ? "Isolate" : "Unisolate");
+ return false;
+ }
if (!prep_move_freepages_block(zone, page, &start_pfn, NULL, NULL))
return false;
@@ -2088,7 +2111,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, isolate);
split_large_buddy(zone, buddy, pfn, order, FPI_NONE);
return true;
}
@@ -2099,16 +2122,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, isolate);
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), isolate);
+
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,
@@ -2300,6 +2345,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] 31+ messages in thread
* Re: [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
2025-05-30 16:22 ` [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
@ 2025-05-30 17:10 ` Vlastimil Babka
2025-05-30 19:52 ` David Hildenbrand
1 sibling, 0 replies; 31+ messages in thread
From: Vlastimil Babka @ 2025-05-30 17:10 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/30/25 18:22, 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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
2025-05-30 16:22 ` [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
2025-05-30 17:10 ` Vlastimil Babka
@ 2025-05-30 19:52 ` David Hildenbrand
2025-05-30 19:54 ` Zi Yan
1 sibling, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 19:52 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Vlastimil Babka, 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 30.05.25 18:22, 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>
> ---
[...]
>
> 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)
The function name is a bit misleading. It's more like "unisolate
pageblock", right?
Maybe something to clean up later.
Acked-by: David Hildenbrand <david@redhat.com>
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate()
2025-05-30 19:52 ` David Hildenbrand
@ 2025-05-30 19:54 ` Zi Yan
0 siblings, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 19:54 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30 May 2025, at 15:52, David Hildenbrand wrote:
> On 30.05.25 18:22, 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>
>> ---
>
> [...]
>
>> 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)
>
> The function name is a bit misleading. It's more like "unisolate pageblock", right?
>
> Maybe something to clean up later.
Sure. It can be done when MIGRATE_ISOLATE is removed.
>
> Acked-by: David Hildenbrand <david@redhat.com>
Thanks.
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v6 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range()
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
` (3 preceding siblings ...)
2025-05-30 16:22 ` [PATCH v6 4/6] mm/page_isolation: remove migratetype from move_freepages_block_isolate() Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 16:22 ` [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
5 siblings, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
---
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 bab114022829..a248faf30ee0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6988,7 +6988,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] 31+ messages in thread
* [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 16:22 [PATCH v6 0/6] Make MIGRATE_ISOLATE a standalone bit Zi Yan
` (4 preceding siblings ...)
2025-05-30 16:22 ` [PATCH v6 5/6] mm/page_isolation: remove migratetype from undo_isolate_page_range() Zi Yan
@ 2025-05-30 16:22 ` Zi Yan
2025-05-30 17:15 ` Vlastimil Babka
2025-05-30 19:56 ` David Hildenbrand
5 siblings, 2 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 16:22 UTC (permalink / raw)
To: David Hildenbrand, Johannes Weiner, Vlastimil Babka, linux-mm
Cc: Andrew Morton, Oscar Salvador, 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
(PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
isolation failures.
alloc_contig_range() no longer needs migratetype. Replace it with
enum 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 | 2 +-
include/linux/gfp.h | 9 ++++-
include/linux/page-isolation.h | 20 ++++++++--
include/trace/events/kmem.h | 14 ++++---
mm/cma.c | 2 +-
mm/memory_hotplug.c | 6 +--
mm/page_alloc.c | 27 ++++++-------
mm/page_isolation.c | 70 +++++++++++++++-------------------
8 files changed, 82 insertions(+), 68 deletions(-)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 56d0dbe62163..6bce70b139b2 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1243,7 +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,
+ rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
GFP_KERNEL);
if (rc == -ENOMEM)
/* whoops, out of memory */
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index be160e8d8bcb..51990d571e3e 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -423,9 +423,16 @@ 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
+
+enum acr_flags_t {
+ ACR_CMA, // CMA allocation
+ ACR_OTHER, // other allocation
+};
+
/* 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);
+ enum 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..3e2f960e166c 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
}
#endif
-#define MEMORY_OFFLINE 0x1
-#define REPORT_FAILURE 0x2
+/*
+ * Pageblock isolation modes:
+ * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
+ * e.g., skip over PageHWPoison() pages and
+ * PageOffline() pages. Unmovable pages will be
+ * reported in this mode.
+ * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
+ * PB_ISOLATE_MODE_OTHER - isolate for other purposes
+ */
+enum pb_isolate_mode {
+ PB_ISOLATE_MODE_MEM_OFFLINE,
+ PB_ISOLATE_MODE_CMA_ALLOC,
+ PB_ISOLATE_MODE_OTHER,
+};
void __meminit init_pageblock_migratetype(struct page *page,
enum migratetype migratetype,
@@ -49,10 +61,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..7c4e2e703a23 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),
+ enum 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(enum 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..19cad4460cee 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -2009,8 +2009,7 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
/* set above range as isolated */
ret = start_isolate_page_range(start_pfn, end_pfn,
- MIGRATE_MOVABLE,
- MEMORY_OFFLINE | REPORT_FAILURE);
+ PB_ISOLATE_MODE_MEM_OFFLINE);
if (ret) {
reason = "failure to isolate range";
goto failed_removal_pcplists_disabled;
@@ -2069,7 +2068,8 @@ int offline_pages(unsigned long start_pfn, unsigned long nr_pages,
goto failed_removal_isolated;
}
- ret = test_pages_isolated(start_pfn, end_pfn, MEMORY_OFFLINE);
+ ret = test_pages_isolated(start_pfn, end_pfn,
+ PB_ISOLATE_MODE_MEM_OFFLINE);
} while (ret);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a248faf30ee0..c28e5c2105e5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6697,11 +6697,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,
+ enum acr_flags_t alloc_flags)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned int nr_reclaimed;
@@ -6773,7 +6774,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);
@@ -6844,10 +6845,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.
@@ -6864,7 +6862,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)
+ enum acr_flags_t alloc_flags, gfp_t gfp_mask)
{
unsigned long outer_start, outer_end;
int ret = 0;
@@ -6879,6 +6877,9 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
.alloc_contig = true,
};
INIT_LIST_HEAD(&cc.migratepages);
+ enum pb_isolate_mode mode = (alloc_flags == ACR_CMA) ?
+ PB_ISOLATE_MODE_CMA_ALLOC :
+ PB_ISOLATE_MODE_OTHER;
gfp_mask = current_gfp_context(gfp_mask);
if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
@@ -6905,7 +6906,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;
@@ -6921,7 +6922,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;
@@ -6955,7 +6956,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;
}
@@ -6998,8 +6999,8 @@ 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, ACR_OTHER,
+ 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..ece3bfc56bcd 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 == PB_ISOLATE_MODE_CMA_ALLOC)
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 == PB_ISOLATE_MODE_MEM_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 == PB_ISOLATE_MODE_MEM_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 == PB_ISOLATE_MODE_MEM_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,11 +560,12 @@ __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 == PB_ISOLATE_MODE_MEM_OFFLINE) &&
+ PageHWPoison(page))
/* A HWPoisoned page cannot be also PageBuddy */
pfn++;
- else if ((flags & MEMORY_OFFLINE) && PageOffline(page) &&
- !page_count(page))
+ else if ((mode == PB_ISOLATE_MODE_MEM_OFFLINE) &&
+ PageOffline(page) && !page_count(page))
/*
* The responsible driver agreed to skip PageOffline()
* pages when offlining memory by dropping its
@@ -591,11 +583,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 %PB_ISOLATE_MODE_MEM_OFFLINE 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 +595,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 +631,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] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 16:22 ` [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
@ 2025-05-30 17:15 ` Vlastimil Babka
2025-05-30 17:21 ` Zi Yan
2025-05-30 18:01 ` Zi Yan
2025-05-30 19:56 ` David Hildenbrand
1 sibling, 2 replies; 31+ messages in thread
From: Vlastimil Babka @ 2025-05-30 17:15 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/30/25 18:22, 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
> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
> isolation failures.
>
> alloc_contig_range() no longer needs migratetype. Replace it with
> enum 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>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> drivers/virtio/virtio_mem.c | 2 +-
> include/linux/gfp.h | 9 ++++-
> include/linux/page-isolation.h | 20 ++++++++--
> include/trace/events/kmem.h | 14 ++++---
> mm/cma.c | 2 +-
> mm/memory_hotplug.c | 6 +--
> mm/page_alloc.c | 27 ++++++-------
> mm/page_isolation.c | 70 +++++++++++++++-------------------
> 8 files changed, 82 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 56d0dbe62163..6bce70b139b2 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -1243,7 +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,
> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
> GFP_KERNEL);
> if (rc == -ENOMEM)
> /* whoops, out of memory */
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index be160e8d8bcb..51990d571e3e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -423,9 +423,16 @@ 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
> +
> +enum acr_flags_t {
> + ACR_CMA, // CMA allocation
> + ACR_OTHER, // other allocation
> +};
> +
> /* 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);
> + enum acr_flags_t alloc_flags,
> + gfp_t gfp_mask);
Nit: when used this way I think it's also rather a "mode" than "flags". It's
probably sufficient though, I don't expect we'll be adding more and want to
combine them in a flags way. So it's just about the enum's name.
> #define alloc_contig_range(...) alloc_hooks(alloc_contig_range_noprof(__VA_ARGS__))
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 17:15 ` Vlastimil Babka
@ 2025-05-30 17:21 ` Zi Yan
2025-05-30 18:01 ` Zi Yan
1 sibling, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 17:21 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 30 May 2025, at 13:15, Vlastimil Babka wrote:
> On 5/30/25 18:22, 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
>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>> isolation failures.
>>
>> alloc_contig_range() no longer needs migratetype. Replace it with
>> enum 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>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
>> ---
>> drivers/virtio/virtio_mem.c | 2 +-
>> include/linux/gfp.h | 9 ++++-
>> include/linux/page-isolation.h | 20 ++++++++--
>> include/trace/events/kmem.h | 14 ++++---
>> mm/cma.c | 2 +-
>> mm/memory_hotplug.c | 6 +--
>> mm/page_alloc.c | 27 ++++++-------
>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 56d0dbe62163..6bce70b139b2 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -1243,7 +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,
>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>> GFP_KERNEL);
>> if (rc == -ENOMEM)
>> /* whoops, out of memory */
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index be160e8d8bcb..51990d571e3e 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -423,9 +423,16 @@ 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
>> +
>> +enum acr_flags_t {
>> + ACR_CMA, // CMA allocation
>> + ACR_OTHER, // other allocation
>> +};
>> +
>> /* 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);
>> + enum acr_flags_t alloc_flags,
>> + gfp_t gfp_mask);
>
> Nit: when used this way I think it's also rather a "mode" than "flags". It's
> probably sufficient though, I don't expect we'll be adding more and want to
> combine them in a flags way. So it's just about the enum's name.
>
So enum acr_flags_t to enum acr_mode_t? Sure. Let me send a fixup.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 17:15 ` Vlastimil Babka
2025-05-30 17:21 ` Zi Yan
@ 2025-05-30 18:01 ` Zi Yan
1 sibling, 0 replies; 31+ messages in thread
From: Zi Yan @ 2025-05-30 18:01 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 30 May 2025, at 13:15, Vlastimil Babka wrote:
> On 5/30/25 18:22, 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
>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>> isolation failures.
>>
>> alloc_contig_range() no longer needs migratetype. Replace it with
>> enum 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>
>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
>
>> ---
>> drivers/virtio/virtio_mem.c | 2 +-
>> include/linux/gfp.h | 9 ++++-
>> include/linux/page-isolation.h | 20 ++++++++--
>> include/trace/events/kmem.h | 14 ++++---
>> mm/cma.c | 2 +-
>> mm/memory_hotplug.c | 6 +--
>> mm/page_alloc.c | 27 ++++++-------
>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 56d0dbe62163..6bce70b139b2 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -1243,7 +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,
>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>> GFP_KERNEL);
>> if (rc == -ENOMEM)
>> /* whoops, out of memory */
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index be160e8d8bcb..51990d571e3e 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -423,9 +423,16 @@ 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
>> +
>> +enum acr_flags_t {
>> + ACR_CMA, // CMA allocation
>> + ACR_OTHER, // other allocation
>> +};
>> +
>> /* 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);
>> + enum acr_flags_t alloc_flags,
>> + gfp_t gfp_mask);
>
> Nit: when used this way I think it's also rather a "mode" than "flags". It's
> probably sufficient though, I don't expect we'll be adding more and want to
> combine them in a flags way. So it's just about the enum's name.
This is the fixup (variable alloc_flags is renamed to alloc_mode too):
From 9674b741eb93eb74de52fb28c644f523387d1e9f Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 30 May 2025 13:58:11 -0400
Subject: [PATCH] rename acr_flags_t to acr_mode.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
include/linux/gfp.h | 4 ++--
include/trace/events/kmem.h | 12 ++++++------
mm/page_alloc.c | 14 +++++++-------
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 51990d571e3e..363771903be3 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -424,14 +424,14 @@ extern gfp_t vma_thp_gfp_mask(struct vm_area_struct *vma);
#ifdef CONFIG_CONTIG_ALLOC
-enum acr_flags_t {
+enum acr_mode {
ACR_CMA, // CMA allocation
ACR_OTHER, // other allocation
};
/* 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,
- enum acr_flags_t alloc_flags,
+ enum acr_mode alloc_mode,
gfp_t gfp_mask);
#define alloc_contig_range(...) alloc_hooks(alloc_contig_range_noprof(__VA_ARGS__))
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 7c4e2e703a23..4ac8fbff224c 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -312,9 +312,9 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
unsigned long nr_migrated,
unsigned long nr_reclaimed,
unsigned long nr_mapped,
- enum acr_flags_t alloc_flags),
+ enum acr_mode alloc_mode),
- TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_flags),
+ TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_mode),
TP_STRUCT__entry(
__field(unsigned long, start)
@@ -322,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(enum acr_flags_t, alloc_flags)
+ __field(enum acr_mode, alloc_mode)
),
TP_fast_assign(
@@ -331,13 +331,13 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
__entry->nr_migrated = nr_migrated;
__entry->nr_reclaimed = nr_reclaimed;
__entry->nr_mapped = nr_mapped;
- __entry->alloc_flags = alloc_flags;
+ __entry->alloc_mode = alloc_mode;
),
- TP_printk("start=0x%lx end=0x%lx alloc_flags=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
+ TP_printk("start=0x%lx end=0x%lx alloc_mode=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
__entry->start,
__entry->end,
- __entry->alloc_flags,
+ __entry->alloc_mode,
__entry->nr_migrated,
__entry->nr_reclaimed,
__entry->nr_mapped)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dd761f5e6310..dc418510aba2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6696,12 +6696,12 @@ static void alloc_contig_dump_pages(struct list_head *page_list)
/*
* [start, end) must belong to a single zone.
- * @alloc_flags: using acr_flags_t to filter the type of migration in
+ * @alloc_mode: using acr_mode 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,
- enum acr_flags_t alloc_flags)
+ enum acr_mode alloc_mode)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned int nr_reclaimed;
@@ -6773,7 +6773,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, alloc_flags,
+ trace_mm_alloc_contig_migrate_range_info(start, end, alloc_mode,
total_migrated,
total_reclaimed,
total_mapped);
@@ -6844,7 +6844,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
- * @alloc_flags: allocation information
+ * @alloc_mode: 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.
@@ -6861,7 +6861,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,
- enum acr_flags_t alloc_flags, gfp_t gfp_mask)
+ enum acr_mode alloc_mode, gfp_t gfp_mask)
{
unsigned long outer_start, outer_end;
int ret = 0;
@@ -6876,7 +6876,7 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
.alloc_contig = true,
};
INIT_LIST_HEAD(&cc.migratepages);
- enum pb_isolate_mode mode = (alloc_flags == ACR_CMA) ?
+ enum pb_isolate_mode mode = (alloc_mode == ACR_CMA) ?
PB_ISOLATE_MODE_CMA_ALLOC :
PB_ISOLATE_MODE_OTHER;
@@ -6921,7 +6921,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, alloc_flags);
+ ret = __alloc_contig_migrate_range(&cc, start, end, alloc_mode);
if (ret && ret != -EBUSY)
goto done;
--
2.47.2
Best Regards,
Yan, Zi
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 16:22 ` [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions Zi Yan
2025-05-30 17:15 ` Vlastimil Babka
@ 2025-05-30 19:56 ` David Hildenbrand
2025-05-30 19:58 ` Zi Yan
1 sibling, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 19:56 UTC (permalink / raw)
To: Zi Yan, Johannes Weiner, Vlastimil Babka, 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 30.05.25 18:22, 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
> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
> isolation failures.
>
> alloc_contig_range() no longer needs migratetype. Replace it with
> enum 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 | 2 +-
> include/linux/gfp.h | 9 ++++-
> include/linux/page-isolation.h | 20 ++++++++--
> include/trace/events/kmem.h | 14 ++++---
> mm/cma.c | 2 +-
> mm/memory_hotplug.c | 6 +--
> mm/page_alloc.c | 27 ++++++-------
> mm/page_isolation.c | 70 +++++++++++++++-------------------
> 8 files changed, 82 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
> index 56d0dbe62163..6bce70b139b2 100644
> --- a/drivers/virtio/virtio_mem.c
> +++ b/drivers/virtio/virtio_mem.c
> @@ -1243,7 +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,
> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
> GFP_KERNEL);
> if (rc == -ENOMEM)
> /* whoops, out of memory */
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index be160e8d8bcb..51990d571e3e 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -423,9 +423,16 @@ 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
> +
> +enum acr_flags_t {
> + ACR_CMA, // CMA allocation
> + ACR_OTHER, // other allocation
> +};
Hm, enum != flags.
If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied
if not set.
And ACR_CMA would then have to be "1" etc.
> +
> /* 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);
> + enum 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..3e2f960e166c 100644
> --- a/include/linux/page-isolation.h
> +++ b/include/linux/page-isolation.h
> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
> }
> #endif
>
> -#define MEMORY_OFFLINE 0x1
> -#define REPORT_FAILURE 0x2
> +/*
> + * Pageblock isolation modes:
> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
> + * e.g., skip over PageHWPoison() pages and
> + * PageOffline() pages. Unmovable pages will be
> + * reported in this mode.
> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
> + */
> +enum pb_isolate_mode {
> + PB_ISOLATE_MODE_MEM_OFFLINE,
> + PB_ISOLATE_MODE_CMA_ALLOC,
> + PB_ISOLATE_MODE_OTHER,
> +};
It's late on friady, but it looks like we are duplicating things here.
Let me think about that once my brain is recharged :)
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 19:56 ` David Hildenbrand
@ 2025-05-30 19:58 ` Zi Yan
2025-05-30 20:08 ` David Hildenbrand
0 siblings, 1 reply; 31+ messages in thread
From: Zi Yan @ 2025-05-30 19:58 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30 May 2025, at 15:56, David Hildenbrand wrote:
> On 30.05.25 18:22, 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
>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>> isolation failures.
>>
>> alloc_contig_range() no longer needs migratetype. Replace it with
>> enum 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 | 2 +-
>> include/linux/gfp.h | 9 ++++-
>> include/linux/page-isolation.h | 20 ++++++++--
>> include/trace/events/kmem.h | 14 ++++---
>> mm/cma.c | 2 +-
>> mm/memory_hotplug.c | 6 +--
>> mm/page_alloc.c | 27 ++++++-------
>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>> index 56d0dbe62163..6bce70b139b2 100644
>> --- a/drivers/virtio/virtio_mem.c
>> +++ b/drivers/virtio/virtio_mem.c
>> @@ -1243,7 +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,
>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>> GFP_KERNEL);
>> if (rc == -ENOMEM)
>> /* whoops, out of memory */
>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>> index be160e8d8bcb..51990d571e3e 100644
>> --- a/include/linux/gfp.h
>> +++ b/include/linux/gfp.h
>> @@ -423,9 +423,16 @@ 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
>> +
>> +enum acr_flags_t {
>> + ACR_CMA, // CMA allocation
>> + ACR_OTHER, // other allocation
>> +};
>
> Hm, enum != flags.
>
> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>
> And ACR_CMA would then have to be "1" etc.
I have a fixup to change acr_flags_t to acr_mode.
>
>> +
>> /* 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);
>> + enum 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..3e2f960e166c 100644
>> --- a/include/linux/page-isolation.h
>> +++ b/include/linux/page-isolation.h
>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>> }
>> #endif
>> -#define MEMORY_OFFLINE 0x1
>> -#define REPORT_FAILURE 0x2
>> +/*
>> + * Pageblock isolation modes:
>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>> + * e.g., skip over PageHWPoison() pages and
>> + * PageOffline() pages. Unmovable pages will be
>> + * reported in this mode.
>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>> + */
>> +enum pb_isolate_mode {
>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>> + PB_ISOLATE_MODE_CMA_ALLOC,
>> + PB_ISOLATE_MODE_OTHER,
>> +};
>
> It's late on friady, but it looks like we are duplicating things here.
>
> Let me think about that once my brain is recharged :)
Sure. Take your time.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 19:58 ` Zi Yan
@ 2025-05-30 20:08 ` David Hildenbrand
2025-05-30 20:46 ` Zi Yan
0 siblings, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 20:08 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30.05.25 21:58, Zi Yan wrote:
> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>
>> On 30.05.25 18:22, 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
>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>> isolation failures.
>>>
>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>> enum 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 | 2 +-
>>> include/linux/gfp.h | 9 ++++-
>>> include/linux/page-isolation.h | 20 ++++++++--
>>> include/trace/events/kmem.h | 14 ++++---
>>> mm/cma.c | 2 +-
>>> mm/memory_hotplug.c | 6 +--
>>> mm/page_alloc.c | 27 ++++++-------
>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>> index 56d0dbe62163..6bce70b139b2 100644
>>> --- a/drivers/virtio/virtio_mem.c
>>> +++ b/drivers/virtio/virtio_mem.c
>>> @@ -1243,7 +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,
>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>> GFP_KERNEL);
>>> if (rc == -ENOMEM)
>>> /* whoops, out of memory */
>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>> index be160e8d8bcb..51990d571e3e 100644
>>> --- a/include/linux/gfp.h
>>> +++ b/include/linux/gfp.h
>>> @@ -423,9 +423,16 @@ 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
>>> +
>>> +enum acr_flags_t {
>>> + ACR_CMA, // CMA allocation
>>> + ACR_OTHER, // other allocation
>>> +};
>>
>> Hm, enum != flags.
>>
>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>
>> And ACR_CMA would then have to be "1" etc.
>
> I have a fixup to change acr_flags_t to acr_mode.
>
>>
>>> +
>>> /* 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);
>>> + enum 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..3e2f960e166c 100644
>>> --- a/include/linux/page-isolation.h
>>> +++ b/include/linux/page-isolation.h
>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>> }
>>> #endif
>>> -#define MEMORY_OFFLINE 0x1
>>> -#define REPORT_FAILURE 0x2
>>> +/*
>>> + * Pageblock isolation modes:
>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>> + * e.g., skip over PageHWPoison() pages and
>>> + * PageOffline() pages. Unmovable pages will be
>>> + * reported in this mode.
>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>> + */
>>> +enum pb_isolate_mode {
>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>> + PB_ISOLATE_MODE_OTHER,
>>> +};
>>
>> It's late on friady, but it looks like we are duplicating things here.
>>
>> Let me think about that once my brain is recharged :)
>
> Sure. Take your time.
Could we abstract both settings and use a single one? Then, we could
simply reject if MEM_OFFLINE is passed into alloc_contig_range().
alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an
allocation. CMA is an allocation.
Just an idea, not sure ...
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 20:08 ` David Hildenbrand
@ 2025-05-30 20:46 ` Zi Yan
2025-05-30 20:55 ` David Hildenbrand
0 siblings, 1 reply; 31+ messages in thread
From: Zi Yan @ 2025-05-30 20:46 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30 May 2025, at 16:08, David Hildenbrand wrote:
> On 30.05.25 21:58, Zi Yan wrote:
>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>
>>> On 30.05.25 18:22, 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
>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>> isolation failures.
>>>>
>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>> enum 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 | 2 +-
>>>> include/linux/gfp.h | 9 ++++-
>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>> include/trace/events/kmem.h | 14 ++++---
>>>> mm/cma.c | 2 +-
>>>> mm/memory_hotplug.c | 6 +--
>>>> mm/page_alloc.c | 27 ++++++-------
>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>
>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>> --- a/drivers/virtio/virtio_mem.c
>>>> +++ b/drivers/virtio/virtio_mem.c
>>>> @@ -1243,7 +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,
>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>> GFP_KERNEL);
>>>> if (rc == -ENOMEM)
>>>> /* whoops, out of memory */
>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>> index be160e8d8bcb..51990d571e3e 100644
>>>> --- a/include/linux/gfp.h
>>>> +++ b/include/linux/gfp.h
>>>> @@ -423,9 +423,16 @@ 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
>>>> +
>>>> +enum acr_flags_t {
>>>> + ACR_CMA, // CMA allocation
>>>> + ACR_OTHER, // other allocation
>>>> +};
>>>
>>> Hm, enum != flags.
>>>
>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>
>>> And ACR_CMA would then have to be "1" etc.
>>
>> I have a fixup to change acr_flags_t to acr_mode.
>>
>>>
>>>> +
>>>> /* 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);
>>>> + enum 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..3e2f960e166c 100644
>>>> --- a/include/linux/page-isolation.h
>>>> +++ b/include/linux/page-isolation.h
>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>> }
>>>> #endif
>>>> -#define MEMORY_OFFLINE 0x1
>>>> -#define REPORT_FAILURE 0x2
>>>> +/*
>>>> + * Pageblock isolation modes:
>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>> + * e.g., skip over PageHWPoison() pages and
>>>> + * PageOffline() pages. Unmovable pages will be
>>>> + * reported in this mode.
>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>> + */
>>>> +enum pb_isolate_mode {
>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>> + PB_ISOLATE_MODE_OTHER,
>>>> +};
>>>
>>> It's late on friady, but it looks like we are duplicating things here.
>>>
>>> Let me think about that once my brain is recharged :)
>>
>> Sure. Take your time.
>
> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>
> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>
> Just an idea, not sure ...
I think so.
This is the fixup of removing acr_flags_t. It is on top of the original
patch. Take your time. I guess I will send V7 with all fixups next week.
The one strange part is that in virtio_mem_fake_offline,
PB_ISOLATE_MODE_OTHER is used instead of PB_ISOLATE_MODE_MEM_OFFLINE.
I guess you do not want to report failures there.
From b4bed6ec8bd07df40952bff2009905ae4093b3be Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 30 May 2025 13:58:11 -0400
Subject: [PATCH] remove acr_flags_t and use enum pb_isolate_mode instead.
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
drivers/virtio/virtio_mem.c | 4 ++--
include/linux/gfp.h | 19 ++++++++++++++-----
include/linux/page-isolation.h | 15 ---------------
include/trace/events/kmem.h | 12 ++++++------
mm/cma.c | 3 ++-
mm/page_alloc.c | 24 ++++++++++++------------
6 files changed, 36 insertions(+), 41 deletions(-)
diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
index 6bce70b139b2..535680a54ff5 100644
--- a/drivers/virtio/virtio_mem.c
+++ b/drivers/virtio/virtio_mem.c
@@ -1243,8 +1243,8 @@ 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, ACR_OTHER,
- GFP_KERNEL);
+ rc = alloc_contig_range(pfn, pfn + nr_pages,
+ PB_ISOLATE_MODE_OTHER, GFP_KERNEL);
if (rc == -ENOMEM)
/* whoops, out of memory */
return rc;
diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 51990d571e3e..17b92888d6de 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -423,15 +423,24 @@ 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
-
-enum acr_flags_t {
- ACR_CMA, // CMA allocation
- ACR_OTHER, // other allocation
+/*
+ * Pageblock isolation modes:
+ * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
+ * e.g., skip over PageHWPoison() pages and
+ * PageOffline() pages. Unmovable pages will be
+ * reported in this mode.
+ * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
+ * PB_ISOLATE_MODE_OTHER - isolate for other purposes
+ */
+enum pb_isolate_mode {
+ PB_ISOLATE_MODE_MEM_OFFLINE,
+ PB_ISOLATE_MODE_CMA_ALLOC,
+ PB_ISOLATE_MODE_OTHER,
};
/* 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,
- enum acr_flags_t alloc_flags,
+ enum pb_isolate_mode isol_mode,
gfp_t gfp_mask);
#define alloc_contig_range(...) alloc_hooks(alloc_contig_range_noprof(__VA_ARGS__))
diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 3e2f960e166c..7ed60a339a02 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -38,21 +38,6 @@ static inline void set_pageblock_isolate(struct page *page)
}
#endif
-/*
- * Pageblock isolation modes:
- * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
- * e.g., skip over PageHWPoison() pages and
- * PageOffline() pages. Unmovable pages will be
- * reported in this mode.
- * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
- * PB_ISOLATE_MODE_OTHER - isolate for other purposes
- */
-enum pb_isolate_mode {
- PB_ISOLATE_MODE_MEM_OFFLINE,
- PB_ISOLATE_MODE_CMA_ALLOC,
- PB_ISOLATE_MODE_OTHER,
-};
-
void __meminit init_pageblock_migratetype(struct page *page,
enum migratetype migratetype,
bool isolate);
diff --git a/include/trace/events/kmem.h b/include/trace/events/kmem.h
index 7c4e2e703a23..e0bcbc43a548 100644
--- a/include/trace/events/kmem.h
+++ b/include/trace/events/kmem.h
@@ -312,9 +312,9 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
unsigned long nr_migrated,
unsigned long nr_reclaimed,
unsigned long nr_mapped,
- enum acr_flags_t alloc_flags),
+ enum pb_isolate_mode isol_mode),
- TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, alloc_flags),
+ TP_ARGS(start, end, nr_migrated, nr_reclaimed, nr_mapped, isol_mode),
TP_STRUCT__entry(
__field(unsigned long, start)
@@ -322,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(enum acr_flags_t, alloc_flags)
+ __field(enum pb_isolate_mode, isol_mode)
),
TP_fast_assign(
@@ -331,13 +331,13 @@ TRACE_EVENT(mm_alloc_contig_migrate_range_info,
__entry->nr_migrated = nr_migrated;
__entry->nr_reclaimed = nr_reclaimed;
__entry->nr_mapped = nr_mapped;
- __entry->alloc_flags = alloc_flags;
+ __entry->isol_mode = isol_mode;
),
- TP_printk("start=0x%lx end=0x%lx alloc_flags=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
+ TP_printk("start=0x%lx end=0x%lx isol_mode=%d nr_migrated=%lu nr_reclaimed=%lu nr_mapped=%lu",
__entry->start,
__entry->end,
- __entry->alloc_flags,
+ __entry->isol_mode,
__entry->nr_migrated,
__entry->nr_reclaimed,
__entry->nr_mapped)
diff --git a/mm/cma.c b/mm/cma.c
index 9ee8fad797bc..23aa35193122 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -822,7 +822,8 @@ 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, ACR_CMA, gfp);
+ ret = alloc_contig_range(pfn, pfn + count,
+ PB_ISOLATE_MODE_CMA_ALLOC, gfp);
mutex_unlock(&cma->alloc_mutex);
if (ret == 0) {
page = pfn_to_page(pfn);
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index dd761f5e6310..619b1a9de9b7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6696,12 +6696,12 @@ static void alloc_contig_dump_pages(struct list_head *page_list)
/*
* [start, end) must belong to a single zone.
- * @alloc_flags: using acr_flags_t to filter the type of migration in
+ * @isol_mode: using pb_isolate_mode 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,
- enum acr_flags_t alloc_flags)
+ enum pb_isolate_mode isol_mode)
{
/* This function is based on compact_zone() from compaction.c. */
unsigned int nr_reclaimed;
@@ -6773,7 +6773,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, alloc_flags,
+ trace_mm_alloc_contig_migrate_range_info(start, end, isol_mode,
total_migrated,
total_reclaimed,
total_mapped);
@@ -6844,7 +6844,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
- * @alloc_flags: allocation information
+ * @isol_mode: allocation information used for pageblock isolation
* @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.
@@ -6861,7 +6861,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,
- enum acr_flags_t alloc_flags, gfp_t gfp_mask)
+ enum pb_isolate_mode isol_mode, gfp_t gfp_mask)
{
unsigned long outer_start, outer_end;
int ret = 0;
@@ -6876,9 +6876,9 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
.alloc_contig = true,
};
INIT_LIST_HEAD(&cc.migratepages);
- enum pb_isolate_mode mode = (alloc_flags == ACR_CMA) ?
- PB_ISOLATE_MODE_CMA_ALLOC :
- PB_ISOLATE_MODE_OTHER;
+
+ if (isol_mode == PB_ISOLATE_MODE_MEM_OFFLINE)
+ return -EINVAL;
gfp_mask = current_gfp_context(gfp_mask);
if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
@@ -6905,7 +6905,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, mode);
+ ret = start_isolate_page_range(start, end, isol_mode);
if (ret)
goto done;
@@ -6921,7 +6921,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, alloc_flags);
+ ret = __alloc_contig_migrate_range(&cc, start, end, isol_mode);
if (ret && ret != -EBUSY)
goto done;
@@ -6955,7 +6955,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, mode)) {
+ if (test_pages_isolated(outer_start, end, isol_mode)) {
ret = -EBUSY;
goto done;
}
@@ -6998,7 +6998,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, ACR_OTHER,
+ return alloc_contig_range_noprof(start_pfn, end_pfn, PB_ISOLATE_MODE_OTHER,
gfp_mask);
}
--
2.47.2
Best Regards,
Yan, Zi
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 20:46 ` Zi Yan
@ 2025-05-30 20:55 ` David Hildenbrand
2025-06-02 14:18 ` Zi Yan
0 siblings, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-05-30 20:55 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30.05.25 22:46, Zi Yan wrote:
> On 30 May 2025, at 16:08, David Hildenbrand wrote:
>
>> On 30.05.25 21:58, Zi Yan wrote:
>>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>>
>>>> On 30.05.25 18:22, 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
>>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>>> isolation failures.
>>>>>
>>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>>> enum 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 | 2 +-
>>>>> include/linux/gfp.h | 9 ++++-
>>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>>> include/trace/events/kmem.h | 14 ++++---
>>>>> mm/cma.c | 2 +-
>>>>> mm/memory_hotplug.c | 6 +--
>>>>> mm/page_alloc.c | 27 ++++++-------
>>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>>
>>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>>> --- a/drivers/virtio/virtio_mem.c
>>>>> +++ b/drivers/virtio/virtio_mem.c
>>>>> @@ -1243,7 +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,
>>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>>> GFP_KERNEL);
>>>>> if (rc == -ENOMEM)
>>>>> /* whoops, out of memory */
>>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>>> index be160e8d8bcb..51990d571e3e 100644
>>>>> --- a/include/linux/gfp.h
>>>>> +++ b/include/linux/gfp.h
>>>>> @@ -423,9 +423,16 @@ 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
>>>>> +
>>>>> +enum acr_flags_t {
>>>>> + ACR_CMA, // CMA allocation
>>>>> + ACR_OTHER, // other allocation
>>>>> +};
>>>>
>>>> Hm, enum != flags.
>>>>
>>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>>
>>>> And ACR_CMA would then have to be "1" etc.
>>>
>>> I have a fixup to change acr_flags_t to acr_mode.
>>>
>>>>
>>>>> +
>>>>> /* 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);
>>>>> + enum 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..3e2f960e166c 100644
>>>>> --- a/include/linux/page-isolation.h
>>>>> +++ b/include/linux/page-isolation.h
>>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>>> }
>>>>> #endif
>>>>> -#define MEMORY_OFFLINE 0x1
>>>>> -#define REPORT_FAILURE 0x2
>>>>> +/*
>>>>> + * Pageblock isolation modes:
>>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>>> + * e.g., skip over PageHWPoison() pages and
>>>>> + * PageOffline() pages. Unmovable pages will be
>>>>> + * reported in this mode.
>>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>>> + */
>>>>> +enum pb_isolate_mode {
>>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>>> + PB_ISOLATE_MODE_OTHER,
>>>>> +};
>>>>
>>>> It's late on friady, but it looks like we are duplicating things here.
>>>>
>>>> Let me think about that once my brain is recharged :)
>>>
>>> Sure. Take your time.
>>
>> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>>
>> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>>
>> Just an idea, not sure ...
>
> I think so.
>
> This is the fixup of removing acr_flags_t. It is on top of the original
> patch. Take your time. I guess I will send V7 with all fixups next week.
Only wondering if we should rename "pb_isolate_mode" to something more
abstract, that covers both use cases clearer.
Hmmm.
>
> The one strange part is that in virtio_mem_fake_offline,
> PB_ISOLATE_MODE_OTHER is used instead of PB_ISOLATE_MODE_MEM_OFFLINE.
> I guess you do not want to report failures there.
That's the right thing to do. It's not ordinary memory offlining code.
Thanks and enjoy the weekend!
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-05-30 20:55 ` David Hildenbrand
@ 2025-06-02 14:18 ` Zi Yan
2025-06-02 16:25 ` David Hildenbrand
0 siblings, 1 reply; 31+ messages in thread
From: Zi Yan @ 2025-06-02 14:18 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 30 May 2025, at 16:55, David Hildenbrand wrote:
> On 30.05.25 22:46, Zi Yan wrote:
>> On 30 May 2025, at 16:08, David Hildenbrand wrote:
>>
>>> On 30.05.25 21:58, Zi Yan wrote:
>>>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>>>
>>>>> On 30.05.25 18:22, 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
>>>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>>>> isolation failures.
>>>>>>
>>>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>>>> enum 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 | 2 +-
>>>>>> include/linux/gfp.h | 9 ++++-
>>>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>>>> include/trace/events/kmem.h | 14 ++++---
>>>>>> mm/cma.c | 2 +-
>>>>>> mm/memory_hotplug.c | 6 +--
>>>>>> mm/page_alloc.c | 27 ++++++-------
>>>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>>>> --- a/drivers/virtio/virtio_mem.c
>>>>>> +++ b/drivers/virtio/virtio_mem.c
>>>>>> @@ -1243,7 +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,
>>>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>>>> GFP_KERNEL);
>>>>>> if (rc == -ENOMEM)
>>>>>> /* whoops, out of memory */
>>>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>>>> index be160e8d8bcb..51990d571e3e 100644
>>>>>> --- a/include/linux/gfp.h
>>>>>> +++ b/include/linux/gfp.h
>>>>>> @@ -423,9 +423,16 @@ 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
>>>>>> +
>>>>>> +enum acr_flags_t {
>>>>>> + ACR_CMA, // CMA allocation
>>>>>> + ACR_OTHER, // other allocation
>>>>>> +};
>>>>>
>>>>> Hm, enum != flags.
>>>>>
>>>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>>>
>>>>> And ACR_CMA would then have to be "1" etc.
>>>>
>>>> I have a fixup to change acr_flags_t to acr_mode.
>>>>
>>>>>
>>>>>> +
>>>>>> /* 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);
>>>>>> + enum 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..3e2f960e166c 100644
>>>>>> --- a/include/linux/page-isolation.h
>>>>>> +++ b/include/linux/page-isolation.h
>>>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>>>> }
>>>>>> #endif
>>>>>> -#define MEMORY_OFFLINE 0x1
>>>>>> -#define REPORT_FAILURE 0x2
>>>>>> +/*
>>>>>> + * Pageblock isolation modes:
>>>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>>>> + * e.g., skip over PageHWPoison() pages and
>>>>>> + * PageOffline() pages. Unmovable pages will be
>>>>>> + * reported in this mode.
>>>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>>>> + */
>>>>>> +enum pb_isolate_mode {
>>>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>> + PB_ISOLATE_MODE_OTHER,
>>>>>> +};
>>>>>
>>>>> It's late on friady, but it looks like we are duplicating things here.
>>>>>
>>>>> Let me think about that once my brain is recharged :)
>>>>
>>>> Sure. Take your time.
>>>
>>> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>>>
>>> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>>>
>>> Just an idea, not sure ...
>>
>> I think so.
>>
>> This is the fixup of removing acr_flags_t. It is on top of the original
>> patch. Take your time. I guess I will send V7 with all fixups next week.
>
> Only wondering if we should rename "pb_isolate_mode" to something more abstract, that covers both use cases clearer.
>
> Hmmm.
It is specifying the purpose of either alloc_contig_range() or
test_pages_isolated(), but these two use cases do not have high level
connection AFAICT, except that pageblock isolation is involved
in the implementation. Let’s postpone the naming to the future.
As we discussed, we have two TODOs: 1) remove MIGRATE_ISOLATE,
and 2) simplify alloc_contig_range() to isolate the whole range in
one shot, then do migration. Probably we can think about naming at
that time. :)
>
>>
>> The one strange part is that in virtio_mem_fake_offline,
>> PB_ISOLATE_MODE_OTHER is used instead of PB_ISOLATE_MODE_MEM_OFFLINE.
>> I guess you do not want to report failures there.
>
> That's the right thing to do. It's not ordinary memory offlining code.
Got it.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-06-02 14:18 ` Zi Yan
@ 2025-06-02 16:25 ` David Hildenbrand
2025-06-02 16:27 ` Zi Yan
0 siblings, 1 reply; 31+ messages in thread
From: David Hildenbrand @ 2025-06-02 16:25 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 02.06.25 16:18, Zi Yan wrote:
> On 30 May 2025, at 16:55, David Hildenbrand wrote:
>
>> On 30.05.25 22:46, Zi Yan wrote:
>>> On 30 May 2025, at 16:08, David Hildenbrand wrote:
>>>
>>>> On 30.05.25 21:58, Zi Yan wrote:
>>>>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>>>>
>>>>>> On 30.05.25 18:22, 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
>>>>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>>>>> isolation failures.
>>>>>>>
>>>>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>>>>> enum 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 | 2 +-
>>>>>>> include/linux/gfp.h | 9 ++++-
>>>>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>>>>> include/trace/events/kmem.h | 14 ++++---
>>>>>>> mm/cma.c | 2 +-
>>>>>>> mm/memory_hotplug.c | 6 +--
>>>>>>> mm/page_alloc.c | 27 ++++++-------
>>>>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>>>>> --- a/drivers/virtio/virtio_mem.c
>>>>>>> +++ b/drivers/virtio/virtio_mem.c
>>>>>>> @@ -1243,7 +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,
>>>>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>>>>> GFP_KERNEL);
>>>>>>> if (rc == -ENOMEM)
>>>>>>> /* whoops, out of memory */
>>>>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>>>>> index be160e8d8bcb..51990d571e3e 100644
>>>>>>> --- a/include/linux/gfp.h
>>>>>>> +++ b/include/linux/gfp.h
>>>>>>> @@ -423,9 +423,16 @@ 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
>>>>>>> +
>>>>>>> +enum acr_flags_t {
>>>>>>> + ACR_CMA, // CMA allocation
>>>>>>> + ACR_OTHER, // other allocation
>>>>>>> +};
>>>>>>
>>>>>> Hm, enum != flags.
>>>>>>
>>>>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>>>>
>>>>>> And ACR_CMA would then have to be "1" etc.
>>>>>
>>>>> I have a fixup to change acr_flags_t to acr_mode.
>>>>>
>>>>>>
>>>>>>> +
>>>>>>> /* 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);
>>>>>>> + enum 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..3e2f960e166c 100644
>>>>>>> --- a/include/linux/page-isolation.h
>>>>>>> +++ b/include/linux/page-isolation.h
>>>>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>>>>> }
>>>>>>> #endif
>>>>>>> -#define MEMORY_OFFLINE 0x1
>>>>>>> -#define REPORT_FAILURE 0x2
>>>>>>> +/*
>>>>>>> + * Pageblock isolation modes:
>>>>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>>>>> + * e.g., skip over PageHWPoison() pages and
>>>>>>> + * PageOffline() pages. Unmovable pages will be
>>>>>>> + * reported in this mode.
>>>>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>>>>> + */
>>>>>>> +enum pb_isolate_mode {
>>>>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>> + PB_ISOLATE_MODE_OTHER,
>>>>>>> +};
>>>>>>
>>>>>> It's late on friady, but it looks like we are duplicating things here.
>>>>>>
>>>>>> Let me think about that once my brain is recharged :)
>>>>>
>>>>> Sure. Take your time.
>>>>
>>>> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>>>>
>>>> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>>>>
>>>> Just an idea, not sure ...
>>>
>>> I think so.
>>>
>>> This is the fixup of removing acr_flags_t. It is on top of the original
>>> patch. Take your time. I guess I will send V7 with all fixups next week.
>>
>> Only wondering if we should rename "pb_isolate_mode" to something more abstract, that covers both use cases clearer.
>>
>> Hmmm.
>
> It is specifying the purpose of either alloc_contig_range() or
> test_pages_isolated(), but these two use cases do not have high level
> connection AFAICT.
Hm, then maybe just keep it as is for now, and have the translation from
ACP -> isolatetype.
isolation modes should probably be an enum.
acp might long-term benefit from flags, where we would essentially for
now only have a single flag ("ACP_CMA").
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-06-02 16:25 ` David Hildenbrand
@ 2025-06-02 16:27 ` Zi Yan
2025-06-02 16:29 ` David Hildenbrand
0 siblings, 1 reply; 31+ messages in thread
From: Zi Yan @ 2025-06-02 16:27 UTC (permalink / raw)
To: David Hildenbrand
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 2 Jun 2025, at 12:25, David Hildenbrand wrote:
> On 02.06.25 16:18, Zi Yan wrote:
>> On 30 May 2025, at 16:55, David Hildenbrand wrote:
>>
>>> On 30.05.25 22:46, Zi Yan wrote:
>>>> On 30 May 2025, at 16:08, David Hildenbrand wrote:
>>>>
>>>>> On 30.05.25 21:58, Zi Yan wrote:
>>>>>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>>>>>
>>>>>>> On 30.05.25 18:22, 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
>>>>>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>>>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>>>>>> isolation failures.
>>>>>>>>
>>>>>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>>>>>> enum 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 | 2 +-
>>>>>>>> include/linux/gfp.h | 9 ++++-
>>>>>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>>>>>> include/trace/events/kmem.h | 14 ++++---
>>>>>>>> mm/cma.c | 2 +-
>>>>>>>> mm/memory_hotplug.c | 6 +--
>>>>>>>> mm/page_alloc.c | 27 ++++++-------
>>>>>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>>>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>>>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>>>>>> --- a/drivers/virtio/virtio_mem.c
>>>>>>>> +++ b/drivers/virtio/virtio_mem.c
>>>>>>>> @@ -1243,7 +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,
>>>>>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>>>>>> GFP_KERNEL);
>>>>>>>> if (rc == -ENOMEM)
>>>>>>>> /* whoops, out of memory */
>>>>>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>>>>>> index be160e8d8bcb..51990d571e3e 100644
>>>>>>>> --- a/include/linux/gfp.h
>>>>>>>> +++ b/include/linux/gfp.h
>>>>>>>> @@ -423,9 +423,16 @@ 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
>>>>>>>> +
>>>>>>>> +enum acr_flags_t {
>>>>>>>> + ACR_CMA, // CMA allocation
>>>>>>>> + ACR_OTHER, // other allocation
>>>>>>>> +};
>>>>>>>
>>>>>>> Hm, enum != flags.
>>>>>>>
>>>>>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>>>>>
>>>>>>> And ACR_CMA would then have to be "1" etc.
>>>>>>
>>>>>> I have a fixup to change acr_flags_t to acr_mode.
>>>>>>
>>>>>>>
>>>>>>>> +
>>>>>>>> /* 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);
>>>>>>>> + enum 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..3e2f960e166c 100644
>>>>>>>> --- a/include/linux/page-isolation.h
>>>>>>>> +++ b/include/linux/page-isolation.h
>>>>>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>>>>>> }
>>>>>>>> #endif
>>>>>>>> -#define MEMORY_OFFLINE 0x1
>>>>>>>> -#define REPORT_FAILURE 0x2
>>>>>>>> +/*
>>>>>>>> + * Pageblock isolation modes:
>>>>>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>>>>>> + * e.g., skip over PageHWPoison() pages and
>>>>>>>> + * PageOffline() pages. Unmovable pages will be
>>>>>>>> + * reported in this mode.
>>>>>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>>>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>>>>>> + */
>>>>>>>> +enum pb_isolate_mode {
>>>>>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>>>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>>> + PB_ISOLATE_MODE_OTHER,
>>>>>>>> +};
>>>>>>>
>>>>>>> It's late on friady, but it looks like we are duplicating things here.
>>>>>>>
>>>>>>> Let me think about that once my brain is recharged :)
>>>>>>
>>>>>> Sure. Take your time.
>>>>>
>>>>> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>>>>>
>>>>> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>>>>>
>>>>> Just an idea, not sure ...
>>>>
>>>> I think so.
>>>>
>>>> This is the fixup of removing acr_flags_t. It is on top of the original
>>>> patch. Take your time. I guess I will send V7 with all fixups next week.
>>>
>>> Only wondering if we should rename "pb_isolate_mode" to something more abstract, that covers both use cases clearer.
>>>
>>> Hmmm.
>>
>> It is specifying the purpose of either alloc_contig_range() or
>> test_pages_isolated(), but these two use cases do not have high level
>> connection AFAICT.
>
> Hm, then maybe just keep it as is for now, and have the translation from ACP -> isolatetype.
>
> isolation modes should probably be an enum.
>
> acp might long-term benefit from flags, where we would essentially for now only have a single flag ("ACP_CMA").
OK, let me send a fixup to my V7.
Best Regards,
Yan, Zi
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v6 6/6] mm/page_isolation: remove migratetype parameter from more functions.
2025-06-02 16:27 ` Zi Yan
@ 2025-06-02 16:29 ` David Hildenbrand
0 siblings, 0 replies; 31+ messages in thread
From: David Hildenbrand @ 2025-06-02 16:29 UTC (permalink / raw)
To: Zi Yan
Cc: Johannes Weiner, Vlastimil Babka, linux-mm, Andrew Morton,
Oscar Salvador, Baolin Wang, Kirill A . Shutemov, Mel Gorman,
Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Richard Chang,
linux-kernel
On 02.06.25 18:27, Zi Yan wrote:
> On 2 Jun 2025, at 12:25, David Hildenbrand wrote:
>
>> On 02.06.25 16:18, Zi Yan wrote:
>>> On 30 May 2025, at 16:55, David Hildenbrand wrote:
>>>
>>>> On 30.05.25 22:46, Zi Yan wrote:
>>>>> On 30 May 2025, at 16:08, David Hildenbrand wrote:
>>>>>
>>>>>> On 30.05.25 21:58, Zi Yan wrote:
>>>>>>> On 30 May 2025, at 15:56, David Hildenbrand wrote:
>>>>>>>
>>>>>>>> On 30.05.25 18:22, 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
>>>>>>>>> (PB_ISOLATE_MODE_MEM_OFFLINE, PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>>>> PB_ISOLATE_MODE_OTHER). Remove REPORT_FAILURE and check
>>>>>>>>> MEMORY_OFFLINE instead, since only PB_ISOLATE_MODE_MEM_OFFLINE reports
>>>>>>>>> isolation failures.
>>>>>>>>>
>>>>>>>>> alloc_contig_range() no longer needs migratetype. Replace it with
>>>>>>>>> enum 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 | 2 +-
>>>>>>>>> include/linux/gfp.h | 9 ++++-
>>>>>>>>> include/linux/page-isolation.h | 20 ++++++++--
>>>>>>>>> include/trace/events/kmem.h | 14 ++++---
>>>>>>>>> mm/cma.c | 2 +-
>>>>>>>>> mm/memory_hotplug.c | 6 +--
>>>>>>>>> mm/page_alloc.c | 27 ++++++-------
>>>>>>>>> mm/page_isolation.c | 70 +++++++++++++++-------------------
>>>>>>>>> 8 files changed, 82 insertions(+), 68 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/virtio/virtio_mem.c b/drivers/virtio/virtio_mem.c
>>>>>>>>> index 56d0dbe62163..6bce70b139b2 100644
>>>>>>>>> --- a/drivers/virtio/virtio_mem.c
>>>>>>>>> +++ b/drivers/virtio/virtio_mem.c
>>>>>>>>> @@ -1243,7 +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,
>>>>>>>>> + rc = alloc_contig_range(pfn, pfn + nr_pages, ACR_OTHER,
>>>>>>>>> GFP_KERNEL);
>>>>>>>>> if (rc == -ENOMEM)
>>>>>>>>> /* whoops, out of memory */
>>>>>>>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>>>>>>>> index be160e8d8bcb..51990d571e3e 100644
>>>>>>>>> --- a/include/linux/gfp.h
>>>>>>>>> +++ b/include/linux/gfp.h
>>>>>>>>> @@ -423,9 +423,16 @@ 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
>>>>>>>>> +
>>>>>>>>> +enum acr_flags_t {
>>>>>>>>> + ACR_CMA, // CMA allocation
>>>>>>>>> + ACR_OTHER, // other allocation
>>>>>>>>> +};
>>>>>>>>
>>>>>>>> Hm, enum != flags.
>>>>>>>>
>>>>>>>> If you want to use flags, then just have ACR_CMA. ACR_OTHER is implied if not set.
>>>>>>>>
>>>>>>>> And ACR_CMA would then have to be "1" etc.
>>>>>>>
>>>>>>> I have a fixup to change acr_flags_t to acr_mode.
>>>>>>>
>>>>>>>>
>>>>>>>>> +
>>>>>>>>> /* 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);
>>>>>>>>> + enum 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..3e2f960e166c 100644
>>>>>>>>> --- a/include/linux/page-isolation.h
>>>>>>>>> +++ b/include/linux/page-isolation.h
>>>>>>>>> @@ -38,8 +38,20 @@ static inline void set_pageblock_isolate(struct page *page)
>>>>>>>>> }
>>>>>>>>> #endif
>>>>>>>>> -#define MEMORY_OFFLINE 0x1
>>>>>>>>> -#define REPORT_FAILURE 0x2
>>>>>>>>> +/*
>>>>>>>>> + * Pageblock isolation modes:
>>>>>>>>> + * PB_ISOLATE_MODE_MEM_OFFLINE - isolate to offline (!allocate) memory
>>>>>>>>> + * e.g., skip over PageHWPoison() pages and
>>>>>>>>> + * PageOffline() pages. Unmovable pages will be
>>>>>>>>> + * reported in this mode.
>>>>>>>>> + * PB_ISOLATE_MODE_CMA_ALLOC - isolate for CMA allocations
>>>>>>>>> + * PB_ISOLATE_MODE_OTHER - isolate for other purposes
>>>>>>>>> + */
>>>>>>>>> +enum pb_isolate_mode {
>>>>>>>>> + PB_ISOLATE_MODE_MEM_OFFLINE,
>>>>>>>>> + PB_ISOLATE_MODE_CMA_ALLOC,
>>>>>>>>> + PB_ISOLATE_MODE_OTHER,
>>>>>>>>> +};
>>>>>>>>
>>>>>>>> It's late on friady, but it looks like we are duplicating things here.
>>>>>>>>
>>>>>>>> Let me think about that once my brain is recharged :)
>>>>>>>
>>>>>>> Sure. Take your time.
>>>>>>
>>>>>> Could we abstract both settings and use a single one? Then, we could simply reject if MEM_OFFLINE is passed into alloc_contig_range().
>>>>>>
>>>>>> alloc_contig_pages and page isolation, hmmmm, MEM_OFFLINE is kind-of an allocation. CMA is an allocation.
>>>>>>
>>>>>> Just an idea, not sure ...
>>>>>
>>>>> I think so.
>>>>>
>>>>> This is the fixup of removing acr_flags_t. It is on top of the original
>>>>> patch. Take your time. I guess I will send V7 with all fixups next week.
>>>>
>>>> Only wondering if we should rename "pb_isolate_mode" to something more abstract, that covers both use cases clearer.
>>>>
>>>> Hmmm.
>>>
>>> It is specifying the purpose of either alloc_contig_range() or
>>> test_pages_isolated(), but these two use cases do not have high level
>>> connection AFAICT.
>>
>> Hm, then maybe just keep it as is for now, and have the translation from ACP -> isolatetype.
>>
>> isolation modes should probably be an enum.
>>
>> acp might long-term benefit from flags, where we would essentially for now only have a single flag ("ACP_CMA").
>
> OK, let me send a fixup to my V7.
Oh, I saw that v7 just now, sorry.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 31+ messages in thread