On 03/21/2019 04:07 PM, Oscar Salvador wrote: > 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. Is there any known corner cases ? > 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. The approach of passing down node ID looks good and will also avoid proposed changes here to get_nid_for_pfn() during memory hot-remove. > > 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/ > Sure. Please keep me copied when you repost this patch. Thank you.
