[PATCH 0/2] mm/pageblock: improve readability of some pageblock handling

[PATCH 0/2] mm/pageblock: improve readability of some pageblock handling

[Wei Yang]

During code reading, found two possible points to improve the readability of
pageblock handling.

Patch 1: isolate bit is standalone and there are dedicated helpers. Instead of
check the bit directly, we could use the helper to do it.

Patch 2: First we could use PB_migratetype_bits to define MIGRATETYPE_MASK.
Second use MIGRATETYPE_MASK to define MIGRATETYPE_AND_ISO_MASK instead of
duplicate the definition.

Wei Yang (2):
  mm/page_alloc: use xxx_pageblock_isolate() for better reading
  mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

 include/linux/pageblock-flags.h | 5 ++---
 mm/page_alloc.c                 | 7 +++----
 2 files changed, 5 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH 1/2] mm/page_alloc: use xxx_pageblock_isolate() for better reading

[Wei Yang]

Since commit e904bce2d9d4 ("mm/page_isolation: make page isolation a
standalone bit"), it provides dedicated helper to handle isolation.

Change to use these helpers to be better reading.

No functional change intended.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: David Hildenbrand <david@redhat.com>
---
 mm/page_alloc.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index baead29b3e67..e534f31a6b39 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -538,8 +538,7 @@ static void set_pageblock_migratetype(struct page *page,
 			"Use set_pageblock_isolate() for pageblock isolation");
 		return;
 	}
