No functional change is intended.

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

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

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

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

<snip>

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

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

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

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

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

Same here.

>  }

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

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

Sure.

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

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

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

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

Ack.

Best Regards,
Yan, Zi