Re: [PATCH 4/7] mm: hugetlb: directly pass order when allocate a hugetlb folio

[jane.chu@oracle.com]

On 8/6/2025 1:05 PM, jane.chu@oracle.com wrote:
> 
> 
> On 8/2/2025 12:31 AM, Kefeng Wang wrote:
>> Use order instead of struct hstate to remove huge_page_order() call
>> from all hugetlb folio allocation.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/hugetlb.c     | 27 +++++++++++++--------------
>>   mm/hugetlb_cma.c |  3 +--
>>   mm/hugetlb_cma.h |  6 +++---
>>   3 files changed, 17 insertions(+), 19 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 436403fb0bed..e174a9269f52 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1473,17 +1473,16 @@ static int hstate_next_node_to_free(struct 
>> hstate *h, nodemask_t *nodes_allowed)
>>   #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>>   #ifdef CONFIG_CONTIG_ALLOC
>> -static struct folio *alloc_gigantic_folio(struct hstate *h, gfp_t 
>> gfp_mask,
>> +static struct folio *alloc_gigantic_folio(int order, gfp_t gfp_mask,
>>           int nid, nodemask_t *nodemask)
>>   {
>>       struct folio *folio;
>> -    int order = huge_page_order(h);
>>       bool retried = false;
>>       if (nid == NUMA_NO_NODE)
>>           nid = numa_mem_id();
>>   retry:
>> -    folio = hugetlb_cma_alloc_folio(h, gfp_mask, nid, nodemask);
>> +    folio = hugetlb_cma_alloc_folio(order, gfp_mask, nid, nodemask);
>>       if (!folio) {
>>           if (hugetlb_cma_exclusive_alloc())
>>               return NULL;
>> @@ -1506,16 +1505,16 @@ static struct folio 
>> *alloc_gigantic_folio(struct hstate *h, gfp_t gfp_mask,
>>   }
>>   #else /* !CONFIG_CONTIG_ALLOC */
>> -static struct folio *alloc_gigantic_folio(struct hstate *h, gfp_t 
>> gfp_mask,
>> -                    int nid, nodemask_t *nodemask)
>> +static struct folio *alloc_gigantic_folio(int order, gfp_t gfp_mask, 
>> int nid,
>> +                      nodemask_t *nodemask)
>>   {
>>       return NULL;
>>   }
>>   #endif /* CONFIG_CONTIG_ALLOC */
>>   #else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE */
>> -static struct folio *alloc_gigantic_folio(struct hstate *h, gfp_t 
>> gfp_mask,
>> -                    int nid, nodemask_t *nodemask)
>> +static struct folio *alloc_gigantic_folio(int order, gfp_t gfp_mask, 
>> int nid,
>> +                      nodemask_t *nodemask)
>>   {
>>       return NULL;
>>   }
>> @@ -1926,11 +1925,9 @@ struct address_space 
>> *hugetlb_folio_mapping_lock_write(struct folio *folio)
>>       return NULL;
>>   }
>> -static struct folio *alloc_buddy_hugetlb_folio(struct hstate *h,
>> -        gfp_t gfp_mask, int nid, nodemask_t *nmask,
>> -        nodemask_t *node_alloc_noretry)
>> +static struct folio *alloc_buddy_hugetlb_folio(int order, gfp_t 
>> gfp_mask,
>> +        int nid, nodemask_t *nmask, nodemask_t *node_alloc_noretry)
>>   {
>> -    int order = huge_page_order(h);
>>       struct folio *folio;
>>       bool alloc_try_hard = true;
>> @@ -1977,11 +1974,13 @@ static struct folio 
>> *only_alloc_fresh_hugetlb_folio(struct hstate *h,
>>           nodemask_t *node_alloc_noretry)
>>   {
>>       struct folio *folio;
>> +    int order = huge_page_order(h);
>> -    if (hstate_is_gigantic(h))
>> -        folio = alloc_gigantic_folio(h, gfp_mask, nid, nmask);
>> +    if (order > MAX_PAGE_ORDER)
>> +        folio = alloc_gigantic_folio(order, gfp_mask, nid, nmask);
> 
> But hstate_is_gigantic() has two definitions depends on whether 
> CONFIG_HUGETLB_PAGE is selected.

Sorry, cross that, don't know what was I thinking.

Reviewed-by: Jane Chu <jane.chu@oracle.com>

thanks!

> 
>>       else
>> -        folio = alloc_buddy_hugetlb_folio(h, gfp_mask, nid, nmask, 
>> node_alloc_noretry);
>> +        folio = alloc_buddy_hugetlb_folio(order, gfp_mask, nid, nmask,
>> +                          node_alloc_noretry);
>>       if (folio)
>>           init_new_hugetlb_folio(h, folio);
>>       return folio;
>> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
>> index f58ef4969e7a..e8e4dc7182d5 100644
>> --- a/mm/hugetlb_cma.c
>> +++ b/mm/hugetlb_cma.c
>> @@ -26,11 +26,10 @@ void hugetlb_cma_free_folio(struct folio *folio)
>>   }
>> -struct folio *hugetlb_cma_alloc_folio(struct hstate *h, gfp_t gfp_mask,
>> +struct folio *hugetlb_cma_alloc_folio(int order, gfp_t gfp_mask,
>>                         int nid, nodemask_t *nodemask)
>>   {
>>       int node;
>> -    int order = huge_page_order(h);
>>       struct folio *folio = NULL;
>>       if (hugetlb_cma[nid])
>> diff --git a/mm/hugetlb_cma.h b/mm/hugetlb_cma.h
>> index f7d7fb9880a2..2c2ec8a7e134 100644
>> --- a/mm/hugetlb_cma.h
>> +++ b/mm/hugetlb_cma.h
>> @@ -4,7 +4,7 @@
>>   #ifdef CONFIG_CMA
>>   void hugetlb_cma_free_folio(struct folio *folio);
>> -struct folio *hugetlb_cma_alloc_folio(struct hstate *h, gfp_t gfp_mask,
>> +struct folio *hugetlb_cma_alloc_folio(int order, gfp_t gfp_mask,
>>                         int nid, nodemask_t *nodemask);
>>   struct huge_bootmem_page *hugetlb_cma_alloc_bootmem(struct hstate 
>> *h, int *nid,
>>                               bool node_exact);
>> @@ -18,8 +18,8 @@ static inline void hugetlb_cma_free_folio(struct 
>> folio *folio)
>>   {
>>   }
>> -static inline struct folio *hugetlb_cma_alloc_folio(struct hstate *h,
>> -        gfp_t gfp_mask, int nid, nodemask_t *nodemask)
>> +static inline struct folio *hugetlb_cma_alloc_folio(int order, gfp_t 
>> gfp_mask,
>> +        int nid, nodemask_t *nodemask)
>>   {
>>       return NULL;
>>   }
> The rest look okay.
> thanks,
> -jane
> 
>