-	VM_WARN_ONCE(get_pfnblock_bit(page, page_to_pfn(page),
-				      PB_migrate_isolate),
+	VM_WARN_ONCE(get_pageblock_isolate(page),
 		     "Use clear_pageblock_isolate() to unisolate pageblock");
 	/* MIGRATETYPE_AND_ISO_MASK clears PB_migrate_isolate if it is set */
 #endif
@@ -2058,9 +2057,9 @@ static unsigned long find_large_buddy(unsigned long 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);
+		set_pageblock_isolate(page);
 	else
-		clear_pfnblock_bit(page, page_to_pfn(page), PB_migrate_isolate);
+		clear_pageblock_isolate(page);
 }
 
 /**
-- 
2.34.1

[PATCH 2/2] mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

[Wei Yang]

MIGRATETYPE_MASK is defined to be the mask of possible migratetype.

Define it with PB_migratetype_bits directly would be more clear.

Also, MIGRATETYPE_AND_ISO_MASK is MIGRATETYPE_MASK add isolation bit.
Use MIGRATETYPE_MASK in the definition of MIGRATETYPE_AND_ISO_MASK looks
cleaner.

No functional change intended.

Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
Cc: Zi Yan <ziy@nvidia.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: David Hildenbrand <david@redhat.com>
---
 include/linux/pageblock-flags.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index 6a44be0f39f4..1489c062a5a7 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -37,11 +37,10 @@ enum pageblock_bits {
 
 #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
 
-#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
+#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
 
 #ifdef CONFIG_MEMORY_ISOLATION
-#define MIGRATETYPE_AND_ISO_MASK \
-	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
+#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
 #else
 #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
 #endif
-- 
2.34.1

Re: [PATCH 1/2] mm/page_alloc: use xxx_pageblock_isolate() for better reading

[Zi Yan]

On 26 Aug 2025, at 5:31, Wei Yang wrote:

> Since commit e904bce2d9d4 ("mm/page_isolation: make page isolation a
> standalone bit"), it provides dedicated helper to handle isolation.
>
> Change to use these helpers to be better reading.
>
> No functional change intended.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: David Hildenbrand <david@redhat.com>
> ---
>  mm/page_alloc.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>

--
Best Regards,
Yan, Zi

Re: [PATCH 1/2] mm/page_alloc: use xxx_pageblock_isolate() for better reading

[David Hildenbrand]

On 26.08.25 11:31, Wei Yang wrote:
> Since commit e904bce2d9d4 ("mm/page_isolation: make page isolation a
> standalone bit"), it provides dedicated helper to handle isolation.
> 
> Change to use these helpers to be better reading.
> 
> No functional change intended.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: David Hildenbrand <david@redhat.com>
> ---

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers

David / dhildenb

Re: [PATCH 2/2] mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

[David Hildenbrand]

On 26.08.25 11:31, Wei Yang wrote:
> MIGRATETYPE_MASK is defined to be the mask of possible migratetype.
> 
> Define it with PB_migratetype_bits directly would be more clear.
> 
> Also, MIGRATETYPE_AND_ISO_MASK is MIGRATETYPE_MASK add isolation bit.
> Use MIGRATETYPE_MASK in the definition of MIGRATETYPE_AND_ISO_MASK looks
> cleaner.
> 
> No functional change intended.
> 
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: David Hildenbrand <david@redhat.com>
> ---
>   include/linux/pageblock-flags.h | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index 6a44be0f39f4..1489c062a5a7 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -37,11 +37,10 @@ enum pageblock_bits {
>   
>   #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
>   
> -#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
>   
>   #ifdef CONFIG_MEMORY_ISOLATION
> -#define MIGRATETYPE_AND_ISO_MASK \
> -	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
> +#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
>   #else
>   #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
>   #endif


Hm, I wonder if we can get rid of PB_migratetype_bits

diff --git a/include/linux/pageblock-flags.h 
b/include/linux/pageblock-flags.h
index 6a44be0f39f45..70c988dbdddc8 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -13,12 +13,11 @@

  #include <linux/types.h>

-#define PB_migratetype_bits 3
  /* Bit indices that affect a whole block of pages */
  enum pageblock_bits {
-       PB_migrate,
-       PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
-                       /* 3 bits required for migrate types */
+       PB_migrate_0,
+       PB_migrate_1,
+       PB_migrate_2,
         PB_compact_skip,/* If set the block is skipped by compaction */

  #ifdef CONFIG_MEMORY_ISOLATION

And then just work with the bits. No magical computations.

-- 
Cheers

David / dhildenb

Re: [PATCH 2/2] mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

[Zi Yan]

On 26 Aug 2025, at 5:31, Wei Yang wrote:

> MIGRATETYPE_MASK is defined to be the mask of possible migratetype.
>
> Define it with PB_migratetype_bits directly would be more clear.
>
> Also, MIGRATETYPE_AND_ISO_MASK is MIGRATETYPE_MASK add isolation bit.
> Use MIGRATETYPE_MASK in the definition of MIGRATETYPE_AND_ISO_MASK looks
> cleaner.
>
> No functional change intended.
>
> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
> Cc: Zi Yan <ziy@nvidia.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: David Hildenbrand <david@redhat.com>
> ---
>  include/linux/pageblock-flags.h | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index 6a44be0f39f4..1489c062a5a7 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -37,11 +37,10 @@ enum pageblock_bits {
>
>  #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
>
> -#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)

I am not sure about this one, even if PB_migrate_end + 1 == PB_migratetype_bits.
PB_migrate_end represents a bit index, where PB_migratetype_bits means
the number of the bits.

David's suggestion in another email might be better.

>
>  #ifdef CONFIG_MEMORY_ISOLATION
> -#define MIGRATETYPE_AND_ISO_MASK \
> -	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
> +#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
>  #else
>  #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
>  #endif

This one looks good to me.

--
Best Regards,
Yan, Zi

Re: [PATCH 2/2] mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

[Wei Yang]

On Tue, Aug 26, 2025 at 03:34:13PM +0200, David Hildenbrand wrote:
>On 26.08.25 11:31, Wei Yang wrote:
>> MIGRATETYPE_MASK is defined to be the mask of possible migratetype.
>> 
>> Define it with PB_migratetype_bits directly would be more clear.
>> 
>> Also, MIGRATETYPE_AND_ISO_MASK is MIGRATETYPE_MASK add isolation bit.
>> Use MIGRATETYPE_MASK in the definition of MIGRATETYPE_AND_ISO_MASK looks
>> cleaner.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: David Hildenbrand <david@redhat.com>
>> ---
>>   include/linux/pageblock-flags.h | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>> 
>> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
>> index 6a44be0f39f4..1489c062a5a7 100644
>> --- a/include/linux/pageblock-flags.h
>> +++ b/include/linux/pageblock-flags.h
>> @@ -37,11 +37,10 @@ enum pageblock_bits {
>>   #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
>> -#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
>> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
>>   #ifdef CONFIG_MEMORY_ISOLATION
>> -#define MIGRATETYPE_AND_ISO_MASK \
>> -	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
>> +#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
>>   #else
>>   #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
>>   #endif
>
>
>Hm, I wonder if we can get rid of PB_migratetype_bits
>
>diff --git a/include/linux/pageblock-flags.h
>b/include/linux/pageblock-flags.h
>index 6a44be0f39f45..70c988dbdddc8 100644
>--- a/include/linux/pageblock-flags.h
>+++ b/include/linux/pageblock-flags.h
>@@ -13,12 +13,11 @@
>
> #include <linux/types.h>
>
>-#define PB_migratetype_bits 3
> /* Bit indices that affect a whole block of pages */
> enum pageblock_bits {
>-       PB_migrate,
>-       PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
>-                       /* 3 bits required for migrate types */
>+       PB_migrate_0,
>+       PB_migrate_1,
>+       PB_migrate_2,
>        PB_compact_skip,/* If set the block is skipped by compaction */
>
> #ifdef CONFIG_MEMORY_ISOLATION
>
>And then just work with the bits. No magical computations.

Something like this?

diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index 6a44be0f39f4..a09b79a1fb7d 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -13,12 +13,11 @@
 
 #include <linux/types.h>
 
-#define PB_migratetype_bits 3
 /* Bit indices that affect a whole block of pages */
 enum pageblock_bits {
-	PB_migrate,
-	PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
-			/* 3 bits required for migrate types */
+	PB_migrate_0,
+	PB_migrate_1,
+	PB_migrate_2,
 	PB_compact_skip,/* If set the block is skipped by compaction */
 
 #ifdef CONFIG_MEMORY_ISOLATION
@@ -37,11 +36,10 @@ enum pageblock_bits {
 
 #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
 
-#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
+#define MIGRATETYPE_MASK (BIT(PB_migrate_2 + 1) - 1)
 
 #ifdef CONFIG_MEMORY_ISOLATION
-#define MIGRATETYPE_AND_ISO_MASK \
-	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
+#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
 #else
 #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
 #endif
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e534f31a6b39..7a67250ccfd5 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -370,7 +370,7 @@ get_pfnblock_bitmap_bitidx(const struct page *page, unsigned long pfn,
 #else
 	BUILD_BUG_ON(NR_PAGEBLOCK_BITS != 4);
 #endif
-	BUILD_BUG_ON(__MIGRATE_TYPE_END >= (1 << PB_migratetype_bits));
+	BUILD_BUG_ON(__MIGRATE_TYPE_END >= BIT(PB_migrate_2 + 1));
 	VM_BUG_ON_PAGE(!zone_spans_pfn(page_zone(page), pfn), page);
 
 	bitmap = get_pageblock_bitmap(page, pfn);

-- 
Wei Yang
Help you, Help me

Re: [PATCH 2/2] mm/pageblock-flags: simplify MIGRATETYPE_MASK definition

[David Hildenbrand]

On 26.08.25 17:19, Wei Yang wrote:
> On Tue, Aug 26, 2025 at 03:34:13PM +0200, David Hildenbrand wrote:
>> On 26.08.25 11:31, Wei Yang wrote:
>>> MIGRATETYPE_MASK is defined to be the mask of possible migratetype.
>>>
>>> Define it with PB_migratetype_bits directly would be more clear.
>>>
>>> Also, MIGRATETYPE_AND_ISO_MASK is MIGRATETYPE_MASK add isolation bit.
>>> Use MIGRATETYPE_MASK in the definition of MIGRATETYPE_AND_ISO_MASK looks
>>> cleaner.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Cc: Zi Yan <ziy@nvidia.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: David Hildenbrand <david@redhat.com>
>>> ---
>>>    include/linux/pageblock-flags.h | 5 ++---
>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
>>> index 6a44be0f39f4..1489c062a5a7 100644
>>> --- a/include/linux/pageblock-flags.h
>>> +++ b/include/linux/pageblock-flags.h
>>> @@ -37,11 +37,10 @@ enum pageblock_bits {
>>>    #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
>>> -#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
>>> +#define MIGRATETYPE_MASK (BIT(PB_migratetype_bits) - 1)
>>>    #ifdef CONFIG_MEMORY_ISOLATION
>>> -#define MIGRATETYPE_AND_ISO_MASK \
>>> -	(((1UL << (PB_migrate_end + 1)) - 1) | BIT(PB_migrate_isolate))
>>> +#define MIGRATETYPE_AND_ISO_MASK (MIGRATETYPE_MASK | BIT(PB_migrate_isolate))
>>>    #else
>>>    #define MIGRATETYPE_AND_ISO_MASK MIGRATETYPE_MASK
>>>    #endif
>>
>>
>> Hm, I wonder if we can get rid of PB_migratetype_bits
>>
>> diff --git a/include/linux/pageblock-flags.h
>> b/include/linux/pageblock-flags.h
>> index 6a44be0f39f45..70c988dbdddc8 100644
>> --- a/include/linux/pageblock-flags.h
>> +++ b/include/linux/pageblock-flags.h
>> @@ -13,12 +13,11 @@
>>
>> #include <linux/types.h>
>>
>> -#define PB_migratetype_bits 3
>> /* Bit indices that affect a whole block of pages */
>> enum pageblock_bits {
>> -       PB_migrate,
>> -       PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
>> -                       /* 3 bits required for migrate types */
>> +       PB_migrate_0,
>> +       PB_migrate_1,
>> +       PB_migrate_2,
>>         PB_compact_skip,/* If set the block is skipped by compaction */
>>
>> #ifdef CONFIG_MEMORY_ISOLATION
>>
>> And then just work with the bits. No magical computations.
> 
> Something like this?
> 
> diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
> index 6a44be0f39f4..a09b79a1fb7d 100644
> --- a/include/linux/pageblock-flags.h
> +++ b/include/linux/pageblock-flags.h
> @@ -13,12 +13,11 @@
>   
>   #include <linux/types.h>
>   
> -#define PB_migratetype_bits 3
>   /* Bit indices that affect a whole block of pages */
>   enum pageblock_bits {
> -	PB_migrate,
> -	PB_migrate_end = PB_migrate + PB_migratetype_bits - 1,
> -			/* 3 bits required for migrate types */
> +	PB_migrate_0,
> +	PB_migrate_1,
> +	PB_migrate_2,
>   	PB_compact_skip,/* If set the block is skipped by compaction */
>   
>   #ifdef CONFIG_MEMORY_ISOLATION
> @@ -37,11 +36,10 @@ enum pageblock_bits {
>   
>   #define NR_PAGEBLOCK_BITS (roundup_pow_of_two(__NR_PAGEBLOCK_BITS))
>   
> -#define MIGRATETYPE_MASK ((1UL << (PB_migrate_end + 1)) - 1)
> +#define MIGRATETYPE_MASK (BIT(PB_migrate_2 + 1) - 1)

I'd just do PB_migrate_0|PB_migrate_1|PB_migrate_2 and have code that
is easier to parse.

-- 
Cheers

David / dhildenb