Re: [PATCH 2/2] mm/compaction: fix the range to pageblock_pfn_to_page()

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

On Wed, Oct 08, 2025 at 12:17:29PM +0200, David Hildenbrand wrote:
>On 02.10.25 05:31, Wei Yang wrote:
>> Current code may skip some part of the range to
>> pageblock_pfn_to_page() to check whether the range is in the same zone.
>> 
>> Function pageblock_pfn_to_page() is first introduced by commit 7d49d8868336
>> ("mm, compaction: reduce zone checking frequency in the migration scanner"),
>> in which it checks and isolates on the same range [pfn, block_end_pfn].
>> 
>> While after commit e1409c325fdc ("mm/compaction: pass only pageblock
>> aligned range to pageblock_pfn_to_page"), we operate on two different
>> ranges for check and isolation.
>> 
>>    * [block_start_pfn, block_end_pfn] to check it is in the same zone
>>    * [pfn, block_end_pfn] to do isolation
>> 
>> It miss some range to check when start_pfn and zone->zone_start_pfn is
>> in the same pageblock but (start_pfn < zone->zone_start_pfn). The range
>> before zone_start_pfn is missed.
>> 
>>           start_pfn     zone_start_pfn
>>      +----+-------------+-------------------+
>>      block_start_pfn                        block_end_pfn
>> 
>> This leads to the range check is passed, but it will isolate a range in
>> two different zones.
>
>Can that actually happen? I recall that a zone always spans full pageblocks.
>

I have asked myself the same question. Here is what I found from history.

1. From the very beginning, there is this commit.

   commit dc9086004b3d5db75997a645b3fe08d9138b7ad0
   Author: Mel Gorman <mgorman@suse.de>
   Date:   Wed Feb 8 17:13:38 2012 -0800
   
       mm: compaction: check for overlapping nodes during isolation for migration

This one add a check on (page_zone(page) != zone) in isolate_migratepages(),
for overlapping nodes.

And the range to be checked is within one pageblock.

This sounds there is possibility of two zones in one pageblock.

2. Then there was code refactor.

   commit 2fe86e0004076128f05d5a774b5c9c03d9dc3de2
   Author: Michal Nazarewicz <mina86@mina86.com>
   Date:   Mon Jan 30 13:16:26 2012 +0100
   
       mm: compaction: introduce isolate_migratepages_range()

This introduce a wrapper to isolate_migratepages()

    commit edc2ca61249679298c1f343cd9c549964b8df4b4
    Author: Vlastimil Babka <vbabka@suse.cz>
    Date:   Thu Oct 9 15:27:09 2014 -0700
    
        mm, compaction: move pageblock checks up from isolate_migratepages_range()

This move up the check, but still keep the form (page_zone(page) != zone).

3. Introduce pageblock_pfn_to_page()

    commit 7d49d8868336bbf4f68714d8282ca5fd65e387ed
    Author: Vlastimil Babka <vbabka@suse.cz>
    Date:   Thu Oct 9 15:27:11 2014 -0700
    
        mm, compaction: reduce zone checking frequency in the migration scanner

Instead of do (page_zone(page) != page) check in loop, it does the check
before do real job, e.g. isolation.

PS: it looks a little behavioral change.

4. Current state

    commit e1409c325fdc1fef7b3d8025c51892355f065d15
    Author: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    Date:   Tue Mar 15 14:57:48 2016 -0700
    
        mm/compaction: pass only pageblock aligned range to pageblock_pfn_to_page

    commit 7cf91a98e607c2f935dbcc177d70011e95b8faff
    Author: Joonsoo Kim <iamjoonsoo.kim@lge.com>
    Date:   Tue Mar 15 14:57:51 2016 -0700
    
        mm/compaction: speed up pageblock_pfn_to_page() when zone is contiguous

This did some optimization, which seems break something from commit 7d49d8868336.


All in all, if my understanding is correct. All the change comes down from
commit dc9086004b3d5. This means we do have possibility to have two different
zones in on pageblock?

Or this case vanished now?

-- 
Wei Yang
Help you, Help me