linux-kernel - Re: [RFC] mm/hotplug: Make get_nid_for_pfn() work with HAVE_ARCH_PFN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 22 Mar 2019 11:49:30 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        logang@...tatee.com, osalvador@...e.de, hannes@...xchg.org,
        akpm@...ux-foundation.org, richard.weiyang@...il.com,
        rientjes@...gle.com, zi.yan@...rutgers.edu
Subject: Re: [RFC] mm/hotplug: Make get_nid_for_pfn() work with
 HAVE_ARCH_PFN_VALID



On 03/21/2019 02:06 PM, Michal Hocko wrote:
> On Thu 21-03-19 13:38:20, 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().
> 
> Could you be more specific on what is the actual problem please? It
> would be also helpful to mention when is the memblock[remove|free]
> called actually.

The problem is in unregister_mem_sect_under_nodes() as it skips calling into both
instances of sysfs_remove_link() which removes node-memory block sysfs symlinks.
The node enumeration of the memory block still remains in sysfs even if the memory
block itself has been removed.

This happens because get_nid_for_pfn() returns -1 for a given pfn even if it has
a valid associated struct page to fetch the node ID from.

On arm64 (with CONFIG_HOLES_IN_ZONE)

get_nid_for_pfn() -> pfn_valid_within() -> pfn_valid -> memblock_is_map_memory()

At this point memblock for the range has been removed.

__remove_memory()
	memblock_free()
	memblock_remove()	--------> memblock has already been removed
	arch_remove_memory()
		__remove_pages()
			__remove_section()
				unregister_memory_section()
 					remove_memory_section()
						unregister_mem_sect_under_nodes()

There is a dependency on memblock (after it has been removed) through pfn_valid().
  			
> 
>> 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().
> 
> I have to say I do not like this. Having pfn_section_valid != pfn_valid_within
> is just confusing as hell. pfn_valid_within should return true whenever
> a struct page exists and it is sensible (same like pfn_valid). So it
> seems that this is something to be solved on that arch specific side of
> pfn_valid.

At present arm64's pfn_valid() implementation validates the pfn inside sparse
memory section mapping as well memblock. The memblock search excludes memory
with MEMBLOCK_NOMAP attribute. But in this particular instance during hotplug
only section mapping validation for the pfn is good enough.

IIUC the current arm64 pfn_valid() already extends the definition beyond the
availability of a valid struct page to operate on.