Re: [PATCH v2 9/9] mm: hugetlb: allocate frozen pages in alloc_gigantic_folio()

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2025/9/9 9:48, jane.chu@oracle.com wrote:
> 
> On 9/2/2025 5:48 AM, Kefeng Wang wrote:
> [..]
>> -
>>   static inline bool subpool_is_free(struct hugepage_subpool *spool)
>>   {
>>       if (spool->count)
>> @@ -1472,43 +1462,20 @@ static int hstate_next_node_to_free(struct 
>> hstate *h, nodemask_t *nodes_allowed)
>>           nr_nodes--)
>>   #ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>> -#ifdef CONFIG_CONTIG_ALLOC
> 
> Why do you remove the CONTIG_ALLOC config switch?
> If it's not selected on x86, gigantic_pages_init() won't be invoked.

alloc_gigantic_folio() stub is useless, we have all stub function for no
CONTIG_ALLOC（CMA will enabled CONTIG_ALLOC), alloc_gigantic_folio()
will return NULL without CONFIG_CONTIG_ALLOC, right?


> 
>>   static struct folio *alloc_gigantic_folio(int order, gfp_t gfp_mask,
>>           int nid, nodemask_t *nodemask)
>>   {
>>       struct folio *folio;
>> -    bool retried = false;
>> -
>> -retry:
>> -    folio = hugetlb_cma_alloc_folio(order, gfp_mask, nid, nodemask);
>> -    if (!folio) {
>> -        if (hugetlb_cma_exclusive_alloc())
>> -            return NULL;
>> -
>> -        folio = folio_alloc_gigantic(order, gfp_mask, nid, nodemask);
>> -        if (!folio)
>> -            return NULL;
>> -    }
>> -    if (folio_ref_freeze(folio, 1))
>> +    folio = hugetlb_cma_alloc_frozen_folio(order, gfp_mask, nid, 
>> nodemask);
>> +    if (folio)
>>           return folio;
>> -    pr_warn("HugeTLB: unexpected refcount on PFN %lu\n", 
>> folio_pfn(folio));
>> -    hugetlb_free_folio(folio);
>> -    if (!retried) {
>> -        retried = true;
>> -        goto retry;
>> -    }
>> -    return NULL;
>> -}
>> +    if (hugetlb_cma_exclusive_alloc())
>> +        return NULL;
>> -#else /* !CONFIG_CONTIG_ALLOC */
>> -static struct folio *alloc_gigantic_folio(int order, gfp_t gfp_mask, 
>> int nid,
>> -                      nodemask_t *nodemask)
>> -{
>> -    return NULL;
>> +    return folio_alloc_frozen_gigantic(order, gfp_mask, nid, nodemask);
>>   }
>> -#endif /* CONFIG_CONTIG_ALLOC */
>>   #else /* !CONFIG_ARCH_HAS_GIGANTIC_PAGE */
> The rest look okay.
> 
> thanks,
> -jane
> 
>