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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <a53cdeee-2e25-5c94-4724-d60af1754b88@arm.com>
Date:   Fri, 22 Mar 2019 12:15:50 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     Oscar Salvador <osalvador@...e.de>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        logang@...tatee.com, hannes@...xchg.org, mhocko@...e.com,
        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 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@....com>
> 
> 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.