Re: [PATCH v2 5/5] mm, memory_hotplug: Refactor shrink_zone/pgdat_span

[osalvador@suse.de]

> OK, so let me try again. What is the difference for a pfn walker to
> start at an offline pfn start from any other offlined section withing a
> zone boundary? I believe there is none because the pfn walker needs to
> skip over offline pfns anyway whether they start at a zone boundary or
> not.

I checked most of the users of zone_start_pnf:

* split_huge_pages_set:
   - It uses pfn_valid().
     I guess this is fine as it will check if the section still has a 
map.

* __reset_isolation_suitable():
   - Safe as it uses pfn_to_online_page().

* isolate_freepages_range():
* isolate_migratepages_range():
* isolate_migratepages():
   - They use pageblock_pfn_to_page().
     If !zone->contiguos, it will use 
__pageblock_pfn_to_page()->pfn_to_online_page()
     If zone->contiguos is true, it will use 
pageblock_pfn_to_page()->pfn_to_page(),
     which is bad because it will not skip over offlined pfns.

* count_highmem_pages:
* count_data_pages:
* copy_data_pages:
   - page_is_saveable()->pfn_valid().
     I guess this is fine as it will check if the section still has a 
map.


So, leaving out isolate_* functions, it seems that we should be safe.
isolate_* functions would depend on !zone->contiguos to call 
__pageblock_pfn_to_page()->pfn_to_online_page().
So whenever we remove a section in a zone, we should clear 
zone->contiguos.
But this really calls for some deep check that we will not shoot in the 
foot.

What I can do for now is to drop this patch from the patchset and 
re-send
v3 without it.