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

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

On 2025/8/7 9:22, jane.chu@oracle.com wrote:
> 
> On 8/2/2025 12:31 AM, Kefeng Wang wrote:
>> The alloc_gigantic_folio() will allocate a folio by alloc_contig_range()
>> with refcount increated and then freeze it, convert to allocate a frozen
>> folio directly to remove the atomic operation about folio refcount and
>> cleanup alloc_gigantic_folio() a bit.
>>
>> Also move folio_alloc_frozen_gigantic(), cma_alloc/free_frozen_folio() 
>> and
>> cma_validate_zones() into mm/internal.h since only hugetlb use it.
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   include/linux/cma.h | 20 --------------------
>>   include/linux/gfp.h | 23 -----------------------
>>   mm/cma.c            |  4 ++--
>>   mm/hugetlb.c        | 43 +++++++++++--------------------------------
>>   mm/hugetlb_cma.c    | 12 ++++++------
>>   mm/hugetlb_cma.h    | 10 +++++-----
>>   mm/internal.h       | 37 +++++++++++++++++++++++++++++++++++++
>>   mm/page_alloc.c     |  8 +++++---
>>   8 files changed, 66 insertions(+), 91 deletions(-)
>>

...

>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index d1d037f97c5f..c542ababb8dc 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6822,6 +6822,8 @@ static int __alloc_contig_verify_gfp_mask(gfp_t 
>> gfp_mask, gfp_t *gfp_cc_mask)
>>    * @gfp_mask:    GFP mask. Node/zone/placement hints are ignored; 
>> only some
>>    *        action and reclaim modifiers are supported. Reclaim modifiers
>>    *        control allocation behavior during compaction/migration/ 
>> reclaim.
>> + *        If gfp_mask contains __GFP_COMP, the refcount of compound page
>> + *        will be not increased.
>>    *
>>    * The PFN range does not have to be pageblock aligned. The PFN 
>> range must
>>    * belong to a single zone.
>> @@ -6955,7 +6957,6 @@ int alloc_contig_range_noprof(unsigned long 
>> start, unsigned long end,
>>           check_new_pages(head, order);
>>           prep_new_page(head, order, gfp_mask, 0);
>> -        set_page_refcounted(head);
>>       } else {
>>           ret = -EINVAL;
>>           WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, 
>> %lu)\n",
>> @@ -7074,10 +7075,11 @@ void free_contig_range(unsigned long pfn, 
>> unsigned long nr_pages)
>>       struct folio *folio = pfn_folio(pfn);
>>       if (folio_test_large(folio)) {
>> -        int expected = folio_nr_pages(folio);
>> +        int order = folio_order(folio);
>> +        int expected = 1 << order;
>>           if (nr_pages == expected)
>> -            folio_put(folio);
>> +            free_frozen_pages(&folio->page, order);
>>           else
>>               WARN(true, "PFN %lu: nr_pages %lu != expected %d\n",
>>                    pfn, nr_pages, expected);
> 
> Is this patch solely for the purpose of saving a few back-and-forth 
> setting refcount calls?
> 
> It seems to me that altering the behavior of alloc_contig_range_noprof() 
> to the contrary of its comtemporaries, such as __alloc_pages_noprof(), 
> alloc_pages_bulk_noprof() in mm/page_alloc.c, might be a source of 
> confusion to unaware callers.  E.g.virtio_mem_fake_offline() calls 
> alloc_contig_range(), for now, without setting __GFP_COMP, but if it 
> does in future, it could be tripped.

OK, we may optimize some driver by adding __GFP_COMP.
> 
> I guess it's helpful to keep the existing convention such that, these 
> alloc_()s from page_alloc.c behave the similar way, in that, head page 
> is returned refcounted.

Will try to keep the original behave for the alloc_contig_* in next 
verison, thanks for the suggestion.

> 
> thanks,
> -jane
> 
>