Re: [PATCH] mm/page_alloc: check the correct buddy if it is a starting block

[Wei Yang <richard.weiyang@gmail.com>]

On Fri, Sep 05, 2025 at 11:40:42AM +0200, David Hildenbrand wrote:
>On 04.09.25 04:06, Wei Yang wrote:
>> find_large_buddy() search buddy based on start_pfn, which maybe
>> different from page's pfn, e.g. when page is not pageblock aligned,
>> because prep_move_freepages_block() always align start_pfn to pageblock.
>> 
>> This means when we found a starting block at start_pfn, it may check
>> on the wrong page theoretically.
>> 
>> The good news is the page passed to __move_freepages_block_isolate() has
>> only two possible cases:
>> 
>>    * page is pageblock aligned
>>    * page is __first_valid_page() of this block
>> 
>> So it is safe for the first case, and it won't get a buddy larger than
>> pageblock for the second case.
>> 
>> To eliminate the ambiguity, unify the handling for starting/tail block.
>> 
>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>> Cc: Johannes Weiner <hannes@cmpxchg.org>
>> Cc: Zi Yan <ziy@nvidia.com>
>> Cc: David Hildenbrand <david@redhat.com>
>> Cc: Baolin Wang <baolin.wang@linux.alibaba.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> 
>> ---
>> Tried memory online/offine, which looks good.
>> ---
>>   mm/page_alloc.c | 17 ++++-------------
>>   1 file changed, 4 insertions(+), 13 deletions(-)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 5183a646fa09..453cd361386c 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -2093,6 +2093,7 @@ static bool __move_freepages_block_isolate(struct zone *zone,
>>   	unsigned long start_pfn, pfn;
>>   	int from_mt;
>>   	int to_mt;
>> +	struct page *buddy;
>>   	if (isolate == get_pageblock_isolate(page)) {
>>   		VM_WARN_ONCE(1, "%s a pageblock that is already in that state",
>> @@ -2107,10 +2108,10 @@ static bool __move_freepages_block_isolate(struct zone *zone,
>>   	if (pageblock_order == MAX_PAGE_ORDER)
>>   		goto move;
>> -	/* We're a tail block in a larger buddy */
>>   	pfn = find_large_buddy(start_pfn);
>> -	if (pfn != start_pfn) {
>> -		struct page *buddy = pfn_to_page(pfn);
>> +	buddy = pfn_to_page(pfn);
>
>While at it, can you please rename "pfn" to "buddy_pfn" ?
>

Ok. 

>> +	/* We're a part of a larger buddy */
>> +	if (PageBuddy(buddy) && buddy_order(buddy) > pageblock_order) {
>>   		int order = buddy_order(buddy);
>
>Yes, I think this should work.
>

@Andrew, looks I will send a v2 with updated changelog. Thanks

-- 
Wei Yang
Help you, Help me