linux-kernel - Re: [PATCH] mm: don't do zero_resv_unavail if memmap is not allocated

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] [day] [month] [year] [list]

Date:   Mon, 16 Jul 2018 13:05:36 -0400
From:   Pavel Tatashin <pasha.tatashin@...cle.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     steven.sistare@...cle.com, daniel.m.jordan@...cle.com,
        linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
        kirill.shutemov@...ux.intel.com, linux-mm@...ck.org,
        mgorman@...hsingularity.net, torvalds@...ux-foundation.org,
        gregkh@...uxfoundation.org
Subject: Re: [PATCH] mm: don't do zero_resv_unavail if memmap is not allocated



On 07/16/2018 12:09 PM, Michal Hocko wrote:
> On Mon 16-07-18 11:16:30, Pavel Tatashin wrote:
>> Moving zero_resv_unavail before memmap_init_zone(), caused a regression on
>> x86-32.
>>
>> The cause is that we access struct pages before they are allocated when
>> CONFIG_FLAT_NODE_MEM_MAP is used.
>>
>> free_area_init_nodes()
>>   zero_resv_unavail()
>>     mm_zero_struct_page(pfn_to_page(pfn)); <- struct page is not alloced
>>   free_area_init_node()
>>     if CONFIG_FLAT_NODE_MEM_MAP
>>       alloc_node_mem_map()
>>         memblock_virt_alloc_node_nopanic() <- struct page alloced here
>>
>> On the other hand memblock_virt_alloc_node_nopanic() zeroes all the memory
>> that it returns, so we do not need to do zero_resv_unavail() here.
> 
> This all is subtle as hell and almost impossible to build a sane code on
> top. Your patch sounds good as a stop gap fix but we really need
> something resembling an actual design rather than ad-hoc hacks piled on
> top of each other.z

I totally agree, I started working on figuring out how to simply and improve memmap_init_zone(). But part of the mess is that we simply have too many memmap layouts. We must start removing some of them. But, that also requires an analysis of how to unify them.

> 
>> Fixes: e181ae0c5db9 ("mm: zero unavailable pages before memmap init")
>> Signed-off-by: Pavel Tatashin <pasha.tatashin@...cle.com>
> 
> Acked-by: Michal Hocko <mhocko@...e.com>
> 

Thank you.

Pavel