Re: [PATCH v2 1/8] mm: page_alloc: optimize pfn_range_valid_contig()

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

On 2025/9/18 23:49, Zi Yan wrote:
> On 18 Sep 2025, at 9:19, Kefeng Wang wrote:
> 
>> The alloc_contig_pages() spends a lot of time in pfn_range_valid_contig(),
>> we could check whether the page in this pfn range could be allocated
>> before alloc_contig_range(), if the page can't be migrated, no further
>> action is required, and also skip some unnecessary iterations for
>> compound pages such as THP and non-compound high order buddy, which
>> save times a lot too. The check is racy, but the only danger is skipping
>> too much.
>>
>> A simple test on machine with 116G free memory, allocate 120 * 1G
>> HugeTLB folios(107 successfully returned),
>>
>>    time echo 120 > /sys/kernel/mm/hugepages/hugepages-1048576kB/nr_hugepages
>>
>> Before: 0m2.124s
>> After:  0m0.602s
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>   mm/page_alloc.c | 20 ++++++++++++++++++--
>>   1 file changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 478beaf95f84..5b7d705e9710 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -7012,6 +7012,7 @@ static bool pfn_range_valid_contig(struct zone *z, unsigned long start_pfn,
>>   {
>>   	unsigned long i, end_pfn = start_pfn + nr_pages;
>>   	struct page *page;
>> +	struct folio *folio;
>>
>>   	for (i = start_pfn; i < end_pfn; i++) {
>>   		page = pfn_to_online_page(i);
>> @@ -7021,11 +7022,26 @@ static bool pfn_range_valid_contig(struct zone *z, unsigned long start_pfn,
>>   		if (page_zone(page) != z)
>>   			return false;
>>
>> -		if (PageReserved(page))
>> +		folio = page_folio(page);
>> +		if (folio_test_reserved(folio))
>>   			return false;
>>
>> -		if (PageHuge(page))
>> +		if (folio_test_hugetlb(folio))
>>   			return false;
>> +
>> +		/* The following type of folios aren't migrated */
> 
> s/aren’t/cannot be/

ACK.

> 
>> +		if (folio_test_pgtable(folio) | folio_test_stack(folio))
>> +			return false;

should be "||", will fix.

> 
> Maybe worth explicitly stating these two types of pages in the commit log.
> 
OK, will update.

>> +
>> +		/*
>> +		 * For compound pages such as THP and non-compound high
>> +		 * order buddy pages, save potentially a lot of iterations
>> +		 * if we can skip them at once.
>> +		 */
>> +		if (PageCompound(page))
>> +			i += (1UL << compound_order(page)) - 1;
> 
> Just a note here, if page is tail, this just move i to the next page
> instead of next folio.

As no reference held，it is not too precise but we optimize for most
scenarios.

> 
>> +		else if (PageBuddy(page))
>> +			i += (1UL << buddy_order(page)) - 1;
>>   	}
>>   	return true;
>>   }
>> -- 
>> 2.27.0
> 
> Otherwise, LGTM. Reviewed-by: Zi Yan <ziy@nvidia.com>
> 
> 

Thanks.

> Best Regards,
> Yan, Zi
>