Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH] mm: don't do zero_resv_unavail if memmap is not allocated
To:     Michal Hocko <mhocko@kernel.org>
Cc:     steven.sistare@oracle.com, daniel.m.jordan@oracle.com,
        linux-kernel@vger.kernel.org, akpm@linux-foundation.org,
        kirill.shutemov@linux.intel.com, linux-mm@kvack.org,
        mgorman@techsingularity.net, torvalds@linux-foundation.org,
        gregkh@linuxfoundation.org
References: <20180716151630.770-1-pasha.tatashin@oracle.com>
 <20180716160956.GW17280@dhcp22.suse.cz>
From:   Pavel Tatashin <pasha.tatashin@oracle.com>
Message-ID: <4a4bd630-94f1-ab0b-6516-0955de9b0281@oracle.com>
Date:   Mon, 16 Jul 2018 13:05:36 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <20180716160956.GW17280@dhcp22.suse.cz>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


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@oracle.com>
> 
> Acked-by: Michal Hocko <mhocko@suse.com>
> 

Thank you.

Pavel