在 2025/2/12 13:40, Shivank Garg 写道:
> On 2/12/2025 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@xxxxxxxxxx>
>> ---
>> 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)
>
> This folio order calculation is only valid for !large folios.
> When it's a single page (not a large folio), the memory is interpreted as struct page.
>
> struct folio {
> ...
> union {
> struct {
> unsigned long _flags_1;
> unsigned long _head_1;
> /* public: */
> atomic_t _large_mapcount;
> atomic_t _entire_mapcount;
> atomic_t _nr_pages_mapped;
> atomic_t _pincount;
> #ifdef CONFIG_64BIT
> unsigned int _folio_nr_pages;
> #endif
> /* private: the union with struct page is transitional */
> };
> struct page __page_1;
> };
> ...
> }
>
> I feel this to be risky, considering someone may directly use FOLIO_ORDER() macro
> without folio_test_large() check.
>
> Correct macro should look like:
>
> #define FOLIO_ORDER(folio) (folio_test_large(folio) ? ((folio)->_flags_1 & 0xff) : 0)
>
Yes, this is safer.
At present, the positions using FOLIO-ORDER have been checked using folio_test_1arge or
test-bit (PG_cead,&folio ->flags), and these positions may need to be simplified.
>
> Thanks,
> Shivank
>> +
>> /*
>> * 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
>> }
>>
>
