Re: [PATCH 1/2] mm/compaction: check the range to pageblock_pfn_to_page() is within the zone first

[David Hildenbrand <david@redhat.com>]

On 09.10.25 04:08, Wei Yang wrote:
> On Wed, Oct 08, 2025 at 12:14:25PM +0200, David Hildenbrand wrote:
>> On 02.10.25 05:31, Wei Yang wrote:
>>> Function pageblock_pfn_to_page() is introduced by commit 7d49d8868336
>>> ("mm, compaction: reduce zone checking frequency in the migration scanner"),
>>> where there is no requirement on start_pfn/end_pfn except they are in
>>> the same pageblock.
>>>
>>> So at that time, pageblock_pfn_to_page() would be passed with pfn
>>> without compared with zone boundary.
>>>
>>> But after commit 7cf91a98e607 ("mm/compaction: speed up
>>> pageblock_pfn_to_page() when zone is contiguous"), pageblock_pfn_to_page()
>>> would think the range is valid and in the same zone if zone->contiguous,
>>> even the range doesn't belong to this zone.
>>>
>>> For example, in fast_isolate_freepages(), min_pfn is assigned to
>>> pageblock_start_pfn() and passed to pageblock_pfn_to_page() without
>>> checking with zone_start_pfn. And mostly, the end_pfn is not checked
>>> with zone_end_pfn() before using.
>>>
>>> To make this function robust, check the range is within the zone first.
>>>
>>> Fixes: 7cf91a98e607 ("mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous")
>>> Signed-off-by: Wei Yang <richard.weiyang@gmail.com>
>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>>> ---
>>>    mm/internal.h | 3 +++
>>>    1 file changed, 3 insertions(+)
>>>
>>> diff --git a/mm/internal.h b/mm/internal.h
>>> index 38607b2821d9..8e1a3819c9f1 100644
>>> --- a/mm/internal.h
>>> +++ b/mm/internal.h
>>> @@ -724,6 +724,9 @@ extern struct page *__pageblock_pfn_to_page(unsigned long start_pfn,
>>>    static inline struct page *pageblock_pfn_to_page(unsigned long start_pfn,
>>>    				unsigned long end_pfn, struct zone *zone)
>>>    {
>>> +	if (start_pfn < zone->zone_start_pfn || end_pfn > zone_end_pfn(zone))
>>> +		return NULL;
>>> +
>>>    	if (zone->contiguous)
>>>    		return pfn_to_page(start_pfn);
>>
>> fast_isolate_around() adjusts the range to the zone boundaries before calling
>> fast_isolate_around().
>>
> 
> Correct.
> 
>> isolate_migratepages_range(), isolate_freepages_range(), isolate_freepages()
>> and isolate_migratepages similarly all seem to take the zone range into
>> account as well.
> 
> Maybe I missed something.
> 
> isolate_freepages() takes care of the end_pfn, but I don't see it adjust
> start_pfn. Or you mean low_pfn is already adjusted?
> 
> isolate_freepages_range() / isolate_migratepages_range() just adjust start_pfn.
> 
> isolate_migratepages() seems not adjust start_pfn nor end_pfn?
> 
> Maybe I misunderstand you, would you mind giving more detail?
> 
>>
>> Only fast_isolate_freepages is a bit more tricky. It definitely takes the end
>> into account. The start comes from cc->free_pfn but IIRC it's always above
>> the zone_start_pfn.
>>

I don't recall all the details, but at least regarding ZONE_MOVABLE we 
make sure that the start is aligned to MAX_ORDER_NR_PAGES and that the 
size is aligned to MAX_ORDER_NR_PAGES, see 
find_zone_movable_pfns_for_nodes().

Note that MAX_ORDER_NR_PAGES >= PAGEBLOCK_NR_PAGES.

I think the same is true for any other zone as well. (DMA / DMA32 etc 
should always be at suitable boundaries already IIRC).

Note that memory hotplug always works in section granularity, and a 
section is always <= MAX_ORDER_NR_PAGES, so it's also guaranteed there.

-- 
Cheers

David / dhildenb