Re: [PATCH v2 6/9] mm: page_alloc: add alloc_contig_frozen_pages()

Kefeng Wang
2025-09-10T02:05:58Z

On 2025/9/10 3:08, Zi Yan wrote:
> On 9 Sep 2025, at 14:55, Matthew Wilcox wrote:
>
>> On Tue, Sep 02, 2025 at 08:48:17PM +0800, Kefeng Wang wrote:
>>> Introduce a ACR_FLAGS_FROZEN flags to indicate that we want to
>>> allocate a frozen compound pages by alloc_contig_range(), also
>>> provide alloc_contig_frozen_pages() to allocate pages without
>>> incrementing their refcount, which may be beneficial to some
>>> users (eg hugetlb).
>>
>> No, this is not the way to do it. We're moving towards a future where
>> pages don't have refcounts (their owning structs eg folios may have a
>> refcount, but struct page does not have a refcount). So work towards
>> that; if you need to keep compatibility with some users, then by all
>> means keep alloc_contig_range_noprof() as a wrapper that calls
>> alloc_contig_range_frozen_noprof() and then calls set_page_refcounted()
>> if alloc_contig_range_frozen_noprof() does not return NULL. But I'd
>> be looking to see if alloc_contig_range_noprof() can always return a
>> non-refcounted page.
>

Yes, Zi has been comment this shown below, alloc_contig_pages_noprof()
is called by may driver(directly or indirectly), I originally only hoped
to minimize the changes, if we follow your plan, we can do a bit more,
let me try the new way. Also I will separate this patchset to let the
cleanup patch could be merged first.

> Something like:
>
> struct page *alloc_contig_pages_noprof(unsigned long nr_pages, gfp_t gfp_mask,
> int nid, nodemask_t *nodemask)
> {
> struct page *page = __alloc_frozen_contig_pages_noprof(...);
>
> // just added page == NULL check
> if (!page)
> return NULL;
>
> if (gfp_mask & __GFP_COMP)
> set_page_refcounted(page);
> else {
> unsigned long i;
>
> for (i = 0; i < nr_pages; i++)
> set_page_refcounted(page + i);
> }
> return page;
> }
>
> as I suggested in another email[1] (of course I forgot to check page == NULL).
>
> And other TODOs:
>
> 1. remove set_page_refcounted() in split_free_pages();
> 2. a new split_frozen_page() to do split_page() work without
> set_page_refcounted() and split_page() can just call it;
> 3. a new free_frozen_contig_range() and free_contig_range()
> calls it.
>

Thanks Matthew/Zi.

>
> [1] https://lore.kernel.org/linux-mm/298B40E0-8EA5-4667-86EF-22F88B832839@nvidia.com/
>
> Best Regards,
> Yan, Zi