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

[David Hildenbrand <david@redhat.com>]

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