linux-kernel - Re: [PATCH 2/2] mm/mm.h: Write folio->_flags

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <52fcd6b2-bbe1-4de7-85d1-1e5968f87e0d@kylinos.cn>
Date: Wed, 12 Feb 2025 15:07:07 +0800
From: liuye <liuye@...inos.cn>
To: Dev Jain <dev.jain@....com>, brauner@...nel.org, dhowells@...hat.com,
 akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH 2/2] mm/mm.h: Write folio->_flags_1 & 0xff as a macro
 definition



在 2025/2/12 13:12, Dev Jain 写道:
> 
> 
> 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@...inos.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.
> 
Or simply for maintenance purposes, if the meaning of a bit changes, only the macro definition needs to be modified.

Thanks，
Liu Ye