Re: [PATCH] mm/contig_alloc: fix alloc_contig_range when __GFP_COMP and order < MAX_ORDER

[Zi Yan <ziy@nvidia.com>]

Hi Jinjiang,

On 17 Apr 2025, at 22:59, Andrew Morton wrote:

> On Wed, 12 Mar 2025 16:47:05 +0800 Jinjiang Tu <tujinjiang@huawei.com> wrote:
>
>> When calling alloc_contig_range() with __GFP_COMP and the order of
>> requested pfn range is pageblock_order, less than MAX_ORDER, I triggered
>> WARNING as follows:
>>
>>  PFN range: requested [2150105088, 2150105600), allocated [2150105088, 2150106112)
>>  WARNING: CPU: 3 PID: 580 at mm/page_alloc.c:6877 alloc_contig_range+0x280/0x340

Basically, you are using alloc_contig_range() to allocate a compound page
that can be allocated from buddy allocator, since order is < MAX_ORDER.
What is the use case? Why is alloc_contig_range() used?

>>
>> alloc_contig_range() marks pageblocks of the requested pfn range to be
>> isolated, migrate these pages if they are in use and will be freed to
>> MIGRATE_ISOLATED freelist.
>>
>> Suppose two alloc_contig_range() calls at the same time and the requested
>> pfn range are [0x80280000, 0x80280200) and [0x80280200, 0x80280400)
>> respectively. Suppose the two memory range are in use, then
>> alloc_contig_range() will migrate and free these pages to MIGRATE_ISOLATED
>> freelist. __free_one_page() will merge MIGRATE_ISOLATE buddy to larger
>> buddy, resulting in a MAX_ORDER buddy. Finally, find_large_buddy() in
>> alloc_contig_range() returns a MAX_ORDER buddy and results in WARNING.
>>
>> To fix it, call free_contig_range() to free the excess pfn range.
>
> This has been in mm-hotfixes for a month without issue.  Is there any
> reviewer interest?
>
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -6528,7 +6528,8 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>>  		goto done;
>>  	}
>>
>> -	if (!(gfp_mask & __GFP_COMP)) {
>> +	if (!(gfp_mask & __GFP_COMP) ||
>> +		(is_power_of_2(end - start) && ilog2(end - start) < MAX_PAGE_ORDER)) {
>>  		split_free_pages(cc.freepages, gfp_mask);

This does not look right to me. When a compound page is requested,
alloc_contig_range() should give a compound page, but split_free_pages()
will make the free page as a list of contiguous order-0 pages.

I do not think we should keep this patch.

Jinjiang, let me know if I miss anything.

>>
>>  		/* Free head and tail (if any) */
>> @@ -6536,7 +6537,15 @@ int alloc_contig_range_noprof(unsigned long start, unsigned long end,
>>  			free_contig_range(outer_start, start - outer_start);
>>  		if (end != outer_end)
>>  			free_contig_range(end, outer_end - end);
>> -	} else if (start == outer_start && end == outer_end && is_power_of_2(end - start)) {
>> +
>> +		outer_start = start;
>> +		outer_end = end;
>> +
>> +		if (!(gfp_mask & __GFP_COMP))
>> +			goto done;
>> +	}
>> +
>> +	if (start == outer_start && end == outer_end && is_power_of_2(end - start)) {
>>  		struct page *head = pfn_to_page(start);
>>  		int order = ilog2(end - start);
>>


Best Regards,
Yan, Zi