Re: [PATCH 2/2] mm/mm.h: Write folio->_flags_1 & 0xff as a macro definition

[Dev Jain <dev.jain@arm.com>]

On 12/02/25 8:28 am, Liu Ye wrote:
> There are multiple locations in mm.h where (folio->_flags_1 & 0xff) is
> used. Write it as a macro definition to improve the readability and
> maintainability of the code.
> 
> Signed-off-by: Liu Ye <liuye@kylinos.cn>
> ---
>   include/linux/mm.h | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7b1068ddcbb7..750e75f45557 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1098,6 +1098,8 @@ int vma_is_stack_for_current(struct vm_area_struct *vma);
>   struct mmu_gather;
>   struct inode;
>   
> +#define FOLIO_ORDER(folio) ((folio)->_flags_1 & 0xff)
> +
>   /*
>    * compound_order() can be called without holding a reference, which means
>    * that niceties like page_folio() don't work.  These callers should be
> @@ -1111,7 +1113,7 @@ static inline unsigned int compound_order(struct page *page)
>   
>   	if (!test_bit(PG_head, &folio->flags))
>   		return 0;
> -	return folio->_flags_1 & 0xff;
> +	return FOLIO_ORDER(folio);
>   }
>   
>   /**
> @@ -1127,7 +1129,7 @@ static inline unsigned int folio_order(const struct folio *folio)
>   {
>   	if (!folio_test_large(folio))
>   		return 0;
> -	return folio->_flags_1 & 0xff;
> +	return FOLIO_ORDER(folio);
>   }
>   
>   #include <linux/huge_mm.h>
> @@ -2061,7 +2063,7 @@ static inline long folio_nr_pages(const struct folio *folio)
>   #ifdef CONFIG_64BIT
>   	return folio->_folio_nr_pages;
>   #else
> -	return 1L << (folio->_flags_1 & 0xff);
> +	return 1L << FOLIO_ORDER(folio);
>   #endif
>   }
>   
> @@ -2086,7 +2088,7 @@ static inline unsigned long compound_nr(struct page *page)
>   #ifdef CONFIG_64BIT
>   	return folio->_folio_nr_pages;
>   #else
> -	return 1L << (folio->_flags_1 & 0xff);
> +	return 1L << FOLIO_ORDER(folio);
>   #endif
>   }
>   

Personally I do not think this is improving readability. You are 
introducing one more macro for people to decipher instead of directly 
seeing folio->_flags_1 & 0xff. This is similar to whether to write
if (x) => do_stuff(), or if (x != 0) => do_stuff(). The former is more 
"readable" by convention but the latter makes it easier and obvious to 
understand.