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

[David Hildenbrand <david@redhat.com>]

On 12.09.25 08:57, Kefeng Wang wrote:
> 
> 
> On 2025/9/12 2:56, David Hildenbrand wrote:
>> On 11.09.25 11:11, Kefeng Wang wrote:
>>>
>>>
>>> On 2025/9/11 16:25, David Hildenbrand wrote:
>>>> On 11.09.25 08:56, Kefeng Wang wrote:
>>>>> The alloc_gigantic_folio() allocates 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,
>>>>> also
>>>>> saving atomic operation during __update_and_free_hugetlb_folio too.
>>>>>
>>>>> Rename some functions to make them more self-explanatory,
>>>>>
>>>>>      folio_alloc_gigantic            -> folio_alloc_frozen_gigantic
>>>>>      cma_{alloc,free}_folio          -> cma_{alloc,free}_frozen_folio
>>>>>      hugetlb_cma_{alloc,free}_folio  -> hugetlb_cma_{alloc,free}
>>>>> _frozen_folio
>>>>
>>>> Can we just get rid of folio_alloc_frozen_gigantic?
>>>>
>>>
>>> OK, we could kill it.
>>>
>>>> Further, can we just get rid of cma_{alloc,free}_frozen_folio() as well
>>>> and just let hugetlb use alloc_contig_range_frozen() etc?
>>>
>>> HugeTLB can allocate folio by alloc_contig_frozen_pages() directly, but
>>> it could allocate from hugetlb_cma, cma_alloc_folio() need change some
>>> cma metadata, so we need to keep it.
>>
>> Hm. Assuming we just have cma_alloc_frozen() -- again, probably what
>> cma_alloc() would look like in the future, hugetlb can just construct a
>> folio out of that.
> 
> I get your point，firstly, we could convert to use cma_alloc_frozen()
> instead of cma_alloc_folio() in hugetlb_cma_alloc_folio().
> 
>>
>> Maybe we just want a helper to create a folio out of a given page range?
>>
>> And that page range is either obtained through cma_alloc_frozen() or
>> alloc_contig_frozen_pages().
>>
>> Just a thought, keeping in mind that these things should probably just
>> work with frozen pages and let allcoating of a memdesc etc. be taken
>> care of someone else.
>>
>> I'd be happy if we can remove the GFP_COMPOUND parameter from
>> alloc_contig*.
> 
> But not sure about this part,  GFP_COMPOUND for alloc_contig* is
> introduced by commit e98337d11bbd "mm/contig_alloc: support __GFP_COMP",
> if we still allocate a range of order-0 pages and create a folio
> outside, it will slow the large folio allocation.

Assuming we leave the refcount untouched (frozen), I guess what's left is

a) Calling post_alloc_hook() on each free buddy chunk we isolated
b) Splitting all pages to order 0

Splitting is updating the page owner + alloc tag + memcg, and currently 
still updating the refcount.


I would assume that most of the overhead came from the atomics when 
updating the refcount in split_page, which we would optimize out.

     Perf profile before:
       Alloc
         - 99.99% alloc_pool_huge_folio
            - __alloc_fresh_hugetlb_folio
               - 83.23% alloc_contig_pages_noprof
                  - 47.46% alloc_contig_range_noprof
                     - 20.96% isolate_freepages_range
                          16.10% split_page
                     - 14.10% start_isolate_page_range
                     - 12.02% undo_isolate_page_range

Would be interesting trying to see how much overhead would remain when 
just dealing

OTOH, maybe we can leave GFP_COMPOUND support in but make the function
more generic, not limited to folios (I suspect many users will not want 
folios, except hugetlb).

Maybe just a

struct page * cma_alloc_compound(struct cma *cma, unsigned int order, 
unsigned int align, bool no_warn);

That would allocate a frozen compoud page starting today already.

-- 
Cheers

David / dhildenb