Re: [RFC] mm/hotplug: Make get_nid_for_pfn() work with HAVE_ARCH_PFN_VALID

[Oscar Salvador]

On Thu, Mar 21, 2019 at 01:38:20PM +0530, Anshuman Khandual wrote:
> Memory hot remove uses get_nid_for_pfn() while tearing down linked sysfs
> entries between memory block and node. It first checks pfn validity with
> pfn_valid_within() before fetching nid. With CONFIG_HOLES_IN_ZONE config
> (arm64 has this enabled) pfn_valid_within() calls pfn_valid().
> 
> pfn_valid() is an arch implementation on arm64 (CONFIG_HAVE_ARCH_PFN_VALID)
> which scans all mapped memblock regions with memblock_is_map_memory(). This
> creates a problem in memory hot remove path which has already removed given
> memory range from memory block with memblock_[remove|free] before arriving
> at unregister_mem_sect_under_nodes().
> 
> During runtime memory hot remove get_nid_for_pfn() needs to validate that
> given pfn has a struct page mapping so that it can fetch required nid. This
> can be achieved just by looking into it's section mapping information. This
> adds a new helper pfn_section_valid() for this purpose. Its same as generic
> pfn_valid().
> 
> This maintains existing behaviour for deferred struct page init case.
> 
> Signed-off-by: Anshuman Khandual <anshuman.khandual@xxxxxxx>

I did not look really close to the patch, but I was dealing with
unregister_mem_sect_under_nodes() some time ago [1].

The thing is, I think we can just make it less complex.
Jonathan tried it out that patch on arm64 back then, and it worked correctly
for him, and it did for me too on x86_64.

I am not sure if I overlooked a corner case during the creation of the patch,
that could lead to problems.
But if not, we can get away with that, and we would not need to worry
about get_nid_for_pfn on hot-remove path.

I plan to revisit the patch in some days, but first I wanted to sort out
the vmemmap stuff, which I am preparing a new version of it.

[1] https://patchwork.kernel.org/patch/10700795/

-- 
Oscar Salvador
SUSE L3