[{"id":1778894,"web_url":"http://patchwork.ozlabs.org/comment/1778894/","msgid":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","list_archive_url":null,"date":"2017-10-03T12:26:58","subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","submitter":{"id":66979,"url":"http://patchwork.ozlabs.org/api/people/66979/","name":"Michal Hocko","email":"mhocko@kernel.org"},"content":"On Wed 20-09-17 16:17:03, Pavel Tatashin wrote:\n> Without deferred struct page feature (CONFIG_DEFERRED_STRUCT_PAGE_INIT),\n> flags and other fields in \"struct page\"es are never changed prior to first\n> initializing struct pages by going through __init_single_page().\n> \n> With deferred struct page feature enabled, however, we set fields in\n> register_page_bootmem_info that are subsequently clobbered right after in\n> free_all_bootmem:\n> \n>         mem_init() {\n>                 register_page_bootmem_info();\n>                 free_all_bootmem();\n>                 ...\n>         }\n> \n> When register_page_bootmem_info() is called only non-deferred struct pages\n> are initialized. But, this function goes through some reserved pages which\n> might be part of the deferred, and thus are not yet initialized.\n> \n>   mem_init\n>    register_page_bootmem_info\n>     register_page_bootmem_info_node\n>      get_page_bootmem\n>       .. setting fields here ..\n>       such as: page->freelist = (void *)type;\n> \n>   free_all_bootmem()\n>    free_low_memory_core_early()\n>     for_each_reserved_mem_region()\n>      reserve_bootmem_region()\n>       init_reserved_page() <- Only if this is deferred reserved page\n>        __init_single_pfn()\n>         __init_single_page()\n>             memset(0) <-- Loose the set fields here\n> \n> We end-up with issue where, currently we do not observe problem as memory\n> is explicitly zeroed. But, if flag asserts are changed we can start hitting\n> issues.\n> \n> Also, because in this patch series we will stop zeroing struct page memory\n> during allocation, we must make sure that struct pages are properly\n> initialized prior to using them.\n> \n> The deferred-reserved pages are initialized in free_all_bootmem().\n> Therefore, the fix is to switch the above calls.\n\nThanks for extending the changelog. This is more informative now.\n \n> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>\n> Reviewed-by: Steven Sistare <steven.sistare@oracle.com>\n> Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>\n> Reviewed-by: Bob Picco <bob.picco@oracle.com>\n\nI hope I haven't missed anything but it looks good to me.\n\nAcked-by: Michal Hocko <mhocko@suse.com>\n\none nit below\n> ---\n>  arch/x86/mm/init_64.c | 9 +++++++--\n>  1 file changed, 7 insertions(+), 2 deletions(-)\n> \n> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c\n> index 5ea1c3c2636e..30fe22558720 100644\n> --- a/arch/x86/mm/init_64.c\n> +++ b/arch/x86/mm/init_64.c\n> @@ -1182,12 +1182,17 @@ void __init mem_init(void)\n>  \n>  \t/* clear_bss() already clear the empty_zero_page */\n>  \n> -\tregister_page_bootmem_info();\n> -\n>  \t/* this will put all memory onto the freelists */\n>  \tfree_all_bootmem();\n>  \tafter_bootmem = 1;\n>  \n> +\t/* Must be done after boot memory is put on freelist, because here we\n\nstandard code style is to do\n\t/*\n\t * text starts here\n\n> +\t * might set fields in deferred struct pages that have not yet been\n> +\t * initialized, and free_all_bootmem() initializes all the reserved\n> +\t * deferred pages for us.\n> +\t */\n> +\tregister_page_bootmem_info();\n> +\n>  \t/* Register memory areas for /proc/kcore */\n>  \tkclist_add(&kcore_vsyscall, (void *)VSYSCALL_ADDR,\n>  \t\t\t PAGE_SIZE, KCORE_OTHER);\n> -- \n> 2.14.1","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y5yvw1zSGz9ryk\n\tfor <patchwork-incoming@ozlabs.org>;\n\tTue,  3 Oct 2017 23:27:08 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753319AbdJCM1F (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 3 Oct 2017 08:27:05 -0400","from mx2.suse.de ([195.135.220.15]:42132 \"EHLO mx1.suse.de\"\n\trhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP\n\tid S1752365AbdJCM1D (ORCPT <rfc822;sparclinux@vger.kernel.org>);\n\tTue, 3 Oct 2017 08:27:03 -0400","from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254])\n\tby mx1.suse.de (Postfix) with ESMTP id 6F558AC68;\n\tTue,  3 Oct 2017 12:27:00 +0000 (UTC)"],"X-Virus-Scanned":"by amavisd-new at test-mx.suse.de","Date":"Tue, 3 Oct 2017 14:26:58 +0200","From":"Michal Hocko <mhocko@kernel.org>","To":"Pavel Tatashin <pasha.tatashin@oracle.com>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","Subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","Message-ID":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-2-pasha.tatashin@oracle.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20170920201714.19817-2-pasha.tatashin@oracle.com>","User-Agent":"NeoMutt/20170609 (1.8.3)","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}},{"id":1779058,"web_url":"http://patchwork.ozlabs.org/comment/1779058/","msgid":"<00978c7c-8d05-fbc3-eaba-9455b66ff02e@oracle.com>","list_archive_url":null,"date":"2017-10-03T15:07:54","subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","submitter":{"id":71010,"url":"http://patchwork.ozlabs.org/api/people/71010/","name":"Pavel Tatashin","email":"pasha.tatashin@oracle.com"},"content":"Hi Michal,\n\n> \n> I hope I haven't missed anything but it looks good to me.\n> \n> Acked-by: Michal Hocko <mhocko@suse.com>\n\nThank you for your review.\n\n> \n> one nit below\n>> ---\n>>   arch/x86/mm/init_64.c | 9 +++++++--\n>>   1 file changed, 7 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c\n>> index 5ea1c3c2636e..30fe22558720 100644\n>> --- a/arch/x86/mm/init_64.c\n>> +++ b/arch/x86/mm/init_64.c\n>> @@ -1182,12 +1182,17 @@ void __init mem_init(void)\n>>   \n>>   \t/* clear_bss() already clear the empty_zero_page */\n>>   \n>> -\tregister_page_bootmem_info();\n>> -\n>>   \t/* this will put all memory onto the freelists */\n>>   \tfree_all_bootmem();\n>>   \tafter_bootmem = 1;\n>>   \n>> +\t/* Must be done after boot memory is put on freelist, because here we\n> \n> standard code style is to do\n> \t/*\n> \t * text starts here\n\nOK, will change for both patch 1 and 2.\n\nPasha\n--\nTo unsubscribe from this list: send the line \"unsubscribe sparclinux\" in\nthe body of a message to majordomo@vger.kernel.org\nMore majordomo info at  http://vger.kernel.org/majordomo-info.html","headers":{"Return-Path":"<sparclinux-owner@vger.kernel.org>","X-Original-To":"patchwork-incoming@ozlabs.org","Delivered-To":"patchwork-incoming@ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=sparclinux-owner@vger.kernel.org;\n\treceiver=<UNKNOWN>)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y62W66Vh6z9s7B\n\tfor <patchwork-incoming@ozlabs.org>;\n\tWed,  4 Oct 2017 02:09:22 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751891AbdJCPJW (ORCPT <rfc822;patchwork-incoming@ozlabs.org>);\n\tTue, 3 Oct 2017 11:09:22 -0400","from aserp1040.oracle.com ([141.146.126.69]:23981 \"EHLO\n\taserp1040.oracle.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751132AbdJCPJV (ORCPT\n\t<rfc822; sparclinux@vger.kernel.org>); Tue, 3 Oct 2017 11:09:21 -0400","from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74])\n\tby aserp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with\n\tESMTP id v93F7wEU011444\n\t(version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:07:59 GMT","from userv0122.oracle.com (userv0122.oracle.com [156.151.31.75])\n\tby userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id v93F7wGL025883\n\t(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256\n\tverify=OK); Tue, 3 Oct 2017 15:07:58 GMT","from abhmp0002.oracle.com (abhmp0002.oracle.com [141.146.116.8])\n\tby userv0122.oracle.com (8.14.4/8.14.4) with ESMTP id v93F7vLL022241; \n\tTue, 3 Oct 2017 15:07:57 GMT","from [192.168.1.10] (/98.216.35.41)\n\tby default (Oracle Beehive Gateway v4.0)\n\twith ESMTP ; Tue, 03 Oct 2017 08:07:57 -0700"],"Subject":"Re: [PATCH v9 01/12] x86/mm: setting fields in deferred pages","To":"Michal Hocko <mhocko@kernel.org>","Cc":"linux-kernel@vger.kernel.org, sparclinux@vger.kernel.org,\n\tlinux-mm@kvack.org, linuxppc-dev@lists.ozlabs.org,\n\tlinux-s390@vger.kernel.org, linux-arm-kernel@lists.infradead.org,\n\tx86@kernel.org, kasan-dev@googlegroups.com, borntraeger@de.ibm.com,\n\theiko.carstens@de.ibm.com, davem@davemloft.net,\n\twilly@infradead.org, ard.biesheuvel@linaro.org,\n\tmark.rutland@arm.com, will.deacon@arm.com, catalin.marinas@arm.com,\n\tsam@ravnborg.org, mgorman@techsingularity.net,\n\tsteven.sistare@oracle.com, daniel.m.jordan@oracle.com,\n\tbob.picco@oracle.com","References":"<20170920201714.19817-1-pasha.tatashin@oracle.com>\n\t<20170920201714.19817-2-pasha.tatashin@oracle.com>\n\t<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","From":"Pasha Tatashin <pasha.tatashin@oracle.com>","Message-ID":"<00978c7c-8d05-fbc3-eaba-9455b66ff02e@oracle.com>","Date":"Tue, 3 Oct 2017 11:07:54 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20171003122658.cv64pxnuavopjid6@dhcp22.suse.cz>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"7bit","X-Source-IP":"userv0022.oracle.com [156.151.31.74]","Sender":"sparclinux-owner@vger.kernel.org","Precedence":"bulk","List-ID":"<sparclinux.vger.kernel.org>","X-Mailing-List":"sparclinux@vger.kernel.org"}}]