Re: [PATCH v3 3/6] mm: page_alloc: add alloc_contig_{range_frozen,frozen_pages}()

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

On 2025/10/20 23:21, Kefeng Wang wrote:
> + Matthew
> 
> On 2025/10/20 21:07, David Hildenbrand wrote:
>>
>>>>
>>>>> +void free_contig_range_frozen(unsigned long pfn, unsigned long 
>>>>> nr_pages)
>>>>> +{
>>>>> +    struct folio *folio = pfn_folio(pfn);
>>>>> +
>>>>> +    if (folio_test_large(folio)) {
>>>>> +        int expected = folio_nr_pages(folio);
>>>>> +
>>>>> +        WARN_ON(folio_ref_count(folio));
>>>>> +
>>>>> +        if (nr_pages == expected)
>>>>> +            free_frozen_pages(&folio->page, folio_order(folio));
>>>>> +        else
>>>>> +            WARN(true, "PFN %lu: nr_pages %lu != expected %d\n",
>>>>> +                 pfn, nr_pages, expected);
>>>>> +        return;
>>>>> +    }
>>>>> +
>>>>> +    for (; nr_pages--; pfn++) {
>>>>> +        struct page *page = pfn_to_page(pfn);
>>>>> +
>>>>> +        WARN_ON(page_ref_count(page));
>>>>> +        free_frozen_pages(page, 0);
>>>>> +    }
>>>>
>>>> That's mostly a copy-and-paste of free_contig_range().
>>>>
>>>> I wonder if there is some way to avoid duplicating a lot of
>>>> free_contig_range() here. Hmmm.
>>>>
>>>> Also, the folio stuff in there looks a bit weird I'm afraid.
>>>>
>>>> Can't we just refuse to free compound pages throught this interface and
>>>> free_contig_range() ? IIRC only hugetlb uses it and uses folio_put()
>>>> either way?
>>>>
>>>> Then we can just document that compound allocations are to be freed
>>>> differently.
>>>
>>>
>>> There is a case for cma_free_folio, which calls free_contig_range for
>>> both in cma_release(), but I will try to check whether we could avoid
>>> the folio stuff in free_contig_range().
>>
>>
>> Ah, right, there is hugetlb_cma_free_folio()->cma_free_folio().
>>
>> And we need that, because we have to make sure that CMA stats are 
>> updated properly.
>>
>> All compound page handling in the freeing path is just nasty and not 
>> particularly future-proof regarding memdescs.
>>
>> I wonder if we could just teach alloc_contig to never hand out 
>> compound pages and then let the freeing path similarly assert that 
>> there are no compound pages.
>>
>> Whoever wants a compound page (currently only hugetlb?) can create 
>> that from a frozen range. Before returning the frozen range the 
>> compound page can be dissolved. That way also any memdesc can be 
>> allocated/freed by the caller later.
>>
>> The only nasty thing is the handing of splitting/merging of 
>> set_page_owner/page_table_check_alloc etc. :(
>>
>>
>>
>> As an alternative, we could only allow compound pages for frozen 
>> pages. This way, we'd force any caller to handle the allocation/ 
>> freeing of the memdesc in the future manually.
>>
>> Essentially, only allow GFP_COMPOUND on the frozen interface, which we 
>> would convert hugetlb to.
>>
>> That means that we can simplify free_contig_range() [no need to handle 
>> compound pages]. For free_contig_frozen_range() we would skip refcount 
>> checks on that level and do something like:


I tried to only allocate/free non-compound pages in
alloc_contig_{range,pages}() and free_contig_range(),

The new added alloc_contig_frozen_{range,pages} can allocate compound/
non-compound frozen pages, let's discuss in the new version[1].

[1] 
https://lore.kernel.org/linux-mm/20251023115940.3573158-1-wangkefeng.wang@huawei.com/

>>
>> void free_contig_frozen_range(unsigned long pfn, unsigned long nr_pages)
>> {
>>      struct page *first_page = pfn_to_page(pfn)
>>      const unsigned int order = ilog2(nr_pages);
>>
>>      if (PageHead(first_page)) {
>>          WARN_ON_ONCE(order != compound_order(first_page));
>>          free_frozen_pages(first_page, order);
>>          return;
>>      }
>>
>>      for (; nr_pages--; pfn++)
>>          free_frozen_pages(pfn_to_page(pfn), 0);
>> }
>>
>> CCing Willy, I don't know yet what will be better in the future. But 
>> the folio stuff in there screams for problems.
>>
> 
> Sorry forget to add cc in v3, the full link[1],
> 
> [1] https://lore.kernel.org/linux-mm/20251013133854.2466530-1- 
> wangkefeng.wang@huawei.com/
> 
> 
>